Skip to content

Commit 15d77e3

Browse files
fix(auth): refactor DPoP nonce manager to use atomic.Pointer and fix empty nonce validation
Replace sync.RWMutex with atomic.Pointer[nonceState] for lock-free reads on the hot path (getCurrentNonce, validateNonce), keeping sync.Mutex only to serialize writes. Also guards validateNonce against empty-string nonce matching the initial zero-value previousNonce. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
1 parent d95a068 commit 15d77e3

2 files changed

Lines changed: 39 additions & 33 deletions

File tree

service/internal/auth/authn.go

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"sort"
1919
"strings"
2020
"sync"
21+
"sync/atomic"
2122
"time"
2223

2324
"connectrpc.com/connect"
@@ -108,72 +109,72 @@ type Authentication struct {
108109
}
109110

110111
// dpopNonceManager manages server-issued DPoP nonces per RFC 9449 §8
111-
type dpopNonceManager struct {
112-
mu sync.RWMutex
112+
type nonceState struct {
113113
currentNonce string
114114
previousNonce string
115-
expiration time.Duration
116-
lastRotation time.Time
117-
requireNonce bool
115+
rotatedAt time.Time
116+
}
117+
118+
type dpopNonceManager struct {
119+
state atomic.Pointer[nonceState]
120+
mu sync.Mutex // serializes rotation only
121+
expiration time.Duration
122+
requireNonce bool
118123
}
119124

120125
func newDPoPNonceManager(requireNonce bool, expiration time.Duration) *dpopNonceManager {
121126
nm := &dpopNonceManager{
122127
requireNonce: requireNonce,
123128
expiration: expiration,
124129
}
130+
nm.state.Store(&nonceState{})
125131
if requireNonce {
126132
nm.rotate()
127133
}
128134
return nm
129135
}
130136

131-
func (nm *dpopNonceManager) rotate() {
132-
nm.mu.Lock()
133-
defer nm.mu.Unlock()
134-
135-
nm.previousNonce = nm.currentNonce
137+
func (nm *dpopNonceManager) storeRotated(s *nonceState) {
136138
nonce := make([]byte, dpopNonceBytes)
137139
if _, err := rand.Read(nonce); err != nil {
138140
panic(fmt.Sprintf("failed to generate nonce: %v", err))
139141
}
140-
nm.currentNonce = hex.EncodeToString(nonce)
141-
nm.lastRotation = time.Now()
142+
nm.state.Store(&nonceState{
143+
previousNonce: s.currentNonce,
144+
currentNonce: hex.EncodeToString(nonce),
145+
rotatedAt: time.Now(),
146+
})
147+
}
148+
149+
func (nm *dpopNonceManager) rotate() {
150+
nm.mu.Lock()
151+
defer nm.mu.Unlock()
152+
nm.storeRotated(nm.state.Load())
142153
}
143154

144155
func (nm *dpopNonceManager) getCurrentNonce() string {
145-
nm.mu.RLock()
146-
if time.Since(nm.lastRotation) <= nm.expiration {
147-
defer nm.mu.RUnlock()
148-
return nm.currentNonce
156+
s := nm.state.Load()
157+
if time.Since(s.rotatedAt) <= nm.expiration {
158+
return s.currentNonce
149159
}
150-
nm.mu.RUnlock()
151160

152161
nm.mu.Lock()
153162
defer nm.mu.Unlock()
154-
// Double-check after acquiring the write lock to prevent concurrent double-rotation.
155-
if time.Since(nm.lastRotation) > nm.expiration {
156-
nm.previousNonce = nm.currentNonce
157-
nonce := make([]byte, dpopNonceBytes)
158-
if _, err := rand.Read(nonce); err != nil {
159-
panic(fmt.Sprintf("failed to generate nonce: %v", err))
160-
}
161-
nm.currentNonce = hex.EncodeToString(nonce)
162-
nm.lastRotation = time.Now()
163+
// Double-check after acquiring the lock to prevent concurrent double-rotation.
164+
s = nm.state.Load()
165+
if time.Since(s.rotatedAt) > nm.expiration {
166+
nm.storeRotated(s)
167+
s = nm.state.Load()
163168
}
164-
165-
return nm.currentNonce
169+
return s.currentNonce
166170
}
167171

168172
func (nm *dpopNonceManager) validateNonce(nonce string) bool {
169173
if !nm.requireNonce {
170174
return true
171175
}
172-
173-
nm.mu.RLock()
174-
defer nm.mu.RUnlock()
175-
176-
return nonce == nm.currentNonce || nonce == nm.previousNonce
176+
s := nm.state.Load()
177+
return nonce != "" && (nonce == s.currentNonce || nonce == s.previousNonce)
177178
}
178179

179180
// Creates new authN which is used to verify tokens for a set of given issuers

service/internal/auth/dpop_nonce_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ func TestDPoPNonceManager(t *testing.T) {
5959
assert.False(t, nm.validateNonce(currentNonce), "nonce older than previous should not validate")
6060
})
6161

62+
t.Run("empty nonce rejected when required", func(t *testing.T) {
63+
nm := newDPoPNonceManager(true, 5*time.Minute)
64+
assert.False(t, nm.validateNonce(""), "empty nonce must not match initial empty previousNonce")
65+
})
66+
6267
t.Run("disabled nonces", func(t *testing.T) {
6368
nm := newDPoPNonceManager(false, 5*time.Minute)
6469
assert.True(t, nm.validateNonce("any-random-nonce"))

0 commit comments

Comments
 (0)