Commit Graph

27 Commits

Author SHA1 Message Date
James Renken dea81c7381
bdns, va: Remove DNSAllowLoopbackAddresses (#8203)
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
2025-05-28 10:08:03 -07:00
James Renken ac68828f43
Replace most uses of net.IP with netip.Addr (#8205)
Retain `net.IP` only where we directly work with `x509.Certificate` and
friends.

Fixes #5925
Depends on #8196
2025-05-27 15:05:35 -07:00
Samantha Frank 7a3feb2ceb
va/rva: Validate user-agent for http-01 and DoH requests (#8114)
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.
2025-04-15 16:31:08 -04:00
Aaron Gable 2c28c4799c
ProblemDetails no longer implements Error (#8078)
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
2025-03-28 13:36:26 -05:00
James Renken 3e6a8e2d25
va: Support IP address identifiers (#8020)
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
2025-03-06 11:39:22 -08:00
Samantha Frank e6c812a3db
va/ra: Deprecate EnforceMultiCAA and EnforceMPIC (#8025)
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 #7965
Fixes #7819
2025-03-03 16:33:27 -05:00
Samantha Frank dda8acc34a
RA/VA: Add MPIC compliant DCV and CAA checks (#7870)
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 #7612
Fixes #7614
Fixes #7615
Fixes #7616
2024-12-10 11:26:08 -05:00
Samantha Frank 27a77142ad
VA: Make performRemoteValidation more generic (#7847)
- 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
2024-11-27 15:29:33 -05:00
Jacob Hoffman-Andrews 577a1e38eb
va: prepare to require minimum of 3 RVAs (#7815)
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.
2024-11-19 10:23:32 -05:00
Jacob Hoffman-Andrews a46c388f66
va: compute maxRemoteFailures based on MPIC (#7810)
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.
2024-11-18 15:36:09 -08:00
Samantha Frank 3baac6f6df
VA: Consolidate multiple metrics into one histogram (#7816)
- 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
2024-11-15 15:51:39 -05:00
Aaron Gable 5b647072b5
Perform remote validation after primary validation (#7522)
Change the VA to perform remote validation wholly after local validation
and CAA checks, and to do so only if those local checks pass. This will
likely increase the latency of our successful validations, by making
them less parallel. However, it will reduce the amount of work we do on
unsuccessful validations, and reduce their latency, by not kicking off
and waiting for remote results.

Fixes https://github.com/letsencrypt/boulder/issues/7509
2024-06-10 14:16:44 -07:00
Aaron Gable 09693f03dc
Deprecate Challenge.ProvidedKeyAuthorization (#7515)
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
2024-06-04 14:48:36 -07:00
Jacob Hoffman-Andrews 3865b46638
va: return error instead of ProblemDetails (#7313)
This allows us to defer creating the user-friendly ProblemDetails to the
highest level (va.PerformValidation), which in turn makes it possible to
log the original error alongside the user-friendly error. It also
reduces the likelihood of "boxed nil" bugs.

Many of the unittests check for a specific ProblemDetails.Type and
specific Details contents. These test against the output of
`detailedError`, which transforms `error` into `ProblemDetails`. So the
updates to the tests include insertion of `detailedError(err)` in many
places.

Several places that were returning a specific ProblemDetails.Type
instead return the corresponding `berrors` type. This follows a pattern
that `berrors` was designed to enable: use the `berrors` types
internally and transform into `ProblemDetails` at the edge where we are
rendering something to present to the user: WFE, and now VA.
2024-02-12 11:34:49 -08:00
Phil Porada 03152aadc6
RVA: Recheck CAA records (#7221)
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
2024-01-25 16:23:25 -05:00
Jacob Hoffman-Andrews c21b376623
Implement DoH for validation queries (#7178)
Fixes: #7141
2023-12-11 10:49:00 -08:00
Samantha 6eee230d69
BDNS: Ensure DNS server addresses are dialable (#5520)
- Add function `validateServerAddress()` to `bdns/servers.go` which ensures that
  DNS server addresses are TCP/ UDP dial-able per: https://golang.org/src/net/dial.go?#L281
- Add unit test for `validateServerAddress()` in `bdns/servers_test.go`
- Update `cmd/boulder-va/main.go` to handle `bdns.NewStaticProvider()`
  potentially returning an error.
- Update unit tests in `bdns/dns_test.go`:
  - Handle `bdns.NewStaticProvider()` potentially returning an error
  - Add an IPv6 address to `TestRotateServerOnErr`
- Ensure DNS server addresses are validated by `validateServerAddress` whenever:
  - `dynamicProvider.update() is called`
  - `staticProvider` is constructed
- Construct server addresses using `net.JoinHostPost()` when
  `dynamicProvider.Addrs()` is called

Fixes #5463
2021-07-20 10:11:11 -07:00
Aaron Gable a19ebfa0e9
VA: Query SRV to preload/cache DNS resolver addrs (#5360)
Abstract out the way that the bdns library keeps track of the
resolvers it uses to do DNS lookups. Create one implementation,
the `StaticProvider`, which behaves exactly the same as the old
mechanism (providing whatever names or addresses were given
in the config). Create another implementation, `DynamicProvider`,
which re-resolves the provided name on a regular basis.

The dynamic provider consumes a single name, does a lookup
on that name for any SRV records suggesting that it is running a
DNS service, and then looks up A records to get the address of
all the names returned by the SRV query. It exports its successes
and failures as a prometheus metric.

Finally, update the tests and config-next configs to work with
this new mechanism. Give sd-test-srv the capability to respond
to SRV queries, and put the names it provides into docker's
default DNS resolver.

Fixes #5306
2021-04-20 10:11:53 -07:00
Aaron Gable ef1d3c4cde
Standardize on `AssertMetricWithLabelsEquals` (#5371)
Update all of our tests to use `AssertMetricWithLabelsEquals`
instead of combinations of the older `CountFoo` helpers with
simple asserts. This coalesces all of our prometheus inspection
logic into a single function, allowing the deletion of four separate
helper functions.
2021-04-01 15:20:43 -07:00
Jacob Hoffman-Andrews 2a8f0fe6ac
Rename several items in bdns (#5260)
[Go style says](https://blog.golang.org/package-names):

> Avoid stutter. Since client code uses the package name as a prefix
> when referring to the package contents, the names for those contents
> need not repeat the package name. The HTTP server provided by the
> http package is called Server, not HTTPServer. Client code refers to
> this type as http.Server, so there is no ambiguity.

Rename DNSClient, DNSClientImpl, NewDNSClientImpl,
NewTestDNSClientImpl, DNSError, and MockDNSClient to follow those
guidelines.

Unexport DNSClientImpl and MockTimeoutError (was only used internally).

Make New and NewTest return the Client interface rather than a concrete
`impl` type.
2021-01-29 17:20:35 -08:00
Jacob Hoffman-Andrews bf7c80792d
core: move to proto3 (#5063)
Builds on #5062
Part of #5050
2020-08-31 17:58:32 -07:00
Aaron Gable 0f5d2064a8
Remove logic from VA PerformValidation wrapper (#5003)
Updates the type of the ValidationAuthority's PerformValidation
method to be identical to that of the corresponding auto-generated
grpc method, i.e. directly taking and returning proto message
types, rather than exploded arguments.

This allows all logic to be removed from the VA wrappers, which
will allow them to be fully removed after the migration to proto3.

Also updates all tests and VA clients to adopt the new interface.

Depends on #4983 (do not review first four commits)
Part of #4956
2020-08-06 10:45:35 -07:00
Jacob Hoffman-Andrews 75024c3ec1
Replace clock.Default() with clock.New() (#4761)
clock.Default is deprecated:
https://godoc.org/github.com/jmhodges/clock#Default
2020-04-08 17:23:43 -07:00
Roland Bracewell Shoemaker 5b2f11e07e Switch away from old style statsd metrics wrappers (#4606)
In a handful of places I've nuked old stats which are not used in any alerts or dashboards as they either duplicate other stats or don't provide much insight/have never actually been used. If we feel like we need them again in the future it's trivial to add them back.

There aren't many dashboards that rely on old statsd style metrics, but a few will need to be updated when this change is deployed. There are also a few cases where prometheus labels have been changed from camel to snake case, dashboards that use these will also need to be updated. As far as I can tell no alerts are impacted by this change.

Fixes #4591.
2019-12-18 11:08:25 -05:00
Daniel McCarney 6ed4ce23a8
bdns: move logDNSError to exchangeOne, log ErrId specially. (#4553)
We've found we need the context offered from logging the error closer to when it
happens in the `bdns` package rather than in the `va`. Adopting the function
requires adapting it slightly. Specifically in the new location we know it won't
be called with any timeout results, with a non-dns error, or with a nil
underlying error.

Having the logging done in `bdns` (and specifically from `exchangeOne`) also
lets us log the wire format of the query and response when we get a `dns.ErrId`
error indicating a query/response ID mismatch. A small unit test is included
that ensures the logging happens as expected.

In case it proves useful for matching against other metrics the DNS ID mismatch
error case also now increments a dedicated prometheus counter vector stat,
`dns_id_mismatch`. The stat is labelled by resolver and query type.

Resolves https://github.com/letsencrypt/boulder/issues/4532
2019-11-15 16:03:45 -05:00
Daniel McCarney ea9871de1e core: split identifier types into separate package. (#4225)
This will allow implementing sub-problems without creating a cyclic
dependency between `core` and `problems`.

The `identifier` package is somewhat small/single-purpose and in the
future we may want to move more "ACME" bits beyond the `identifier`
types into a dedicated package outside of `core`.
2019-05-23 13:24:41 -07:00
Daniel McCarney cc0d15841f va: split up va.go by challenge type. (#4170)
When I introduced the new HTTP-01 code I did it in `va/http.go` intending to try and make the very large `va.go` file a little bit smaller. This is the continuation of that work.

* f96ad92 - moves remaining HTTP-01 specific code to `va/http.go`.
* 1efb9a1 - moves TLS-ALPN-01 code into `va/tlsalpn.go`.
* 95ea567 - moves DNS-01 code into `va/dns.go`.
* 6ff0395 - moves unit tests from `va/va_test.go` into `va/http_test.go`, `va/tlsalpn_test.go` and `va/dns_test.go`.

In the end `va/va.go` contains code related to metrics, top level RPCs (e.g. `PerformValidation`), and the multi-VA code. This makes the file lengths much more manageable overall.

Note: There is certainly room for cleaning up some of the older unit test cruft from `va/va_test.go`. For now I only moved it as-is into the challenge specific test files.
2019-04-19 11:34:58 -07:00