Skip to content

Commit

Permalink
Fix downstream race conditions with NewAccountFunc
Browse files Browse the repository at this point in the history
These functions typically modify the ACMEIssuer.
Only one such consumer of this API is known (Caddy).
  • Loading branch information
mholt committed Jul 19, 2023
1 parent 693a79b commit 51b3190
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
3 changes: 3 additions & 0 deletions acmeclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ func (iss *ACMEIssuer) newACMEClientWithAccount(ctx context.Context, useTestCA,
// register account if it is new
if account.Status == "" {
if iss.NewAccountFunc != nil {
// obtain lock here, since NewAccountFunc calls happen concurrently and they typically read and change the issuer
iss.mu.Lock()
account, err = iss.NewAccountFunc(ctx, iss, account)
iss.mu.Unlock()
if err != nil {
return nil, fmt.Errorf("account pre-registration callback: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion acmeissuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ type ACMEIssuer struct {
// synchronize properly.
email string
agreed bool
mu *sync.Mutex // protects the above grouped fields
mu *sync.Mutex // protects the above grouped fields, as well as entire struct during NewAccountFunc calls
}

// NewACMEIssuer constructs a valid ACMEIssuer based on a template
Expand Down

0 comments on commit 51b3190

Please sign in to comment.