Skip to content

Commit e7968de

Browse files
committed
fix(authx): prevent deadlock in dynamic auth fetching
resolve deadlock that occurs when dynamic auth templates trigger recursive auth requests during execution. RCA: 1. `GetStrategies()` calls `Fetch()` to retrieve auth creds. 2. `Fetch()` executes auth template via cb. 3. template exec triggers HTTP requests requiring auth. 4. recursive calls `GetStrategies()` → `Fetch()` cause deadlock on mutex. notable changes: * add `fetching` flag to `Dynamic` struct to track fetch-in-progress state. * modify `GetStrategies()` to return empty strategies if already fetching. * update `Fetch()` method with proper recursive call prevention. * use mutex-protected flag reads to ensure thread safety. * refactor `GetStrategies()` with local function for code reuse. this prevents infinite recursion during auth template execution while maintaining proper sync and err handling. fixes goroutine deadlocks in auth system when using dynamic secrets with templates that require auth. Signed-off-by: Dwi Siswanto <[email protected]>
1 parent 5ef37da commit e7968de

File tree

1 file changed

+53
-12
lines changed

1 file changed

+53
-12
lines changed

pkg/authprovider/authx/dynamic.go

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type Dynamic struct {
3232
fetchCallback LazyFetchSecret `json:"-" yaml:"-"`
3333
m *sync.Mutex `json:"-" yaml:"-"` // mutex for lazy fetch
3434
fetched bool `json:"-" yaml:"-"` // flag to check if the secret has been fetched
35+
fetching bool `json:"-" yaml:"-"` // flag to check if we're currently fetching (prevents recursion)
3536
error error `json:"-" yaml:"-"` // error if any
3637
}
3738

@@ -184,34 +185,74 @@ func (d *Dynamic) applyValuesToSecret(secret *Secret) error {
184185

185186
// GetStrategy returns the auth strategies for the dynamic secret
186187
func (d *Dynamic) GetStrategies() []AuthStrategy {
187-
if !d.fetched {
188-
_ = d.Fetch(true)
188+
getStrategies := func() []AuthStrategy {
189+
var strategies []AuthStrategy
190+
if d.Secret != nil {
191+
strategies = append(strategies, d.GetStrategy())
192+
}
193+
194+
for _, secret := range d.Secrets {
195+
strategies = append(strategies, secret.GetStrategy())
196+
}
197+
198+
return strategies
189199
}
190-
if d.error != nil {
191-
return nil
200+
201+
d.m.Lock()
202+
isFetched := d.fetched
203+
isFetching := d.fetching
204+
d.m.Unlock()
205+
206+
if isFetched {
207+
if d.error != nil {
208+
return nil
209+
}
210+
211+
return getStrategies()
192212
}
193-
var strategies []AuthStrategy
194-
if d.Secret != nil {
195-
strategies = append(strategies, d.GetStrategy())
213+
214+
if isFetching {
215+
// NOTE(dwisiswant0): Bail out w/ empty here, or we will yeet into
216+
// recursion hell. See line 242-245.
217+
return nil
196218
}
197-
for _, secret := range d.Secrets {
198-
strategies = append(strategies, secret.GetStrategy())
219+
220+
_ = d.Fetch(true)
221+
if d.error != nil {
222+
return nil
199223
}
200-
return strategies
224+
225+
return getStrategies()
201226
}
202227

203228
// Fetch fetches the dynamic secret
204229
// if isFatal is true, it will stop the execution if the secret could not be fetched
205230
func (d *Dynamic) Fetch(isFatal bool) error {
206231
d.m.Lock()
207232
defer d.m.Unlock()
233+
208234
if d.fetched {
235+
return d.error
236+
}
237+
238+
if d.fetching {
209239
return nil
210240
}
241+
242+
d.fetching = true
243+
defer func() {
244+
d.fetching = false
245+
}()
246+
211247
d.error = d.fetchCallback(d)
212-
if d.error != nil && isFatal {
213-
gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.error)
248+
if d.error != nil {
249+
if isFatal {
250+
gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.error)
251+
}
252+
} else {
253+
d.fetched = true
214254
}
255+
215256
return d.error
216257
}
217258

0 commit comments

Comments
 (0)