Repository navigation
chore(authn): Add SAML Assertion cache to prevent replay attacks #23500
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| package saml | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/stackrox/rox/pkg/concurrency" | ||
| "github.com/stackrox/rox/pkg/sync" | ||
| ) | ||
|
|
||
| // Clock is a function that returns the current time. Used for testing. | ||
| type Clock func() time.Time | ||
|
|
||
| // replayCache tracks SAML assertion IDs to prevent replay attacks. | ||
| // It automatically expires entries at their SessionNotOnOrAfter time. | ||
| type replayCache struct { | ||
| // mu guards access to entries | ||
| mu sync.RWMutex | ||
| entries map[string]time.Time // assertion ID -> expiration time | ||
|
|
||
| // cleanupMu guards the start and stop of cache pruning | ||
| cleanupMu sync.Mutex | ||
| stopSig concurrency.Signal | ||
| isRunning bool // Tracks whether pruning goroutine is active | ||
|
|
||
| clock Clock // Clock function for obtaining current time | ||
| } | ||
|
|
||
| func newReplayCache() *replayCache { | ||
| return newReplayCacheWithClock(time.Now) | ||
| } | ||
|
|
||
| func newReplayCacheWithClock(clock Clock) *replayCache { | ||
| if clock == nil { | ||
| clock = time.Now | ||
| } | ||
| return &replayCache{ | ||
| entries: make(map[string]time.Time), | ||
| clock: clock, | ||
| } | ||
| } | ||
|
|
||
| // add checks if an assertion ID has been seen before and is still valid. | ||
| // If not seen or expired, it records the assertion ID with the given expiration time. | ||
| // Returns false if the assertion was already seen and not expired (replay attack), true otherwise. | ||
| func (c *replayCache) add(assertionID string, expiredAt time.Time) bool { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| if _, exists := c.entries[assertionID]; exists { | ||
| return false // Already seen - likely a replay attack | ||
| } | ||
|
|
||
| c.entries[assertionID] = expiredAt | ||
| return true | ||
| } | ||
|
|
||
| // prune removes expired entries from the cache. | ||
| func (c *replayCache) prune() { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| now := c.clock() | ||
| for id, expiredAt := range c.entries { | ||
| if now.After(expiredAt) { | ||
| delete(c.entries, id) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // start starts periodic pruning of expired entries. | ||
| // Can be called multiple times safely - it will start the pruning goroutine if not already running. | ||
| func (c *replayCache) start() { | ||
| c.cleanupMu.Lock() | ||
| defer c.cleanupMu.Unlock() | ||
|
|
||
| // If pruning is already running, do nothing | ||
| if c.isRunning { | ||
| return | ||
| } | ||
|
|
||
| c.stopSig.Reset() | ||
| c.isRunning = true | ||
|
Comment on lines
+77
to
+82
Contributor
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win A
Fix: read the done channel inside 🔒️ Proposed fix c.stopSig.Reset()
c.isRunning = true
+ done := c.stopSig.Done()
go func() {
ticker := time.NewTicker(5 * time.Minute)
defer ticker.Stop()
for {
select {
case <-ticker.C:
c.prune()
- case <-c.stopSig.Done():
- c.cleanupMu.Lock()
- c.isRunning = false
- c.cleanupMu.Unlock()
+ case <-done:
return
}
}
}()
}
func (c *replayCache) stop() {
c.cleanupMu.Lock()
defer c.cleanupMu.Unlock()
c.stopSig.Signal()
+ c.isRunning = false
}Also applies to: 92-96, 103-107 🤖 Prompt for AI Agents |
||
|
|
||
| go func() { | ||
| ticker := time.NewTicker(5 * time.Minute) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ticker.C: | ||
| c.prune() | ||
| case <-c.stopSig.Done(): | ||
| concurrency.WithLock(&c.cleanupMu, func() { | ||
| c.isRunning = false | ||
| }) | ||
| return | ||
| } | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| // stop halts the pruning goroutine. Can be called multiple times safely. | ||
| func (c *replayCache) stop() { | ||
| c.cleanupMu.Lock() | ||
| defer c.cleanupMu.Unlock() | ||
| c.stopSig.Signal() | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Base the cache expiry on how long the assertion stays valid, not on the session lifetime.
The cache must keep an assertion ID for as long as the assertion can still be accepted. That window ends at
Conditions.NotOnOrAfterorSubjectConfirmationData.NotOnOrAfter.SessionNotOnOrAfteris a different value: it sets the session lifetime, and it can be earlier than the assertion's validity end. When no session expiry is present, the code uses a 1-hour fallback, which can also end before the assertion does.In both cases,
prune()deletes the ID while the assertion is still valid. A captured response can then be replayed until its validity window closes.The fallback also uses
time.Now().validateAssertionWarningschecks validity againstp.sp.Clock, so the cache and the validity check can disagree about the current time.Fix: compute
expiresAtas the latest of these times:assertion.Conditions.NotOnOrAfterSubjectConfirmationData.NotOnOrAfterSessionNotOnOrAfterFall back to the current time plus a fixed TTL only when none of these values parse.
🛡️ Sketch
🤖 Prompt for AI Agents