Add an "algorithms" field to all problem documents, but tag it so it
won't be included in the serialized json unless populated. Populate it
only when the problem type is "badSignatureAlgorithm", as specified in
RFC 8555 Section 6.2.
The resulting problem document looks like this:
```json
{
"type": "urn:ietf:params:acme:error:badSignatureAlgorithm",
"detail": "Unable to validate JWS :: JWS signature header contains unsupported algorithm
\"RS512\", expected one of [RS256 ES256 ES384 ES512]",
"status": 400,
"algorithms": [
"RS256",
"ES256",
"ES384",
"ES512"
]
}
```
Fixes https://github.com/letsencrypt/boulder/issues/8155
solve https://github.com/letsencrypt/boulder/issues/8088
RFC8555 6.2 requires badSignatureAlgorithm on unacceptable JWS signing
algorithm, but current boulder return malform:failed to parse jws error
instead
Its because this only checks about JWS protected header's signature
algorithm, current checkAlgorithm is while too late to catch parse time
error but not redundant, as it checks against a key and signed message
---------
Co-authored-by: Samantha Frank <hello@entropy.cat>
Remove the deprecated `DnsNames` field from the `corepb.Order` proto
message. All users of this struct use `Identifiers` instead.
This unblocks future changes that will require `Order` users to handle
different identifier types.
Part of #7311
Remove the deprecated `DnsNames` field from the `CountFQDNSetsRequest`,
`FQDNSetExistsRequest`, and `GetOrderForNamesRequest` structs. All users
of these structs use `Identifier` instead.
Part of #7311
Remove the deprecated `DnsName` field from the core `Authorization`
struct. All users of this struct use `Identifier` instead.
This unblocks future changes that will require `Authorization` users to
handle different identifier types.
Part of #7311
Remove the .Error() method from probs.ProblemDetails, so that it can no
longer be returned from functions which return an error. Update various
call sites to use the .String() method to get a textual representation
of the problem instead. Simplify ProblemDetailsForError to not
special-case and pass-through ProblemDetails, since they are no longer a
valid input to that function.
This reduces instances of "boxed nil" bugs, and paves the way for all of
the WFE methods to be refactored to simply return errors instead of
writing them directly into the response object.
Part of https://github.com/letsencrypt/boulder/issues/4980
Change all of the helper methods and functions in verify.go to return an
`error` instead of a `probs.ProblemDetails`. Add a few new types to our
errors package, and support for those types in ProblemDetailsForError,
to maintain the same public-facing error types. Update the tests to
check for specific errors instead of specific problems.
This is a building block towards making the probs.ProblemDetails type
not implement the Error interface, and only be used when rendering
errors to the user (i.e. not within Boulder logic itself).
Part of https://github.com/letsencrypt/boulder/issues/4980
Add `identifier` fields, which will soon replace the `dnsName` fields,
to:
- `corepb.Authorization`
- `corepb.Order`
- `rapb.NewOrderRequest`
- `sapb.CountFQDNSetsRequest`
- `sapb.CountInvalidAuthorizationsRequest`
- `sapb.FQDNSetExistsRequest`
- `sapb.GetAuthorizationsRequest`
- `sapb.GetOrderForNamesRequest`
- `sapb.GetValidAuthorizationsRequest`
- `sapb.NewOrderRequest`
Populate these `identifier` fields in every function that creates
instances of these structs.
Use these `identifier` fields instead of `dnsName` fields (at least
preferentially) in every function that uses these structs. When crossing
component boundaries, don't assume they'll be present, for
deployability's sake.
Deployability note: Mismatched `cert-checker` and `sa` versions will be
incompatible because of a type change in the arguments to
`sa.SelectAuthzsMatchingIssuance`.
Part of #7311
Change the wfe.parseRevocation function to return `error` instead of
`probs.ProblemDetails`. This slightly changes some of our user-facing
error messages to be more complete and verbose, thanks to how
ProblemDetailsForError works.
This is a building block towards making the probs.ProblemDetails type
not implement the Error interface, and only be used when rendering
errors to the user (i.e. not within Boulder logic itself).
Part of https://github.com/letsencrypt/boulder/issues/4980
Rename `FromDNSNames` to `NewDNSSlice`, since it's exactly `NewDNS`
except for slices.
Rename `AsProto` to use the "To" prefix, since it's the opposite of
"From".
Add a named type `ACMEIdentifiers` so that we can add methods to slices.
We will have a lot of slice handling code coming up, which this will
make more elegant and readable.
Add a comment to explain naming conventions in the `identifier` package.
Part of #7311
Alternative to #8068
Update the SA to re-query the database for the updated account after
deactivating it, and return this to the RA. Update the RA to pass this
value through to the WFE. Update the WFE to return this value, rather
than locally modifying the pre-deactivation account object, if it gets
one (for deployability).
Also remove the RA's requirement that the request object specify its
current status so that the request can be trimmed down to just an ID.
This proto change is backwards-compatible because the new
DeactivateRegistrationRequest's registrationID field has the same type
(int64) and field number (1) as corepb.Registration's id field.
Part of https://github.com/letsencrypt/boulder/issues/5554
- Plumb the "replaces" value from the WFE through to the SA via the RA
- Store validated "replaces" value for new orders in the orders table
- Reflect the stored "replaces" value to subscribers in the order object
- Reorder CertificateProfileName before Replaces/ReplacesSerial in RA
and SA protos for consistency
Fixes#8034
Return "alreadyReplaced" in addition to HTTP 409 Conflict to signal that
an order indicates that it replaces a certificate which already has a
replacement order.
When we receive an update-account request which is not empty, but
doesn't contain the "contact" field, don't assume that they want to
remove their contacts. Only remove contacts if the "contact" field is
present, but empty.
Add a unit test and an integration test which will catch regressions in
this behavior.
PR #8018 integrated the email-exporter service with WFE, updating
wfe.NewAccount and wfe.updateAccount to submit valid email contacts to
the Salesforce Pardot API. However, our new_or_updated_contact metric
shows that (account) contact updates currently exceed the highest
Salesforce tier’s daily submission limit by several times.
This change can be reverted if additional filtering logic reduces
updated (+ new) account contacts below the daily submission limit.
Add a new boulder service, email-exporter, which uses the Pardot API
client added in #8016 and the email.Exporter gRPC service added in
#8017.
Add pardot-test-srv, a test-only service for mocking communication with
Salesforce OAuth and Pardot APIs in non-production environments. Since
Salesforce does not provide Pardot functionality in developer sandboxes,
pardot-test-srv must run in all non-production environments (e.g.,
sre-development and staging).
Integrate the email-exporter service with the WFE and modify
WFE.NewAccount and WFE.UpdateAccount to submit valid email contacts.
Ensure integration tests verify that contacts eventually reach
pardot-test-srv.
Update configuration where necessary to:
- Build pardot-test-srv as a standalone binary.
- Bring up pardot-test-srv and cmd/email-exporter for integration
testing.
- Integrate WFE with cmd/email-exporter when running test/config-next.
Closes#7966
Compute the width of the ARI suggested renewal window as 2% of the
validity period. This means that 90-day certificates have their
suggested window shrink slightly from 48 hours to 43.2 hours, and gives
six-day (160h) certs a suggested window 3.2 hours wide.
Also move the center of that window to the midpoint of the certificate
validity period for certs which are valid for less than 10 days, so that
operators have (proportionally) a little more time to respond to renewal
issues.
Fixes https://github.com/letsencrypt/boulder/issues/7996
Add MaxNames to the set of things that can be configured on a
per-profile basis. Remove all references to the RA's global maxNames,
replacing them with reference's to the current profile's maxNames. Add
code to the RA's main() to copy a globally-configured MaxNames into each
profile, for deployability.
Also remove any understanding of MaxNames from the WFE, as it is
redundant with the RA and is not configured in staging or prod. Instead,
hardcode the upper limit of 100 into the ratelimit package itself.
Fixes https://github.com/letsencrypt/boulder/issues/7993
These paths receive (literally) zero traffic, and they require the WFE
to duplicate the RA's authorization lifetime configuration. Since that
configuration is now per-profile, the WFE can no longer easily replicate
it, and the resulting staleness calculations will be wrong. Remove the
duplicated configuration, remove the unused endpoints that rely on it,
and remove the staleness-checking code which supported those endpoints.
Leave the non-ACME /get/ endpoint for certificates in place, because
checking staleness for those does not require any additional
configuration, and having a non-ACME serial-based API for certificates
is a good thing.
Fixes https://github.com/letsencrypt/boulder/issues/8007
The current profiles draft
(https://datatracker.ietf.org/doc/draft-aaron-acme-profiles/00/) says:
> If a server receives a request to finalize an Order whose profile the
> CA is no longer willing to issue under, it MUST respond with a
> problem document of type "invalidProfile". The server SHOULD attempt
> to avoid this situation, e.g. by ensuring that all Orders for a
> profile have expired before it stops issuing under that profile.
Add types and helper functions representing this new error type to the
berrors, probs, and web packages. Update the WFE code which rejects
new-order requests with unrecognized profiles to use these new types,
and add similar code to the WFE's finalize path. Update the unit and
integration tests to reflect the fact that we now configure at least one
profile in both Staging and Prod (tracked in IN-10574).
This check is duplicated in the next stanza. Instead, replace it with a
check that the acctID encoded in the URL and the acctID corresponding to
the JWS used to sign the request match.
The initialIP column has been defaulted to 0.0.0.0 since #7760. Remove
this field from the all structs while leaving the schema itself intact.
Part of #7917
Remove code using `certificatesPerName` & `newOrdersRL` tables.
Deprecate `DisableLegacyLimitWrites` & `UseKvLimitsForNewOrder` flags.
Remove legacy `ratelimit` package.
Delete these RA test cases:
- `TestAuthzFailedRateLimitingNewOrder` (rl:
`FailedAuthorizationsPerDomainPerAccount`)
- `TestCheckCertificatesPerNameLimit` (rl: `CertificatesPerDomain`)
- `TestCheckExactCertificateLimit` (rl: `CertificatesPerFQDNSet`)
- `TestExactPublicSuffixCertLimit` (rl: `CertificatesPerDomain`)
Rate limits in NewOrder are now enforced by the WFE, starting here:
5a9b4c4b18/wfe2/wfe.go (L781)
We collect a batch of transactions to check limits, check them all at
once, go through and find which one(s) failed, and serve the failure
with the Retry-After that's furthest in the future. All this code
doesn't really need to be tested again; what needs to be tested is that
we're returning the correct failure. That code is
`NewOrderLimitTransactions`, and the `ratelimits` package's tests cover
this.
The public suffix handling behavior is tested by
`TestFQDNsToETLDsPlusOne`:
5a9b4c4b18/ratelimits/utilities_test.go (L9)
Some other RA rate limit tests were deleted earlier, in #7869.
Part of #7671.
And in the RA, log the notBefore of the previous issuance.
To make this happen, I had to hoist the "check for previous certificate"
up a level into `issueCertificateOuter`. That meant I also had to hoist
the "split off a WithoutCancel context" logic all the way up to
`FinalizeOrder`.
This reverts commit 242d746040 (#7796)
We want to make this change, but it carries some risk that we'd prefer
not to take over the holiday. We'd also like to keep `main` in a state
where it would be reasonable to deploy (even if, in practice, any
over-the-holiday deploy would be a hotfix, not a direct tag from
`main`).
This removes the `handlerPath` parameter to various calls, which was
used solely to distinguish the `-v3`-style paths from the `WithAccount`
paths.
Also, this removes `WithAccount` from all names that had it. The fact
that these URLS include an account ID is now implicit.
Add a new `ratelimits.NewTransactionBuilderWithLimits` constructor which
takes pre-populated rate limit data, instead of filenames for reading it
off disk.
Use this new constructor to change rate limits during RA tests, instead
of using extra `testdata` files.
Fix ARI renewals' exception from rate limits: consider `isARIRenewal` as
part of the `isRenewal` arg to `checkNewOrderLimits`.
Remove obsolete RA tests for rate limits that are now only checked in
the WFE.
Update remaining new order rate limit tests from deprecated `ratelimit`s
to new Redis `ratelimits`.
The RA's DeactivateAccount method expects the account provided to it by
the WFE to still have status Valid. The new WFE deactivation code was
hardcoding the status to Deactivated. Fix the WFE to pass the account's
current status instead.
Add an integration test to confirm both the breakage and the fix. Also
leave behind some TODOs to simplify this codepath further, and not
require the status to be provided at all.
Part of #5554
Early drafts of the ACME spec said that clients should retrieve their existing account information by POSTing the empty JSON object `{}` to their account URL. This instruction was removed in the final version of ACME, replaced by the concept of POST-as-GET, which uses a wholly empty body to accomplish the same goal. However, Boulder has continued to incidentally support this behavior: when we receive an empty JSON object, our `updateAccount` code in the RA applies their desired diff (none) on top of their current account, writes it back to the database, and returns the updated account object...which hasn't actually changed. This behavior is also half-tested by `TestEmptyAccount`, but that test is actually testing that the MockRA implements the same behavior as the real RA; it's not truly testing the WFE's behavior.
This PR changes the WFE to explicitly treat receiving the empty JSON object as a request to retrieve the account data unchanged, rather than implicitly relying on internal details of the RA's account-update logic, which are expected to change in #7827.
---------
Co-authored-by: Jacob Hoffman-Andrews <jsha+github@letsencrypt.org>
All 4 usages of the `maps.Keys` function from the
`golang.org/x/exp/maps` package can be refactored to a simpler
alternative. If we need it in the future, it is available in the
standard library since Go 1.23.
For batch operations, include the operation and the number of keys in
the error message. This should help diagnose whether we are getting `i/o
timeout` errors disproportionately for larger requests, or for certain
operations.
Also, make the ignored errors part of the overall WFE request logs,
which allows us to get additional context, like whether certain
requesters or domain names are getting disproportionately many errors.
Related to #7846.
The purpose of these RA and WFE unit tests is to test how they deal with
certain rate limit conditions, not to test talking to an actual redis
instance. Streamline the tests by having them talk to an in-memory rate
limits store, rather than a redis-backed one.
Now that the paths with an account (and no `-v3`) are the default,
rename the old-style path constants and handlers to reflect that they
are deprecated.
Part of #7683.
This means that most traffic will go to the authz URLs with account.
After this has been deployed for 30 days (the max lifetime of an order),
we can remove support for the old paths.
Part of #7683
Aaron Gable
orangepizza
James Renken
Samantha Frank
Jacob Hoffman-Andrews
Eng Zer Jun