From a9791167f9d19d6107d0b2e466deedcefd8e688f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Jan 2026 14:01:10 +0100 Subject: [PATCH] refactor: split NonceManager and NonceSource --- acme/api/api.go | 2 +- acme/api/internal/nonces/nonce_manager.go | 47 +++++++++----- .../api/internal/nonces/nonce_manager_test.go | 55 ++++++++++++++-- acme/api/internal/secure/jws.go | 5 +- acme/api/internal/secure/jws_test.go | 63 ------------------- 5 files changed, 83 insertions(+), 89 deletions(-) delete mode 100644 acme/api/internal/secure/jws_test.go diff --git a/acme/api/api.go b/acme/api/api.go index 76f246e76..c7cb896c9 100644 --- a/acme/api/api.go +++ b/acme/api/api.go @@ -109,7 +109,7 @@ func (a *Core) retrievablePost(ctx context.Context, uri string, content []byte, } func (a *Core) signedPost(ctx context.Context, uri string, content []byte, response any) (*http.Response, error) { - signedContent, err := a.jws.SignContent(uri, content) + signedContent, err := a.jws.SignContent(ctx, uri, content) if err != nil { return nil, fmt.Errorf("failed to post JWS message: failed to sign content: %w", err) } diff --git a/acme/api/internal/nonces/nonce_manager.go b/acme/api/internal/nonces/nonce_manager.go index 7ceaeafc3..f2d0c5ca7 100644 --- a/acme/api/internal/nonces/nonce_manager.go +++ b/acme/api/internal/nonces/nonce_manager.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/go-acme/lego/v5/acme/api/internal/sender" + "github.com/go-jose/go-jose/v4" ) // Manager Manages nonces. @@ -27,6 +28,14 @@ func NewManager(do *sender.Doer, nonceURL string) *Manager { } } +// Push Pushes nonce. +func (n *Manager) Push(nonce string) { + n.Lock() + defer n.Unlock() + + n.nonces = append(n.nonces, nonce) +} + // Pop Pops a nonce. func (n *Manager) Pop() (string, bool) { n.Lock() @@ -42,26 +51,11 @@ func (n *Manager) Pop() (string, bool) { return nonce, true } -// Push Pushes a nonce. -func (n *Manager) Push(nonce string) { - n.Lock() - defer n.Unlock() - - n.nonces = append(n.nonces, nonce) -} - -// Nonce implement jose.NonceSource. -func (n *Manager) Nonce() (string, error) { +func (n *Manager) getNonce(ctx context.Context) (string, error) { if nonce, ok := n.Pop(); ok { return nonce, nil } - // TODO(ldez): the Nonce method signature cannot be changed because it must implement jose.NonceSource. - // Maybe use a dirty context struct field in this case. - return n.getNonce(context.Background()) -} - -func (n *Manager) getNonce(ctx context.Context) (string, error) { resp, err := n.do.Head(ctx, n.nonceURL) if err != nil { return "", fmt.Errorf("failed to get nonce from HTTP HEAD: %w", err) @@ -70,7 +64,7 @@ func (n *Manager) getNonce(ctx context.Context) (string, error) { return GetFromResponse(resp) } -// GetFromResponse Extracts a nonce from an HTTP response. +// GetFromResponse Extracts nonce from an HTTP response. func GetFromResponse(resp *http.Response) (string, error) { if resp == nil { return "", errors.New("nil response") @@ -83,3 +77,22 @@ func GetFromResponse(resp *http.Response) (string, error) { return nonce, nil } + +var _ jose.NonceSource = (*NonceSource)(nil) + +// NonceSource implements [jose.NonceSource]. +// +//nolint:containedctx // This is the only way to use the context in this case. +type NonceSource struct { + ctx context.Context + m *Manager +} + +// NewNonceSource Creates a new NonceSource. +func NewNonceSource(ctx context.Context, manager *Manager) *NonceSource { + return &NonceSource{ctx: ctx, m: manager} +} + +func (n *NonceSource) Nonce() (string, error) { + return n.m.getNonce(n.ctx) +} diff --git a/acme/api/internal/nonces/nonce_manager_test.go b/acme/api/internal/nonces/nonce_manager_test.go index 028f15c6b..eb1c22c2d 100644 --- a/acme/api/internal/nonces/nonce_manager_test.go +++ b/acme/api/internal/nonces/nonce_manager_test.go @@ -11,8 +11,8 @@ import ( "github.com/go-acme/lego/v5/platform/tester/servermock" ) -func TestNotHoldingLockWhileMakingHTTPRequests(t *testing.T) { - manager := servermock.NewBuilder( +func mockBuilder() *servermock.Builder[*Manager] { + return servermock.NewBuilder( func(server *httptest.Server) (*Manager, error) { doer := sender.NewDoer(server.Client(), "lego-test") @@ -25,14 +25,19 @@ func TestNotHoldingLockWhileMakingHTTPRequests(t *testing.T) { rw.Header().Set("Retry-After", "0") servermock.JSONEncode(&acme.Challenge{Type: "http-01", Status: "Valid", URL: "https://example.com/", Token: "token"}).ServeHTTP(rw, req) - })). - BuildHTTPS(t) + })) +} + +func TestManager_getNonce_notHoldingLockWhileMakingHTTPRequests(t *testing.T) { + manager := mockBuilder().BuildHTTPS(t) + + ctx := t.Context() ch := make(chan bool) resultCh := make(chan bool) go func() { - _, errN := manager.Nonce() + _, errN := manager.getNonce(ctx) if errN != nil { t.Log(errN) } @@ -40,7 +45,45 @@ func TestNotHoldingLockWhileMakingHTTPRequests(t *testing.T) { ch <- true }() go func() { - _, errN := manager.Nonce() + _, errN := manager.getNonce(ctx) + if errN != nil { + t.Log(errN) + } + + ch <- true + }() + go func() { + <-ch + <-ch + + resultCh <- true + }() + + select { + case <-resultCh: + case <-time.After(500 * time.Millisecond): + t.Fatal("JWS is probably holding a lock while making HTTP request") + } +} + +func TestNewNonceSource_notHoldingLockWhileMakingHTTPRequests(t *testing.T) { + manager := mockBuilder().BuildHTTPS(t) + + ctx := t.Context() + + ch := make(chan bool) + resultCh := make(chan bool) + + go func() { + _, errN := NewNonceSource(ctx, manager).Nonce() + if errN != nil { + t.Log(errN) + } + + ch <- true + }() + go func() { + _, errN := NewNonceSource(ctx, manager).Nonce() if errN != nil { t.Log(errN) } diff --git a/acme/api/internal/secure/jws.go b/acme/api/internal/secure/jws.go index 02b2d7494..4b29ddabf 100644 --- a/acme/api/internal/secure/jws.go +++ b/acme/api/internal/secure/jws.go @@ -1,6 +1,7 @@ package secure import ( + "context" "crypto" "crypto/ecdsa" "crypto/elliptic" @@ -34,7 +35,7 @@ func (j *JWS) SetKid(kid string) { } // SignContent Signs a content with the JWS. -func (j *JWS) SignContent(url string, content []byte) (*jose.JSONWebSignature, error) { +func (j *JWS) SignContent(ctx context.Context, url string, content []byte) (*jose.JSONWebSignature, error) { var alg jose.SignatureAlgorithm switch k := j.privKey.(type) { @@ -54,7 +55,7 @@ func (j *JWS) SignContent(url string, content []byte) (*jose.JSONWebSignature, e } options := jose.SignerOptions{ - NonceSource: j.nonces, + NonceSource: nonces.NewNonceSource(ctx, j.nonces), ExtraHeaders: map[jose.HeaderKey]any{ "url": url, }, diff --git a/acme/api/internal/secure/jws_test.go b/acme/api/internal/secure/jws_test.go deleted file mode 100644 index 9f6a49402..000000000 --- a/acme/api/internal/secure/jws_test.go +++ /dev/null @@ -1,63 +0,0 @@ -package secure - -import ( - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/go-acme/lego/v5/acme" - "github.com/go-acme/lego/v5/acme/api/internal/nonces" - "github.com/go-acme/lego/v5/acme/api/internal/sender" - "github.com/go-acme/lego/v5/platform/tester/servermock" -) - -func TestNotHoldingLockWhileMakingHTTPRequests(t *testing.T) { - manager := servermock.NewBuilder( - func(server *httptest.Server) (*nonces.Manager, error) { - doer := sender.NewDoer(server.Client(), "lego-test") - - return nonces.NewManager(doer, server.URL), nil - }). - Route("HEAD /", http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - time.Sleep(250 * time.Millisecond) - - rw.Header().Set("Replay-Nonce", "12345") - rw.Header().Set("Retry-After", "0") - - servermock.JSONEncode(&acme.Challenge{Type: "http-01", Status: "Valid", URL: "https://example.com/", Token: "token"}).ServeHTTP(rw, req) - })). - BuildHTTPS(t) - - ch := make(chan bool) - resultCh := make(chan bool) - - go func() { - _, errN := manager.Nonce() - if errN != nil { - t.Log(errN) - } - - ch <- true - }() - go func() { - _, errN := manager.Nonce() - if errN != nil { - t.Log(errN) - } - - ch <- true - }() - go func() { - <-ch - <-ch - - resultCh <- true - }() - - select { - case <-resultCh: - case <-time.After(500 * time.Millisecond): - t.Fatal("JWS is probably holding a lock while making HTTP request") - } -}