Remove `ToDNSSlice`, `FromProtoWithDefault`, and
`FromProtoSliceWithDefault` now that all their callers are gone. All
protobufs but one have migrated from DnsNames to Identifiers.
Remove TODOs for the exception, `ValidationRecord`, where an identifier
type isn't appropriate and it really only needs a string.
Rename `corepb.ValidationRecord.DnsName` to `Hostname` for clarity, to
match the corresponding PB's field name.
Improve various comments and docs re: IP address identifiers.
Depends on #8221 (which removes the last callers)
Fixes#8023
This partially reverts https://github.com/letsencrypt/boulder/pull/8203,
which was landed as commit dea81c7381.
It leaves all of the boulder integration test environment changes in
place, while restoring the DNSAllowLoopbackAddresses config key and its
ability to influence the VA's behavior.
We no longer need a code path to resolve reserved IP addresses during
integration tests.
Move to a public IP for the remaining tests, after #8187 did so for many
of them.
Depends on #8187
Move `IsReservedIP` and its supporting vars from `bdns` to `policy`.
Rewrite `IsReservedIP` to:
* Use `netip` because `netip.Prefix` can be used as a map key, allowing
us to define prefix lists more elegantly. This will enable future work
to import prefix lists from IANA's primary source data.
* Return an error including the reserved network's name.
Refactor `IsReservedIP` tests to be table-based.
Fixes#8040
In #8153, we started using identifiers in `vapb.IsCAAValidRequest`, and
added logic at the top of `va.DoCAA` to populate the `ident` variable
from the deprecated `Domain` value, in order to accommodate clients that
don't yet populate the `Identifier`.
Unfortunately, we didn't use the `ident` variable throughout the entire
function. Two places refer directly to `req.Identifier` and can't handle
it being nil.
Fixes#8189
In `vapb.IsCAAValidRequest`, even though CAA is only for DNS names,
deprecate `Domain` in favour of `Identifier` for consistency.
In `va.DoCAA`, reject attempts to validate CAA for non-DNS identifiers.
Rename `identifier` to `ident` inside some VA functions, also for
consistency.
In `ra.checkDCVAndCAA` & `ra.checkAuthorizationsCAA`, bypass CAA checks
for IP address identifiers.
Part of #7995
In `ra.NewOrder`, improve safety of authz reuse logic by making it
explicit that only DNS identifiers might be wildcards. Also, now that
the conditional statements need to be more complicated, collapse them
for brevity.
In `vapb.PerformValidationRequest`, remove `DnsName`.
In `ra.PerformValidation`, pass an `Identifier` instead of a `DnsName`.
In `ra.RevokeCertByApplicant`, check that the requester controls
identifiers of all types (not just DNS).
Fixes#7995 (the RA now fully supports IP address identifiers, except
for rate limits)
Fixes#7647
Part of #8023
https://github.com/letsencrypt/boulder/pull/8150 updated our runtime
protobuf dependency from v1.34.1 to v1.36.5. This change does the same
for our build-time dependency, to keep them in sync.
Plumb the userAgent field, used to set http-01 User-Agent headers, from
va/rva configuration through to where User-Agent headers can be set for
DoH queries. Use integration tests to validate that the User-Agent is
set for http-01 challenges, dns-01 challenges over DoH, and CAA checks
over DoH.
Fixes#7963.
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
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
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
Add a new "MPICFullResults" feature flag. When this flag is enabled in
the VA, it will wait for all Remote VAs to return their results for both
Domain Control Validation and CAA checking, rather than short-circuiting
as soon as it has seen enough results to know whether corroboration will
or will not be achieved.
We make this change because waiting for these to return honestly doesn't
take that long, because we do validation (although not CAA rechecking)
asynchronously, and because it improves the quality of our MPIC quorum
summary logs (so we don't always say only 3/4 concurred because the
fourth was cancelled).
Fixes https://github.com/letsencrypt/boulder/issues/7809
Previously this test was passing not because the VA was returning early,
but because the fake HTTP server was only sleeping for 1000 nanoseconds
instead of 1000 milliseconds. The test cases were not exercising the
VA's early-return codepath, because they do not include sufficiently
high ratios of passing or failing remotes to hit quorum early.
Fix the sleep time so the fake HTTP server works as expected, and reduce
the (desired) sleep time from 1000ms to 100ms because that's more than
sufficient for the behavior we're testing.
Fix and diversify the test cases to actually hit positive or negative
quorum, so that the VA's early-return codepath is actually exercised.
This PR will be followed by a non-test PR which removes this
early-return codepath and modifies this test further, but I thought it
was important to have this test in fully working order before modifying
the code it tests.
Part of https://github.com/letsencrypt/boulder/issues/7809
Add an `identifier` field to the `va.PerformValidationRequest` proto, which will soon replace its `dnsName` field.
Accept and prefer the `identifier` field in every VA function that uses this struct. Don't (yet) assume it will be present.
Throughout the VA, accept and handle the IP address identifier type. Handling is similar to DNS names, except that `getAddrs` is not called, and consider that:
- IPs are represented in a different field in the `x509.Certificate` struct.
- IPs must be presented as reverse DNS (`.arpa`) names in SNI for [TLS-ALPN-01 challenge requests](https://datatracker.ietf.org/doc/html/rfc8738#name-tls-with-application-layer-).
- IPv6 addresses are enclosed in square brackets when composing or parsing URLs.
For HTTP-01 challenges, accept redirects to bare IP addresses, which were previously rejected.
Fixes#2706
Part of #7311
When checking CAA, issuance is allowed if the relevant RRSet (as defined
in RFC 8659, Section 3) does not contain any records of the right
Property kind (issue or issuewild) for the kind of checking being
attempted. Previously, we correctly detected that a non-wildcard
issuance attempt could short-circuit our validation logic if no issue
records are present. However, we did not do a similar short-circuit for
wildcard issuance attempts when no issue records and no issuewild
records are present.
Add a test which demonstrates that a nearly-empty RRSet accidentally
forbade issuance of wildcard certs. Update our logic to perform the "no
relevant records" check slightly later, so that it catches both the
wildcard and non-wildcard cases, causing the new test to pass.
Fixes https://github.com/letsencrypt/boulder/issues/8032
Fix a remaining edge case after #7468: one call to `newIPError` did not
account for when we retry *successfully,* but then are served a redirect
which errors. In those cases, our `client.Do` call results in our
redirect handler `processRedirect` appending yet another validation
record to `records`, which was missed.
Fixes#7347
Replace DCV and CAA checks (PerformValidation and IsCAAValid) in
va/va.go and va/caa.go with their MPIC compliant counterparts (DoDCV and
DoCAA) in va/vampic.go. Deprecate EnforceMultiCAA and EnforceMPIC and
default code paths as though they are both true. Require that RIR and
Perspective be set for primary and remote VAs.
Fixes#7965Fixes#7819
Update from go1.23.1 to go1.23.6 for our primary CI and release builds.
This brings in a few security fixes that aren't directly relevant to us.
Add go1.24.0 to our matrix of CI and release versions, to prepare for
switching to this next major version in prod.
Today, we have VA.PerformValidation, a method called by the RA at
challenge time to perform DCV and check CAA. We also have VA.IsCAAValid,
a method invoked by the RA at finalize time when a CAA re-check is
necessary. Both of these methods can be executed on remote VA
perspectives by calling the generic VA.performRemoteValidation.
This change splits VA.PerformValidation into VA.DoDCV and VA.DoCAA,
which are both called on remote VA perspectives by calling the generic
VA.doRemoteOperation. VA.DoDCV, VA.DoCAA, and VA.doRemoteOperation
fulfill the requirements of SC-067 V3: Require Multi-Perspective
Issuance Corroboration by:
- Requiring at least three distinct perspectives, as outlined in the
"Phased Implementation Timeline" in BRs section 3.2.2.9 ("Effective
March 15, 2025").
- Ensuring that the number of non-corroborating (failing) perspectives
remains below the threshold defined by the "Table: Quorum Requirements"
in BRs section 3.2.2.9.
- Ensuring that corroborating (passing) perspectives reside in at least
2 distinct Regional Internet Registries (RIRs) per the "Phased
Implementation Timeline" in BRs section 3.2.2.9 ("Effective March 15,
2026").
- Including an MPIC summary consisting of: passing perspectives, failing
perspectives, passing RIRs, and a quorum met for issuance (e.g., 2/3 or
3/3) in each validation audit log event, per BRs Section 5.4.1,
Requirement 2.8.
When the new SeparateDCVAndCAAChecks feature flag is enabled on the RA,
calls to VA.IsCAAValid (during finalization) and VA.PerformValidation
(during challenge) are replaced with calls to VA.DoCAA and a sequence of
VA.DoDCV followed by VA.DoCAA, respectively.
Fixes#7612Fixes#7614Fixes#7615Fixes#7616
- Ensure the Perspective and RIR reported by each remoteVA in the
*vapb.ValidationResult returned by VA.PerformValidation, matches the
expected local configuration when that configuration is present.
- Correct "AfriNIC" to "AFRINIC", everywhere.
Part of https://github.com/letsencrypt/boulder/issues/7819
- Remove undeployed feature flag MultiCAAFullResults
- Perform local CAA checks prior to initiating remote checks, instead of
starting remote checks and proceeding to perform local checks.
- Remove VA.IsCAAValid specific remote validation logic, use
VA.performRemoteOperation instead
- Refactor va.logRemoteResults to be easier to test and omit the RVA
problem
- Drive-by fix: Calculate logEvent.Latency with va.clk.Since() instead
of time.Since() like everything else in VA.performRemoteOperation
- 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
- Make the primary VA aware of the expected Perspective and RIR of each
remote VA.
- All Perspectives should be unique, have the primary VA check for
duplicate Perspectives at startup.
- Update test setup functions to ensure that each remote VA client and
corresponding inmem impl have a matching perspective and RIR.
Part of #7819
- 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.
Fix two bugs introduced in #7816:
- Fix localLatency time for CAA rechecking is always 0
- Fix logEvent.InternalError is no longer being written to log
To prepare for the MPIC requirement of having a minimum of 3
perspectives, I added code to `NewValidationAuthorityImpl` to error if
there aren't enough remote VAs configured _and_ the current VA is the
primary perspective. Then I fixed all the tests, which involved adding
some backends in the unittests, and spinning up `remoteva-c` in the
integration tests.
As a reminder, the `boulder va` command always considers itself the
primary perspective, while `boulder remoteva` gives itself a perspective
based on its config.
I wound up backing out the code in `NewValidationAuthorityImpl` because
right now our remote VAs are actually running the `boulder va` command,
so they would error out in prod, even though our actual primary
perspective does have enough backends. So this wound up as a test-only
change.
Previously this was a configuration field.
Ports `maxAllowedFailures()` from `determineMaxAllowedFailures()` in
#7794.
Test updates:
Remove the `maxRemoteFailures` param from `setup` in all VA tests.
Some tests were depending on setting this param directly to provoke
failures.
For example, `TestMultiVAEarlyReturn` previously relied on "zero allowed
failures". Since the number of allowed failures is now 1 for the number
of remote VAs we were testing (2), the VA wasn't returning early with an
error; it was succeeding! To fix that, make sure there are two failures.
Since two failures from two RVAs wouldn't exercise the right situation,
add a third RVA, so we get two failures from three RVAs.
Similarly, TestMultiCAARechecking had several test cases that omitted
this field, effectively setting it to zero allowed failures. I updated
the "1 RVA failure" test case to expect overall success and added a "2
RVA failures" test case to expect overall failure (we previously
expected overall failure from a single RVA failing).
In TestMultiVA I had to change a test for `len(lines) != 1` to
`len(lines) == 0`, because with more backends we were now logging more
errors, and finding e.g. `len(lines)` to be 2.
- Add a new histogram, validationLatency
- Add a VA.observeLatency for observing validation latency
- Refactor to ensure this metric can be observed exclusively within
VA.PerformValidation and VA.IsCAAValid.
- Replace validationTime, localValidationTime, remoteValidationTime,
remoteValidationFailures, caaCheckTime, localCAACheckTime,
remoteCAACheckTime, and remoteCAACheckFailures with validationLatency
- Add `Perspective` and `RIR` fields to the remote-va configuration
- Configure RVA ValidationAuthorityImpl instances with the contents of
the JSON configuration
- Configure VA ValidationAuthorityImpl instances with the constant
`va.PrimaryPerspective`
- Log `Perspective` for non-Primary Perspectives, per the MPIC
requirements in section 5.4.1 (2) vii of the BRs. Also log the RIR for
posterity.
- Introduce `ValidationResult` RPC fields `Perspective` and `Rir`, which
are not currently used but will be required for corroboration in #7616
Fixes https://github.com/letsencrypt/boulder/issues/7613
Part of https://github.com/letsencrypt/boulder/issues/7615
Part of https://github.com/letsencrypt/boulder/issues/7616
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
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.
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
Replace all of Boulder's usage of the Go stdlib "math/rand" package with
the newer "math/rand/v2" package which first became available in go1.22.
This package has an improved API and faster performance across the
board.
See https://go.dev/blog/randv2 and https://go.dev/blog/chacha8rand for
details.
Prevent misconfigured DNS CAA entries from passing CAA checks.
Previously, boulder would take the last `accounturi` and
`validationmethods` in the event there were multiple, cause by DNS
misconfiguration. An example of what that looks like is below.
```
example.com CAA 0 issue "example.net; accounturi=https://example.net/acme/acct/123; accounturi=https://example.net/acme/acct/456;"
example.com CAA 0 issue "example.net; validationmethods=dns-01,http-01; validationmethods=tls-alpn-01;"
```
[RFC 8567 Section
3](https://www.rfc-editor.org/rfc/rfc8657#name-extensions-to-the-caa-recor)
states the following. The RFC does not explicitly state how to handle
multiple `validationmethods`, but I chose to be consistent with
`accounturi` and reject multiple parameter tags there too.
> A Property without an "accounturi" parameter matches any account. A
Property with an invalid or unrecognized "accounturi" parameter is
unsatisfiable. A Property with multiple "accounturi" parameters is
unsatisfiable.
James Renken
Aaron Gable
Jacob Hoffman-Andrews
Samantha Frank
Phil Porada