Pending authz reuse is a nice-to-have feature because it allows us to
create fewer rows in the authz database table when creating new orders.
However, stats show that less than 2% of authorizations that we attach
to new orders are reused pending authzs. And as we move towards using a
more streamlined database schema to store our orders, authorizations,
and validation attempts, disabling pending authz reuse will greatly
simplify our database schema and code.
CPS Compliance Review: our CPS does not speak to whether or not we reuse
pending authorizations for new orders.
IN-10859 tracks enabling this flag in prod
Part of https://github.com/letsencrypt/boulder/issues/7715
This reduces the number of validations that get left indefinitely in
"pending" state.
Rename `DrainFinalize()` to `Drain()` to indicate that it now covers
more cases than just finalize.
- Make performRemoteValidation a more generic function that returns a
new remoteResult interface
- Modify the return value of IsCAAValid and PerformValidation to satisfy
the remoteResult interface
- Include compile time checks and tests that pass an arbitrary operation
When a client deactivates a pending authorization, count that towards
their FailedAuthorizationsPerDomainPerAccount and
FailedAuthorizationsForPausingPerDomainPerAccount rate limits. This
should help curb the few clients which constantly create new orders and
authzs, deactivate those pending authzs preventing reuse of them or
their orders, and then rinse and repeat.
Fixes https://github.com/letsencrypt/boulder/issues/7834
- Remove Perspective and RIR from ValidationRecords
- Make ValidationResultToPB Perspective and RIR aware
- Update comment for VA.PerformValidation
- Make verificationRequestEvent more like doDCVAuditLog
- Update language used in problems created by performRemoteValidation to
be more like doRemoteDCV.
Solves CI flakes in TestCertificatesPerDomain and
TestIdentifiersPausedForAccount that are the result of a race on the
Redis database. This has the downside of making failed validations and
successful finalizations take slightly longer.
Return an error and do logging in the caller. This adds early returns on
a number of error conditions, which can prevent nil pointer dereference
in those cases.
Also update the description for AutomaticallyPauseZombieClients.
Follows up #7763.
- Added a new key-value ratelimit
`FailedAuthorizationsForPausingPerDomainPerAccount` which is incremented
each time a client fails a validation.
- As long as capacity exists in the bucket, a successful validation
attempt will reset the bucket back to full capacity.
- Upon exhausting bucket capacity, the RA will send a gRPC to the SA to
pause the `account:identifier`. Further validation attempts will be
rejected by the [WFE](https://github.com/letsencrypt/boulder/pull/7599).
- Added a new feature flag, `AutomaticallyPauseZombieClients`, which
enables automatic pausing of zombie clients in the RA.
- Added a new RA metric `paused_pairs{"paused":[bool],
"repaused":[bool], "grace":[bool]}` to monitor use of this new
functionality.
- Updated `ra_test.go` `initAuthorities` to allow accessing the
`*ratelimits.RedisSource` for checking that the new ratelimit functions
as intended.
Co-authored-by: @pgporada
Fixes https://github.com/letsencrypt/boulder/issues/7738
---------
Co-authored-by: Phil Porada <pporada@letsencrypt.org>
Co-authored-by: Phil Porada <philporada@gmail.com>
Introduce separate UpdateRegistrationContact & UpdateRegistrationKey
methods in RA & SA
Clear contact field during DeactivateRegistration
Part of #7716
Part of #5554
Clean up how we handle identifiers throughout the Boulder codebase by
- moving the Identifier protobuf message definition from sa.proto to
core.proto;
- adding support for IP identifier to the "identifier" package;
- renaming the "identifier" package's exported names to be clearer; and
- ensuring we use the identifier package's helper functions everywhere
we can.
This will make future work to actually respect identifier types (such as
in Authorization and Order protobuf messages) simpler and easier to
review.
Part of https://github.com/letsencrypt/boulder/issues/7311
When creating an authorization, populate it with all challenges
appropriate for that identifier, regardless of whether those challenge
types are currently "enabled" in the config. This ensures that
authorizations created during a incident for which we can temporarily
disabled a single challenge type can still be validated via that
challenge type after the incident is over.
Also, when finalizing an order, check that the challenge type used to
validation each authorization is not currently disabled. This ensures
that, if we temporarily disable a single challenge due to an incident,
we don't issue any more certificates using authorizations which were
fulfilled using that disabled challenge.
Note that standard rolling deployment of this change is not safe if any
challenges are disabled at the same time, due to the possibility of an
updated RA not filtering a challenge when writing it to the database,
and then a non-updated RA not filtering it when reading from the
database. But if all challenges are enabled then this change is safe for
normal deploy.
Fixes https://github.com/letsencrypt/boulder/issues/5913
Begin testing on go1.23. To facilitate this, also update /x/net,
golangci-lint, staticcheck, and pebble-challtestsrv to versions which
support go1.23. As a result of these updates, also fix a handful of new
lint findings, mostly regarding passing non-static (i.e. potentially
user-controlled) format strings into Sprintf-style functions.
Additionally, delete one VA unittest that was duplicating the checks
performed by a different VA unittest, but with a context timeout bug
that caused it to break when go1.23 subtly changed DialContext behavior.
- Add feature flag `UseKvLimitsForNewOrder`
- Add feature flag `UseKvLimitsForNewAccount`
- Flush all Redis shards before running integration or unit tests, this
avoids false positives between local testing runs
Fixes#7664
Blocked by #7676
- Check `CertificatesPerDomain` at newOrder and spend at Finalize time.
- Check `CertificatesPerAccountPerDomain` at newOrder and spend at
Finalize time.
- Check `CertificatesPerFQDNSet` at newOrder and spend at Finalize time.
- Fix a bug
in`FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction()` which
results in failed authorizations being spent for the exact FQDN, not the
eTLD+1.
- Remove redundant "max names" check at transaction construction time
- Enable key-value rate limits in the RA
Remove the id, identifierValue, status, and challenges fields from
sapb.NewAuthzRequest. These fields were left behind from the previous
corepb.Authorization request type, and are now being ignored by the SA.
Since the RA is no longer constructing full challenge objects to include
in the request, remove pa.ChallengesFor and replace it with the much
simpler pa.ChallengeTypesFor.
Part of https://github.com/letsencrypt/boulder/issues/5913
Find all gRPC fields which represent DNS Names -- sometimes called
"identifier", "hostname", "domain", "identifierValue", or other things
-- and unify their naming. This naming makes it very clear that these
values are strings which may be included in the SAN extension of a
certificate with type dnsName.
As we move towards issuing IP Address certificates, all of these fields
will need to be replaced by fields which carry both an identifier type
and value, not just a single name. This unified naming makes it very
clear which messages and methods need to be updated to support
non-dnsName identifiers.
Part of https://github.com/letsencrypt/boulder/issues/7647
Add a new "GetAuthorization" method to the RA. This method is very
similar to the SA's existing "GetAuthorization2" method, except that it
also uses the RA's built-in Policy Authority to filter out any
challenges which are currently disabled.
In a follow-up change, the WFE will be updated to use this method when
retrieving authorizations and challenges for display, so that we can
ensure disabled challenges are not presented to ACME clients.
Part of https://github.com/letsencrypt/boulder/issues/5913
This is a followup to https://github.com/letsencrypt/boulder/pull/7646,
updating two other RA methods (RevokeCertByApplicant and NewOrder) which
call different SA methods (GetValidAuthorizations2 and
GetAuthorizations2) but receive the same return type
(sapb.Authorizations) from the SA to use that type's new field.
Simplify SA.GetValidOrderAuthorizations2 so that it no longer conditions
the query on the status, expiry, or registration ID of the authorization
rows. This gives the query much better performance, because it no longer
tries to use an overly-large index, and fall back to large row-scans
when the query planner decides the index is too large.
While we're here, also improve the return type of
GetValidOrderAuthorizations2, so that instead of returning a map of
names to authorizations, it simply returns a list of authzs. This both
reduces the size of the gRPC message (once the old map is fully
removed), and improves its correctness because we cannot count on names
to be unique across multiple identifier types.
Finally, improve the RA code which calls SA.GetValidOrderAuthorizations2
to handle this improved return type, to make fewer assumptions about
identifier types, and to separate static authorization-checking from CAA
rechecking.
Fixes https://github.com/letsencrypt/boulder/issues/7645
Within the NewOrderAndAuthzsRequest, replace the corepb.Authorization
field with a new sapb.NewAuthzRequest message. This message has all of
the same field types and numbers, and the RA still populates all of
these fields when constructing a request, for backwards compatibility.
But it also has new fields (an Identifier carrying both type and value,
a list of challenge types, and a challenge token) which the RA
preferentially consumes if present.
This causes the content of our NewOrderAndAuthzsRequest to more closely
match the content that will be created at the database layer. Although
this may seem like a step backwards in terms of abstraction, it is also
a step forwards in terms of both efficiency (not having to transmit
multiple nearly-identical challenge objects) and correctness (being
guaranteed that the token is actually identical across all challenges).
After this change is deployed, it will be followed by a change which
removes the old fields from the NewAuthzRequest message, to realize the
efficiency gains.
Part of https://github.com/letsencrypt/boulder/issues/5913
Add three new keys to the CA's ProfileConfig:
- OmitKeyEncipherment causes the keyEncipherment Key Usage to be omitted
from certificates with RSA public keys. We currently include it for
backwards compatibility with TLS 1.1 servers that don't support modern
cipher suites, but this KU is completely useless as of TLS 1.3.
- OmitClientAuth causes the tlsClientAuthentication Extended Key Usage
to be omitted from all certificates. We currently include it to support
any subscribers who may be relying on it, but Root Programs are moving
towards single-purpose hierarchies and its inclusion is being
discouraged.
- OmitSKID causes the Subject Key Identifier extension to be omitted
from all certificates. We currently include this extension because it is
recommended by RFC 5280, but it serves little to no practical purpose
and consumes a large number of bytes, so it is now NOT RECOMMENDED by
the Baseline Requirements.
Make substantive changes to issuer.requestValid and issuer.Prepare to
implement the desired behavior for each of these options. Make a very
slight change to ra.matchesCSR to generally allow for serverAuth-only
EKUs. Improve the unit tests of both the //ca and //issuance packages to
cover the new behavior.
Part of https://github.com/letsencrypt/boulder/issues/7610
Have the RA's UnpauseAccount gRPC method forward the requested account
ID to the SA's corresponding method, and in turn forward the SA's count
of unpaused identifiers back to the caller in the response.
Changing the response message from emptypb.Empty to a new
rapb.UnpauseAccountResponse is safe, because message names are not
transmitted on the wire, only message field numbers.
While we're here, drastically simplify the wfe_test and sfe_test Mock
RAs, so they don't have to implement methods that aren't actually used
by the tests.
Fixes https://github.com/letsencrypt/boulder/issues/7536
The name "now" was always misleading, because we never set the value to
be the actual current time, we always set it to be some time in the
future to avoid returning authzs which expire in the very near future.
Changing the name to "validUntil" matches the current naming in
GetPendingAuthorizationRequest.
When receiving a NewOrder request from the WFE, pass the specified
profile name (if any) through to the SA for storage. Also, when
retrieving previous orders for potential re-use, don't reuse them unless
they have the same profile name (including the empty/default profile
name).
Fixes https://github.com/letsencrypt/boulder/issues/7607
This code path was a safety net to ensure that CAA got rechecked if the
authorization was going to expire less than 30d+7h from now, i.e. if the
authorization had originally been checked more than 7h ago. The metrics
show that, as expected, this code path has not been executed in living
memory, because all situations in which it might be hit instead hit the
preceding `if staleCAA` clause.
- Rename `NewOrderRequest` field `LimitsExempt` to `IsARIRenewal`
- Introduce a new `NewOrderRequest` field, `IsRenewal`
- Introduce a new (temporary) feature flag, `CheckRenewalExemptionAtWFE`
WFE:
- Perform renewal detection in the WFE when `CheckRenewalExemptionAtWFE`
is set
- Skip (key-value) `NewOrdersPerAccount` and `CertificatesPerDomain`
limit checks when renewal detection indicates the the order is a
renewal.
RA:
- Leave renewal detection in the RA intact
- Skip renewal detection and (legacy) `NewOrdersPerAccount` and
`CertificatesPerDomain` limit checks when `CheckRenewalExemptionAtWFE`
is set and the `NewOrderRequest` indicates that the order is a renewal.
Fixes#7508
Part of #5545
Add the `ra.UnpauseAccount` which takes an `rapb.UnpauseAccountRequest`
input parameter. The method is just a stub to allow downstream SFE
development to continue. There is relevant ongoing work in the SA which
will eventually reside in this stub method.
The core.Challenge.ProvidedKeyAuthorization field is problematic, both
because it is poorly named (which is admittedly easily fixable) and
because it is a field which we never expose to the client yet it is held
on a core type. Deprecate this field, and replace it with a new
vapb.PerformValidationRequest.ExpectedKeyAuthorization field.
Within the VA, this also simplifies the primary logic methods to just
take the expected key authorization, rather than taking a whole (largely
unnecessary) challenge object. This has large but wholly mechanical
knock-on effects on the unit tests.
While we're here, improve the documentation on core.Challenge itself,
and remove Challenge.URI, which was deprecated long ago and is wholly
unused.
Part of https://github.com/letsencrypt/boulder/issues/7514
Replaced our embeds of foopb.UnimplementedFooServer with
foopb.UnsafeFooServer. Per the grpc-go docs this reduces the "forwards
compatibility" of our implementations, but that is only a concern for
codebases that are implementing gRPC interfaces maintained by third
parties, and which want to be able to update those third-party
dependencies without updating their own implementations in lockstep.
Because we update our protos and our implementations simultaneously, we
can remove this safety net to replace runtime type checking with
compile-time type checking.
However, that replacement is not enough, because we never pass our
implementation objects to a function which asserts that they match a
specific interface. So this PR also replaces our reflect-based unittests
with idiomatic interface assertions. I do not view this as a perfect
solution, as it relies on people implementing new gRPC servers to add
this line, but it is no worse than the status quo which relied on people
adding the "TestImplementation" test.
Fixes https://github.com/letsencrypt/boulder/issues/7497
Replace "mocks.StorageAuthority" with "sapb.StorageAuthorityClient" in
our test mocks. The improves them by removing implementations of the
methods the tests don't actually need, instead of inheriting lots of
extraneous methods from the huge and cumbersome mocks.StorageAuthority.
This reduces our usage of mocks.StorageAuthority to only the WFE tests
(which create one in the frequently-used setup() function), which will
make refactoring those mocks in the pursuit of
https://github.com/letsencrypt/boulder/issues/7476 much easier.
Part of https://github.com/letsencrypt/boulder/issues/7476
Adds a new metric `cert_csr_mismatch` for SRE to create an alert for.
This should never happen, but if it does we should know about it as soon
as possible. The details of the failure will end up in logs due to error
propagation.
Fixes https://github.com/letsencrypt/boulder/issues/6587
Also update the CA and RA doccomments to link to it and describe the
roles of key functions a little better.
Remove outdated reference to generating OCSP at issuance time.
* Adds `CertProfileName` to the CAs `capb.IssuePrecertificateResponse`
so the RA can receive the CAs configured default profile name for audit
logging/metrics. This is useful for when the RA sends an empty string as
the profile name to the CA, but we want to know exactly what the profile
name chosen by the CA was, rather than just relying on comparing hashes
between CA and RA audit logs.
* Adds the profile name and hash to RA audit logs emitted after a
successful issuance.
* Adds new labels to the existing `new_certificates` metric exported by
the RA.
```
# HELP new_certificates A counter of new certificates including the certificate profile name and hexadecimal certificate profile hash
# TYPE new_certificates counter
new_certificates{profileHash="de4c8c8866ed46b1d4af0d79e6b7ecf2d1ea625e26adcbbd3979ececd8fbd05a",profileName="defaultBoulderCertificateProfile"} 2
```
Fixes https://github.com/letsencrypt/boulder/issues/7421
Upgrade from the old go-jose v2.6.1 to the newly minted go-jose v4.0.1.
Cleans up old code now that `jose.ParseSigned` can take a list of
supported signature algorithms.
Fixes https://github.com/letsencrypt/boulder/issues/7390
---------
Co-authored-by: Aaron Gable <aaron@letsencrypt.org>
When the order object retrieved from the SA contains a profile name,
propagate that into the request for the CA to issue a precertificate.
Similarly, when the CA's precertificate issuance response contains a
profile hash, propagate that into the request for the CA to issue the
corresponding final certificate.
Fixes https://github.com/letsencrypt/boulder/issues/7366
- Add new `replaces` field to RA.NewOrder requests
- Pass new `replaces` field to `SA.NewOrderAndAuthzs`
- Add new `limitsExempt` field to RA.NewOrder requests
- Ensure the RA follows this exemption for all NewOrder rate limits
The RA.AdministrativelyRevokeCertificate method has two primary modes of
operation: if a certificate DER blob is provided, it parses and extracts
information from that blob, and revokes the cert; if no DER is provided,
it assumes the cert is malformed, and revokes it (but doesn't do an OCSP
cache purge) based on the serial alone. However, this scheme has
slightly confusing semantics in the RA and requires that the admin
tooling look up the certificates to provide them to the RA.
Instead, add a new "malformed" field to the RA's
AdministrativelyRevokeCertificateRequest, and deprecate the "cert" field
of that same request. When the malformed boolean is false, the RA will
look up and parse the certificate itself. When the malformed field is
true, it will revoke the cert based on serial alone.
Note that the main logic of AdministrativelyRevokeCertificate -- namely
revoking, potentially re-revoking, doing an akamai cache purge, etc --
is not changed by this PR. The only thing that changes here is how the
RA gets access to the to-be-revoked certificate's information.
Part of https://github.com/letsencrypt/boulder/issues/7135
Previously, `va.IsCAAValid` would only check CAA records from the
primary VA during initial domain control validation, completely ignoring
any configured RVAs. The upcoming
[MPIC](https://github.com/ryancdickson/staging/pull/8) ballot will
require that it be done from multiple perspectives. With the currently
deployed [Multi-Perspective
Validation](https://letsencrypt.org/2020/02/19/multi-perspective-validation.html)
in staging and production, this change brings us in line with the
[proposed phase
3](https://github.com/ryancdickson/staging/pull/8/files#r1368708684).
This change reuses the existing
[MaxRemoteValidationFailures](21fc191273/cmd/boulder-va/main.go (L35))
variable for the required non-corroboration quorum.
> Phase 3: June 15, 2025 - December 14, 2025 ("CAs MUST implement MPIC
in blocking mode*"):
>
> MUST implement MPIC? Yes
> Required quorum?: Minimally, 2 remote perspectives must be used. If
using less than 6 remote perspectives, 1 non-corroboration is allowed.
If using 6 or more remote perspectives, 2 non-corroborations are
allowed.
> MUST block issuance if quorum is not met: Yes.
> Geographic diversity requirements?: Perspectives must be 500km from 1)
the primary perspective and 2) all other perspectives used in the
quorum.
>
> * Note: "Blocking Mode" is a nickname. As opposed to "monitoring mode"
(described in the last milestone), CAs MUST NOT issue a certificate if
quorum requirements are not met from this point forward.
Adds new VA feature flags:
* `EnforceMultiCAA` instructs a primary VA to command each of its
configured RVAs to perform a CAA recheck.
* `MultiCAAFullResults` causes the primary VA to block waiting for all
RVA CAA recheck results to arrive.
Renamed `va.logRemoteValidationDifferentials` to
`va.logRemoteDifferentials` because it can handle initial domain control
validations and CAA rechecking with minimal editing.
Part of https://github.com/letsencrypt/boulder/issues/7061
When a client is attempting to open a new Order which is identical to an
already-issued certificate, allow that request to bypass the normal New
Orders rate limit. This will allow renewals to go through even when a
client is exhibiting other bad behavior. This should not open the door
to floods of requests for the same certificate in rapid success, as the
Duplicate Certificates rate limit will still block those.
Fixes https://github.com/letsencrypt/boulder/issues/6792
Rename "IssuerNameID" to just "NameID". Similarly rename the standalone
functions which compute it to better describe their function. Add a
.NameID() directly to issuance.Issuer, so that callers in other packages
don't have to directly access the .Cert member of an Issuer. Finally,
rearrange the code in issuance.go to be sensibly grouped as concerning
NameIDs, Certificates, or Issuers, rather than all mixed up between the
three.
Fixes https://github.com/letsencrypt/boulder/issues/5152
The last rows using the old-style IssuerID were written to the database
in late 2021. Those rows have long since aged out -- we no longer serve
certificates or revocation information for them -- so we can remove the
code which handles those old-style IDs. This allows for some nice
simplifications in the CA's ocspImpl and in the Issuance package, which
will be useful for further reorganization of the CA and issuance
packages.
Fixes https://github.com/letsencrypt/boulder/issues/5152
Revamp WillingToIssueWildcards to WillingToIssue. Remove the need for
identifier.ACMEIdentifiers in the WillingToIssue(Wildcards) method.
Previously, before invoking this method, a slice of identifiers was
created by looping over each dnsName. However, these identifiers were
solely used in error messages.
Segment the validation process into distinct parts for domain
validation, wildcard validation, and exact blocklist checks. This
approach eliminates the necessity of substituting *. with x. in wildcard
domains.
Introduce a new helper, ValidDomain. It checks that a domain is valid
and that it doesn't contain any invalid wildcard characters.
Functionality from the previous ValidDomain is preserved in
ValidNonWildcardDomain.
Fixes#3323
If a client attempts to validate a challenge twice in rapid succession,
we'll kick off two background validation routines. One of these will
complete first, updating the database with success or failure. The other
will fail when it attempts to update the database and finds that there
are no longer any authorizations with that ID in the "pending" state.
Reduce the level at which we log such events, since we don't
particularly care about them.
Fixes https://github.com/letsencrypt/boulder/issues/3995
Replace the current three-piece setup (enum of feature variables, map of
feature vars to default values, and autogenerated bidirectional maps of
feature variables to and from strings) with a much simpler one-piece
setup: a single struct with one boolean-typed field per feature. This
preserves the overall structure of the package -- a single global
feature set protected by a mutex, and Set, Reset, and Enabled methods --
although the exact function signatures have all changed somewhat.
The executable config format remains the same, so no deployment changes
are necessary. This change does deprecate the AllowUnrecognizedFeatures
feature, as we cannot tell the json config parser to ignore unknown
field names, but that flag is set to False in all of our deployment
environments already.
Fixes https://github.com/letsencrypt/boulder/issues/6802
Fixes https://github.com/letsencrypt/boulder/issues/5229
- Move default and override limits, and associated methods, out of the
Limiter to new limitRegistry struct, embedded in a new public
TransactionBuilder.
- Export Transaction and add corresponding Transaction constructor
methods for each limit Name, making Limiter and TransactionBuilder the
API for interacting with the ratelimits package.
- Implement batched Spends and Refunds on the Limiter, the new methods
accept a slice of Transactions.
- Add new boolean fields check and spend to Transaction to support more
complicated cases that can arise in batches:
1. the InvalidAuthorizations limit is checked at New Order time in a
batch with many other limits, but should only be spent when an
Authorization is first considered invalid.
2. the CertificatesPerDomain limit is overridden by
CertficatesPerDomainPerAccount, when this is the case, spends of the
CertificatesPerDomain limit should be "best-effort" but NOT deny the
request if capacity is lacking.
- Modify the existing Spend/Refund methods to support
Transaction.check/spend and 0 cost Transactions.
- Make bucketId private and add a constructor for each bucket key format
supported by ratelimits.
- Move domainsForRateLimiting() from the ra.go to ratelimits. This
avoids a circular import issue in ra.go.
Part of #5545
This is a cleanup PR finishing the migration from int64 timestamps to
protobuf `*timestamppb.Timestamps` by removing all usage of the old
int64 fields. In the previous PR
https://github.com/letsencrypt/boulder/pull/7121 all fields were
switched to read from the protobuf timestamppb fields.
Adds a new case to `core.IsAnyNilOrZero` to check various properties of
a `*timestamppb.Timestamp` reducing the visual complexity for receivers.
Fixes https://github.com/letsencrypt/boulder/issues/7060
- Emit override utilization only when resource counts are under
threshold.
- Override utilization accounts for anticipated issuance.
- Correct the limit metric label for `CertificatesPerName` and
`CertificatesPerFQDNSet/Fast`.
Part of #5545
The CA, RA, and tools importing the PA (policy authority) will no longer
be able to live reload specific config files. Each location is now
responsible for loading the config file.
* Removed the reloader package
* Removed unused `ecdsa_allow_list_status` metric from the CA
* Removed mutex from all ratelimit `limitsImpl` methods
Fixes https://github.com/letsencrypt/boulder/issues/7111
---------
Co-authored-by: Samantha <hello@entropy.cat>
Co-authored-by: Aaron Gable <aaron@letsencrypt.org>
* Renames all of int64 as a time.Duration fields to `<fieldname>NS` to
indicate they are Unix nanoseconds.
* Adds new `google.protobuf.Duration` fields to each .proto file where
we previously had been using an int64 field to populate a time.Duration.
* Updates relevant gRPC messages.
Part 1 of 3 for https://github.com/letsencrypt/boulder/issues/7097
* Adds new `google.protobuf.Timestamp` fields to each .proto file where
we had been using `int64` fields as a timestamp.
* Updates relevant gRPC messages to populate the new
`google.protobuf.Timestamp` fields in addition to the old `int64`
timestamp fields.
* Added tests for each `<x>ToPB` and `PBto<x>` functions to ensure that
new fields passed into a gRPC message arrive as intended.
* Removed an unused error return from `PBToCert` and `PBToCertStatus`
and cleaned up each call site.
Built on-top of https://github.com/letsencrypt/boulder/pull/7069
Part 2 of 4 related to
https://github.com/letsencrypt/boulder/issues/7060
This is a partial revert of #7103. Because that PR introduces a new
Boulder error code that ocsp-responder needs to be specially aware of,
it has a deployability issue: ocsp-responder needs to be made aware of
the new error code before the RA can start emitting it.
The simple fix is to not emit that error code in the RA until the
ocsp-responder is ready to receive it.
In #6478, we stopped passing through Redis errors to the top-level
Responder object, preferring instead to live-sign. As part of that
change, we logged the Redis errors so they wouldn't disappear. However,
the sample rate for those errors was hard coded to 1-in-1000, instead of
following the LogSampleRate configured in the JSON.
This adds a field to redisSource for logSampleRate, and passes it
through from the JSON config in ocsp-responder/main.go.
Part of #7091
- Use constants for each rate limit name to ensure consistency when
labeling metrics
- Consistently check `.Enabled()` outside of each limit check RA method
- Replace the existing checks counter with a latency histogram
Part of #5545
This new standard library method returns a context with all of the
original metadata (e.g. tracing spans) still attached, but which will
not be canceled by any cancel funcs, deadlines, or timeouts set on the
parent context. We do this manually in a few places to prevent client
cancellations (usually disconnects) from disrupting our work, so this
just makes that code slightly simpler.
Fixes https://github.com/letsencrypt/boulder/issues/5506
Remove the remaining divergences from RFC8555 regarding what error types
we use in certain situations. Specifically:
- use "invalidContact" instead of "invalidEmail";
- use "unsupportedContact" for contact addresses that use a protocol
other than "mailto:"; and
- use "unsupportedIdentifier" for identifiers that specify a type other
than "dns".
This adds Jaeger's all-in-one dev container (with no persistent storage)
to boulder's dev docker-compose. It configures config-next/ to send all
traces there.
A new integration test creates an account and issues a cert, then
verifies the trace contains some set of expected spans.
This test found that async finalize broke spans, so I fixed that and a
few related spots where we make a new context.
In order to get rid of the orphan queue, we want to make sure that
before we sign a precertificate, we have enough data in the database
that we can fulfill our revocation-checking obligations even if storing
that precertificate in the database fails. That means:
- We should have a row in the certificateStatus table for the serial.
- But we should not serve "good" for that serial until we are positive
the precertificate was issued (BRs 4.9.10).
- We should have a record in the live DB of the proposed certificate's
public key, so the bad-key-revoker can mark it revoked.
- We should have a record in the live DB of the proposed certificate's
names, so it can be revoked if we are required to revoke based on names.
The SA.AddPrecertificate method already achieves these goals for
precertificates by writing to the various metadata tables. This PR
repurposes the SA.AddPrecertificate method to write "proposed
precertificates" instead.
We already create a linting certificate before the precertificate, and
that linting certificate is identical to the precertificate that will be
issued except for the private key used to sign it (and the AKID). So for
instance it contains the right pubkey and SANs, and the Issuer name is
the same as the Issuer name that will be used. So we'll use the linting
certificate as the "proposed precertificate" and store it to the DB,
along with appropriate metadata.
In the new code path, rather than writing "good" for the new
certificateStatus row, we write a new, fake OCSP status string "wait".
This will cause us to return internalServerError to OCSP requests for
that serial (but we won't get such requests because the serial has not
yet been published). After we finish precertificate issuance, we update
the status to "good" with SA.SetCertificateStatusReady.
Part of #6665
Overhaul tracking of order and authz age/ reuse to gather data for
#5061.
- Modify `ra.orderAges` histogram to track the method that observed the
order.
- Add observation of the order age at NewOrder time.
- Modify `ra.authzAges` histogram to track the method that observed the
authz as well its type (valid or pending).
- Add observation of authz age at Finalize time.
- Remove `reusedValidAuthzCounter`, which erroneously counted authzs
with status pending as valid.
Previously, we were computing the age of the authorization as "how much
time it has left, minus its total lifetime". In retrospect, this
universally results in negative values, which then all end up in the
zero bucket of the histogram. Fix the arithmetic so we can start getting
accurate data.
Enable the errcheck linter. Update the way we express exclusions to use
the new, non-deprecated, non-regex-based format. Fix all places where we
began accidentally violating errcheck while it was disabled.
Deprecate the ROCSPStage7 feature flag, which caused the RA and CA to
stop generating OCSP responses when issuing new certs and when revoking
certs. (That functionality is now handled just-in-time by the
ocsp-responder.) Delete the old OCSP-generating codepaths from the RA
and CA. Remove the CA's internal reference to an OCSP implementation,
because it no longer needs it.
Additionally, remove the SA's "Issuers" config field, which was never
used.
Fixes#6285
We used to check the CertificatesPerName "renewal exemption" after
checking to see if the rate limit was going to kick in at all. But
checking the rate limit is rather expensive, so #4174 introduced a
feature flag and a new block of code so that we'd check the renewal
exemption first, and short-circuit out of the whole function if it was
met. But when #4771 deprecated the feature flag, it left both blocks of
code in, instead of deleting the old location.
Remove the redundant exemption check.
* Perform Akamai OCSP cache purges on already revoked certificates in
`ra.RevokeCertByKey` and `ra.AdministrativelyRevokeCertificate` for
subsequent client calls to `/acme/revoke-cert`.
* Add `addToBlockedKeys` helper function used by
`ra.AdministrativelyRevokeCertificate` and `ra.RevokeCertByKey` to
de-duplicate code adding subject public key info hash to the
`blockedKeys` table.
* Add a duplicate revocation test to
`ra.TestAdministrativelyRevokeCertificate`
* Fix some grammar in an RA comment.
* Fixes https://github.com/letsencrypt/boulder/issues/4862
The RA's matchesCSR function checks to make sure that various aspects of
the final issued certificate match the CSR provided by the client. One
of these checks is that the CommonName matches the CN specified in the
CSR, or matches a specific one of the SANs if no CN was specified in the
CSR. However, if the logic to select which of the SANs gets promoted
into the CN ever differs between the RA and the CA (for example, in the
midst of an incremental deploy that updates the version of one service
before the other) this causes conflicts and results in Finalize requests
failing with 500s.
Relax the check, so that the RA simply guarantees that the final
certificate's CN matches any of the names in the CSR, not a specific one
of the names. This fixes a deployability bug introduced in #6706.
Part of #5112
Remove tracing using Beeline from Boulder. The only remnant left behind
is the deprecated configuration, to ensure deployability.
We had previously planned to swap in OpenTelemetry in a single PR, but
that adds significant churn in a single change, so we're doing this as
multiple steps that will each be significantly easier to reason about
and review.
Part of #6361
Add a new feature flag, `SetCommonName`, which defaults to `true`. In
this default state, no behavior changes.
When set to `false` on the CA, this flag will cause the CA to leave the
Subject commonName field of the certificate blank, as is recommended by
the Baseline Requirements Section 7.1.4.2.2(a).
Also slightly modify the behavior of the RA's `matchesCSR()` function,
to allow for both certificates that have a CN and certificates that
don't. It is not feasible to put this behavior behind the same
SetCommonName flag, because that would require an atomic deploy of both
the RA and the CA.
Obsoletes #5112
In particular, document that it does not purge the Akamai cache.
Also, in the RA, avoid creating a "fake" certificate object containing
only the serial. Instead, use req.Serial directly in most places. This
uncovered some incorrect logic. Fix that logic by gating the operations
that actually need a full *x509.Certificate: revoking by key, and
purging the Akamai cache.
Also, make `req.Serial` mandatory for AdministrativelyRevokeCertificate.
This is a reopen of #6693, which accidentally got merged into a
different feature branch.
Removes all code related to the `ReuseValidAuthz` feature flag. The
Boulder default is to now always reuse valid authorizations.
Fixes a panic in `test.AssertErrorIs` when `err` is unexpectedly `nil`
that was found this while reworking the
`TestPerformValidationAlreadyValid` test. The go stdlib `func Is`[1]
does not check for this.
1. https://go.dev/src/errors/wrap.go
Part 2/2, fixes https://github.com/letsencrypt/boulder/issues/2734
Add the "AsyncFinalize" feature flag. When enabled, this causes the RA
to return almost immediately from FinalizeOrder requests, with the
actual hard work of issuing the precertificate, getting SCTs, issuing
the final certificate, and updating the database accordingly all
occuring in a background goroutine while the client polls the GetOrder
endpoint waiting for the result.
This is implemented by factoring out the majority of the finalization
work into a new `issueCertificateOuter` helper function, and simply
using the new flag to determine whether we call that helper in a
goroutine or not. This makes removing the feature flag in the future
trivially easy.
Also add a new prometheus metric named `inflight_finalizes` which can be
used to count the number of simultaneous goroutines which are performing
finalization work. This metric is exported regardless of the state of
the AsyncFinalize flag, so that we can observe any changes to this
metric when the flag is flipped.
Fixes#6575
- Log counts of Authzs where CAA was rechecked/reused.
- Move the CAA recheck duration to a single variable in the RA.
- Add new method `InfoObject` to our logger.
Fixes#6560
Part of #6623
`ReuseValidAuthz` was introduced
here [1] and enabled in staging and production configs on 2016-07-13.
There was a brief stint during the TLS-SNI-01 challenge type removal where
SRE disabled it. However, time has finally come to remove this configuration
option. Issue #6623 will determine the feasibility of shorter authz
lifetimes and potentially the removal of authz reuse.
This change is broken up into two parts to allow SRE to safely remove
the flag from staging and production configs. We'll merge this PR, SRE
will deploy boulder and the config change, then we'll finish removing
`ReuseValidAuthz` configuration from the codebase.
[1] boulder commit 9abc212448
Part 1 of 2 for fixing #2734.
For consistency, put the error field at the end of unstructured log
lines to make them more ... structured.
Adds the `issuerID` field to "orphaning certificate" log line in the CA
to match the "orphaning precertificate" log line.
Fixes broken tests as a result of the CA and bdns log line change.
Fixes#5457
Simplify the control flow of the FinalizeOrder handler to make it easier
to read and reason about:
- Move all validation to before we set the order to Processing, and put
it all in a single helper funcion.
- Move almost all logEvent/trace handling directly into FinalizeOrder so
it cannot be missed.
- Flatten issueCertificate and issueCertificateInner into a single
helper function, now that they're no longer being called from both
ACMEv1 and v2 entry points.
- Other minor cleanups, such as making SolvedBy not return a pointer and
making matchesCSR private.
This paves the way for making both issueCertificateInner and failOrder
asynchronous, which we plan to do in the near future.
Part of #6575
The core.CertificateRequest wrapper type only had two fields: an
x509.CertificateRequest, and a slice of bytes representing the unparsed
version of the same request. However, x509.CertificateRequest carries
around its own field .Raw, which serves the same purpose. Also, we
weren't even referencing the .Bytes field anyway. Therefore, get rid of
this redundant wrapper type.
This also makes it clearer just how far the original CSR makes it
through our system. I'd like a future change to remove the CSR from the
request to the CA service, replacing it with a structured object that
only exposes exactly the fields of the CSR we care about, such as names,
public key, and whether or not the must-staple extension should be set.
Although we have a metric for counting how often we reuse a valid
authorization, no code has been incrementing that metric since 2021.
Re-enable it.
Additionally, that metric doesn't help us know how old authorizations
are when they get reused, which is useful information when deciding
whether or how much to shrink our authorization reuse lifetime. Add a
new histogram metric to track the ages of authorizations when they're
attached to an order.
Aaron Gable
Jacob Hoffman-Andrews
Samantha Frank
Kruti Sutaria
James Renken
Phil Porada
forcedebug
Matthew McPherrin
Matthew McPherrin