From c9b8d8b1f485f1e8d8cbaa660e0a39d0e2981934 Mon Sep 17 00:00:00 2001 From: Akshith Gunasekaran Date: Thu, 11 Jun 2026 13:20:33 -0700 Subject: [PATCH 1/2] Use fresh account state for ACME auth --- wfe2/cache.go | 6 +++ wfe2/cache_test.go | 16 +++++++ wfe2/verify.go | 89 ++++++++++++++++++++++++++++----------- wfe2/verify_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++- wfe2/wfe.go | 41 ++++++++++++------ 5 files changed, 214 insertions(+), 38 deletions(-) diff --git a/wfe2/cache.go b/wfe2/cache.go index 442594440ef..d59dec9e747 100644 --- a/wfe2/cache.go +++ b/wfe2/cache.go @@ -98,6 +98,12 @@ func (ac *accountCache) GetRegistration(ctx context.Context, regID *sapb.Registr return copied, nil } +func (ac *accountCache) purgeRegistration(regID int64) { + ac.Lock() + ac.cache.Remove(regID) + ac.Unlock() +} + func (ac *accountCache) queryAndStore(ctx context.Context, regID *sapb.RegistrationID) (*corepb.Registration, error) { account, err := ac.under.GetRegistration(ctx, regID) if err != nil { diff --git a/wfe2/cache_test.go b/wfe2/cache_test.go index f4f0fafe1e4..56ff36f88af 100644 --- a/wfe2/cache_test.go +++ b/wfe2/cache_test.go @@ -107,6 +107,22 @@ func TestCacheExpires(t *testing.T) { test.AssertEquals(t, len(backend.requests), 2) } +func TestCachePurgeRegistration(t *testing.T) { + ctx := context.Background() + backend := &recordingBackend{} + cache := NewAccountCache(backend, 10, time.Second, clock.NewFake(), metrics.NoopRegisterer) + + _, err := cache.GetRegistration(ctx, &sapb.RegistrationID{Id: 1234}) + test.AssertNotError(t, err, "getting registration") + test.AssertEquals(t, len(backend.requests), 1) + + cache.purgeRegistration(1234) + + _, err = cache.GetRegistration(ctx, &sapb.RegistrationID{Id: 1234}) + test.AssertNotError(t, err, "getting registration") + test.AssertEquals(t, len(backend.requests), 2) +} + type wrongIDBackend struct{} func (wib wrongIDBackend) GetRegistration( diff --git a/wfe2/verify.go b/wfe2/verify.go index a6f84d057b4..fe16ed7c5d3 100644 --- a/wfe2/verify.go +++ b/wfe2/verify.go @@ -300,9 +300,9 @@ func (wfe *WebFrontEndImpl) validPOSTURL( func (wfe *WebFrontEndImpl) matchJWSURLs(outer, inner jose.Header) error { // Verify that the outer JWS has a non-empty URL header. This is strictly // defensive since the expectation is that endpoints using `matchJWSURLs` - // have received at least one of their JWS from calling validPOSTForAccount(), - // which checks the outer JWS has the expected URL header before processing - // the inner JWS. + // have received at least one of their JWS from an account-authenticated + // verifier, which checks the outer JWS has the expected URL header before + // processing the inner JWS. outerURL, ok := outer.ExtraHeaders[jose.HeaderKey("url")].(string) if !ok || len(outerURL) == 0 { wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverOuterJWSNoURL"}).Inc() @@ -493,6 +493,15 @@ func (wfe *WebFrontEndImpl) lookupJWK( ctx context.Context, request *http.Request, logEvent *web.RequestEvent) (*jose.JSONWebKey, *core.Registration, error) { + return wfe.lookupJWKUsing(header, ctx, request, logEvent, wfe.accountGetter) +} + +func (wfe *WebFrontEndImpl) lookupJWKUsing( + header jose.Header, + ctx context.Context, + request *http.Request, + logEvent *web.RequestEvent, + accountGetter AccountGetter) (*jose.JSONWebKey, *core.Registration, error) { // We expect the request to be using an embedded Key ID auth type and to not // contain the mutually exclusive embedded JWK. if err := wfe.enforceJWSAuthType(header, embeddedKeyID); err != nil { @@ -506,8 +515,8 @@ func (wfe *WebFrontEndImpl) lookupJWK( return nil, nil, err } - // Try to find the account for this account ID - account, err := wfe.accountGetter.GetRegistration(ctx, &sapb.RegistrationID{Id: accountID}) + // Try to find the account for this account ID. + account, err := accountGetter.GetRegistration(ctx, &sapb.RegistrationID{Id: accountID}) if err != nil { // If the account isn't found, return a suitable error if errors.Is(err, berrors.NotFound) { @@ -605,8 +614,23 @@ func (wfe *WebFrontEndImpl) validJWSForAccount( request *http.Request, ctx context.Context, logEvent *web.RequestEvent) ([]byte, *bJSONWebSignature, *core.Registration, error) { + return wfe.validJWSForAccountUsing(jws, request, ctx, logEvent, wfe.accountGetter) +} + +func (wfe *WebFrontEndImpl) validJWSForAccountUsing( + jws *bJSONWebSignature, + request *http.Request, + ctx context.Context, + logEvent *web.RequestEvent, + accountGetter AccountGetter) ([]byte, *bJSONWebSignature, *core.Registration, error) { // Lookup the account and JWK for the key ID that authenticated the JWS - pubKey, account, err := wfe.lookupJWK(jws.Signatures[0].Header, ctx, request, logEvent) + pubKey, account, err := wfe.lookupJWKUsing( + jws.Signatures[0].Header, + ctx, + request, + logEvent, + accountGetter, + ) if err != nil { return nil, nil, nil, err } @@ -620,35 +644,52 @@ func (wfe *WebFrontEndImpl) validJWSForAccount( return payload, jws, account, nil } -// validPOSTForAccount checks that a given POST request has a valid JWS -// using `validJWSForAccount`. If valid, the authenticated JWS body and the -// registration that authenticated the body are returned. Otherwise a error is -// returned. The returned JWS body may be empty if the request is a POST-as-GET -// request. +// validPOSTForAccount checks a POST request against the configured account +// getter, which may be the WFE-local account cache. Use +// validPOSTForCurrentAccount before account-authenticated requests cross +// issuance or account mutation boundaries. func (wfe *WebFrontEndImpl) validPOSTForAccount( request *http.Request, ctx context.Context, logEvent *web.RequestEvent) ([]byte, *bJSONWebSignature, *core.Registration, error) { + return wfe.validPOSTForAccountUsing(request, ctx, logEvent, wfe.accountGetter) +} + +// validPOSTForCurrentAccount checks a POST request against current SA account +// state, bypassing the WFE-local account cache. Use it before account-authenticated +// requests cross issuance or account mutation boundaries. +func (wfe *WebFrontEndImpl) validPOSTForCurrentAccount( + request *http.Request, + ctx context.Context, + logEvent *web.RequestEvent) ([]byte, *bJSONWebSignature, *core.Registration, error) { + return wfe.validPOSTForAccountUsing(request, ctx, logEvent, wfe.sa) +} + +func (wfe *WebFrontEndImpl) validPOSTForAccountUsing( + request *http.Request, + ctx context.Context, + logEvent *web.RequestEvent, + accountGetter AccountGetter) ([]byte, *bJSONWebSignature, *core.Registration, error) { // Parse the JWS from the POST request jws, err := wfe.parseJWSRequest(request) if err != nil { return nil, nil, nil, err } - return wfe.validJWSForAccount(jws, request, ctx, logEvent) + return wfe.validJWSForAccountUsing(jws, request, ctx, logEvent, accountGetter) } // validPOSTAsGETForAccount checks that a given POST request is valid using -// `validPOSTForAccount`. It additionally validates that the JWS request payload -// is empty, indicating that it is a POST-as-GET request per ACME draft 15+ -// section 6.3 "GET and POST-as-GET requests". If a non empty payload is -// provided in the JWS the invalidPOSTAsGETErr error is returned. This -// function is useful only for endpoints that do not need to handle both POSTs -// with a body and POST-as-GET requests (e.g. Order, Certificate). +// the configured account getter, which may be the WFE-local account cache. It +// additionally validates that the JWS request payload is empty, indicating +// that it is a POST-as-GET request per ACME draft 15+ section 6.3 "GET and +// POST-as-GET requests". If a non empty payload is provided in the JWS the +// invalidPOSTAsGETErr error is returned. This function is useful only for +// endpoints that do not need to handle both POSTs with a body and POST-as-GET +// requests (e.g. Order, Certificate). func (wfe *WebFrontEndImpl) validPOSTAsGETForAccount( request *http.Request, ctx context.Context, logEvent *web.RequestEvent) (*core.Registration, error) { - // Call validPOSTForAccount to verify the JWS and extract the body. body, _, reg, err := wfe.validPOSTForAccount(request, ctx, logEvent) if err != nil { return nil, err @@ -747,9 +788,9 @@ type rolloverOperation struct { // validKeyRollover checks if the innerJWS is a valid key rollover operation // given the outer JWS that carried it. It is assumed that the outerJWS has -// already been validated per the normal ACME process using `validPOSTForAccount`. -// It is *critical* this is the case since `validKeyRollover` does not check the -// outerJWS signature. This function checks that: +// already been validated per the normal ACME process. It is *critical* this is +// the case since `validKeyRollover` does not check the outerJWS signature. This +// function checks that: // 1) the inner JWS is valid and well formed // 2) the inner JWS has the same "url" header as the outer JWS // 3) the inner JWS is self-authenticated with an embedded JWK @@ -798,8 +839,8 @@ func (wfe *WebFrontEndImpl) validKeyRollover( return nil, berrors.MalformedError("Inner JWS does not verify with embedded JWK") } // NOTE(@cpu): we do not stomp the web.RequestEvent's payload here since that is set - // from the outerJWS in validPOSTForAccount and contains the inner JWS and inner - // payload already. + // from the outerJWS by the account-authenticated verifier and contains the + // inner JWS and inner payload already. // Verify that the outer and inner JWS protected URL headers match if err := wfe.matchJWSURLs(outerJWS.Signatures[0].Header, innerJWS.Signatures[0].Header); err != nil { diff --git a/wfe2/verify_test.go b/wfe2/verify_test.go index 1b62c3cfa5f..5ce13ab045d 100644 --- a/wfe2/verify_test.go +++ b/wfe2/verify_test.go @@ -24,9 +24,12 @@ import ( "github.com/letsencrypt/boulder/goodkey" bgrpc "github.com/letsencrypt/boulder/grpc" "github.com/letsencrypt/boulder/grpc/noncebalancer" + "github.com/letsencrypt/boulder/metrics" + "github.com/letsencrypt/boulder/nonce" noncepb "github.com/letsencrypt/boulder/nonce/proto" sapb "github.com/letsencrypt/boulder/sa/proto" "github.com/letsencrypt/boulder/test" + inmemnonce "github.com/letsencrypt/boulder/test/inmem/nonce" "github.com/letsencrypt/boulder/web" ) @@ -1420,8 +1423,103 @@ func TestValidPOSTForAccount(t *testing.T) { } } +type staticRegistrationGetter struct { + sapb.StorageAuthorityReadOnlyClient + registration *corepb.Registration + calls int +} + +func (getter *staticRegistrationGetter) GetRegistration( + _ context.Context, + in *sapb.RegistrationID, + _ ...grpc.CallOption, +) (*corepb.Registration, error) { + getter.calls++ + if in.Id == getter.registration.Id { + return getter.registration, nil + } + return nil, berrors.NotFound +} + +type rejectingRegistrationGetter struct { + sapb.StorageAuthorityReadOnlyClient + t *testing.T +} + +func (getter rejectingRegistrationGetter) GetRegistration( + _ context.Context, + _ *sapb.RegistrationID, + _ ...grpc.CallOption, +) (*corepb.Registration, error) { + getter.t.Fatal("validPOSTForCurrentAccount used cached account getter") + return nil, nil +} + +func registrationForAuthTest(key []byte, status core.AcmeStatus) *corepb.Registration { + return &corepb.Registration{ + Id: 1, + Key: key, + Agreement: agreementURL, + Status: string(status), + } +} + +func setupAuthOnlyWFE( + t *testing.T, + freshAccount *corepb.Registration, +) (WebFrontEndImpl, requestSigner, *staticRegistrationGetter) { + t.Helper() + + rncKey := []byte("b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f") + noncePrefix := nonce.DerivePrefix("192.168.1.1:8080", rncKey) + nonceService, err := nonce.NewNonceService(metrics.NoopRegisterer, 100, noncePrefix) + test.AssertNotError(t, err, "making nonceService") + + inmemNonceService := &inmemnonce.NonceService{Impl: nonceService} + freshGetter := &staticRegistrationGetter{registration: freshAccount} + wfe := WebFrontEndImpl{ + sa: freshGetter, + rnc: inmemNonceService, + rncKey: rncKey, + accountGetter: rejectingRegistrationGetter{t: t}, + stats: initStats(metrics.NoopRegisterer), + } + + return wfe, requestSigner{t, inmemNonceService.AsSource()}, freshGetter +} + +func TestValidPOSTForCurrentAccountRejectsCachedDeactivatedAccount(t *testing.T) { + wfe, signer, freshGetter := setupAuthOnlyWFE( + t, + registrationForAuthTest([]byte(test1KeyPublicJSON), core.StatusDeactivated), + ) + + _, _, body := signer.byKeyID(1, nil, "http://localhost/test", "{}") + request := makePostRequestWithPath("test", body) + + _, _, _, err := wfe.validPOSTForCurrentAccount(request, ctx, newRequestEvent()) + test.AssertErrorIs(t, err, berrors.Unauthorized) + test.AssertContains(t, err.Error(), `Account is not valid, has status "deactivated"`) + test.AssertEquals(t, freshGetter.calls, 1) +} + +func TestValidPOSTForCurrentAccountRejectsCachedPreRolloverKey(t *testing.T) { + wfe, signer, freshGetter := setupAuthOnlyWFE( + t, + registrationForAuthTest([]byte(test2KeyPublicJSON), core.StatusValid), + ) + + _, _, body := signer.byKeyID(1, nil, "http://localhost/test", "{}") + request := makePostRequestWithPath("test", body) + + _, _, _, err := wfe.validPOSTForCurrentAccount(request, ctx, newRequestEvent()) + test.AssertErrorIs(t, err, berrors.Malformed) + test.AssertContains(t, err.Error(), "JWS verification error") + test.AssertEquals(t, freshGetter.calls, 1) +} + // TestValidPOSTAsGETForAccount tests POST-as-GET processing. Because -// wfe.validPOSTAsGETForAccount calls `wfe.validPOSTForAccount` to do all +// wfe.validPOSTAsGETForAccount uses the configured account getter for all // processing except the empty body test we do not duplicate the // `TestValidPOSTForAccount` testcases here. func TestValidPOSTAsGETForAccount(t *testing.T) { diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 03b7740ad32..049c70e5a92 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -185,6 +185,17 @@ type WebFrontEndImpl struct { certProfiles map[string]string } +type accountCachePurger interface { + purgeRegistration(regID int64) +} + +func (wfe *WebFrontEndImpl) purgeCachedAccount(regID int64) { + cache, ok := wfe.accountGetter.(accountCachePurger) + if ok { + cache.purgeRegistration(regID) + } +} + // NewWebFrontEndImpl constructs a web service for Boulder func NewWebFrontEndImpl( stats prometheus.Registerer, @@ -1016,7 +1027,13 @@ func (wfe *WebFrontEndImpl) revokeCertBySubscriberKey( logEvent *web.RequestEvent) error { // For Key ID revocations we authenticate the outer JWS by using // `validJWSForAccount` similar to other WFE endpoints - jwsBody, _, acct, err := wfe.validJWSForAccount(outerJWS, request, ctx, logEvent) + jwsBody, _, acct, err := wfe.validJWSForAccountUsing( + outerJWS, + request, + ctx, + logEvent, + wfe.sa, + ) if err != nil { return err } @@ -1109,7 +1126,7 @@ func (wfe *WebFrontEndImpl) RevokeCertificate( // The ACME specification handles the verification of revocation requests // differently from other endpoints. For this reason we do *not* immediately - // call `wfe.validPOSTForAccount` like all of the other endpoints. + // authenticate the request against an account like most other endpoints. // For this endpoint we need to accept a JWS with an embedded JWK, or a JWS // with an embedded key ID, handling each case differently in terms of which // certificates are authorized to be revoked by the requester @@ -1327,10 +1344,9 @@ func (wfe *WebFrontEndImpl) postChallenge( authz core.Authorization, challengeIndex int, logEvent *web.RequestEvent) { - body, _, currAcct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + body, _, currAcct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { - // validPOSTForAccount handles its own setting of logEvent.Errors wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) return } @@ -1419,10 +1435,9 @@ func (wfe *WebFrontEndImpl) Account( logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { - body, _, currAcct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + body, _, currAcct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { - // validPOSTForAccount handles its own setting of logEvent.Errors wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) return } @@ -1454,6 +1469,7 @@ func (wfe *WebFrontEndImpl) Account( wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update account"), nil) return } + wfe.purgeCachedAccount(acct.ID) } if len(wfe.SubscriberAgreementURL) > 0 { @@ -1574,7 +1590,7 @@ func (wfe *WebFrontEndImpl) Authorization( // B) a POST-as-GET to query the authorization details if request.Method == "POST" { // Both POST options need to be authenticated by an account - body, _, acct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + body, _, acct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) @@ -1944,9 +1960,8 @@ func (wfe *WebFrontEndImpl) KeyRollover( logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { - // Validate the outer JWS on the key rollover in standard fashion using - // validPOSTForAccount - outerBody, outerJWS, acct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + // Validate the outer JWS against current account state before changing keys. + outerBody, outerJWS, acct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) @@ -2045,6 +2060,7 @@ func (wfe *WebFrontEndImpl) KeyRollover( wfe.sendError(response, logEvent, probs.ServerInternal("Error marshaling proto to registration"), err) return } + wfe.purgeCachedAccount(updatedAcct.ID) err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, updatedAcct) if err != nil { @@ -2332,10 +2348,9 @@ func (wfe *WebFrontEndImpl) NewOrder( logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { - body, _, acct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + body, _, acct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { - // validPOSTForAccount handles its own setting of logEvent.Errors wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) return } @@ -2588,7 +2603,7 @@ func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestE func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { // Validate the POST body signature and get the authenticated account for this // finalize order request - body, _, acct, err := wfe.validPOSTForAccount(request, ctx, logEvent) + body, _, acct, err := wfe.validPOSTForCurrentAccount(request, ctx, logEvent) addRequesterHeader(response, logEvent.Requester) if err != nil { wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) From 29fac42dd5325c5331f1cde1c97c130db2a65170 Mon Sep 17 00:00:00 2001 From: Akshith Gunasekaran Date: Thu, 11 Jun 2026 14:54:04 -0700 Subject: [PATCH 2/2] Fix fresh account auth CI failures --- test/integration/key_rollover_test.go | 2 +- wfe2/verify.go | 7 ++++++- wfe2/wfe_test.go | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/integration/key_rollover_test.go b/test/integration/key_rollover_test.go index 1873864e309..9a44956fb0c 100644 --- a/test/integration/key_rollover_test.go +++ b/test/integration/key_rollover_test.go @@ -41,7 +41,7 @@ func TestAccountKeyChange(t *testing.T) { key3, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) test.AssertNotError(t, err, "creating P-384 account key") - acct3, err := c.AccountKeyChange(acct1, key3) + acct3, err := c.AccountKeyChange(acct2, key3) test.AssertNotError(t, err, "rolling over account key") test.AssertEquals(t, acct3.URL, acct1.URL) } diff --git a/wfe2/verify.go b/wfe2/verify.go index fe16ed7c5d3..6bb528f9837 100644 --- a/wfe2/verify.go +++ b/wfe2/verify.go @@ -652,7 +652,12 @@ func (wfe *WebFrontEndImpl) validPOSTForAccount( request *http.Request, ctx context.Context, logEvent *web.RequestEvent) ([]byte, *bJSONWebSignature, *core.Registration, error) { - return wfe.validPOSTForAccountUsing(request, ctx, logEvent, wfe.accountGetter) + // Parse the JWS from the POST request + jws, err := wfe.parseJWSRequest(request) + if err != nil { + return nil, nil, nil, err + } + return wfe.validJWSForAccount(jws, request, ctx, logEvent) } // validPOSTForCurrentAccount checks a POST request against current SA account diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index ab0e4b92b46..a4e6efdd44d 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -4077,6 +4077,7 @@ func TestOrderMatchesReplacement(t *testing.T) { test.AssertNotError(t, err, "failed to create test certificate") wfe.sa = &mockSAForARI{ + StorageAuthorityReadOnlyClient: wfe.sa, cert: &corepb.Certificate{ RegistrationID: 1, Serial: expectSerial.String(), @@ -4244,6 +4245,7 @@ func TestCountNewOrderWithReplaces(t *testing.T) { // MockSA that returns the certificate with the expected serial. wfe.sa = &mockSAForARI{ + StorageAuthorityReadOnlyClient: wfe.sa, cert: &corepb.Certificate{ RegistrationID: 1, Serial: core.SerialToString(expectSerial), @@ -4310,6 +4312,7 @@ func TestNewOrderRateLimits(t *testing.T) { // Mock SA that returns the certificate with the expected serial. wfe.sa = &mockSAForARI{ + StorageAuthorityReadOnlyClient: wfe.sa, cert: &corepb.Certificate{ RegistrationID: 1, Serial: core.SerialToString(extantCert.SerialNumber),