Commit Graph

48 Commits

Author SHA1 Message Date
Aaron Gable 1d713ed8eb
Ignore IP CNs in CSRs (#8231)
If a finalize CSR contains a SAN which looks like an IP address, don't
actually include that CN in our IssuanceRequest, and don't promote any
other SAN to be the CN either. This is similar to how we ignore the
CSR's CN when it is too long.
2025-06-06 14:57:12 -07:00
James Renken 103ffb03d0
wfe, csr: Add IP address identifier support & integration test (#8187)
Permit all valid identifier types in `wfe.NewOrder` and `csr.VerifyCSR`.

Permit certs with just IP address identifiers to skip
`sa.addIssuedNames`.

Check that URI SANs are empty in `csr.VerifyCSR`, which was previously
missed.

Use a real (Let's Encrypt) IP address range in integration testing, to
let challtestsrv satisfy IP address challenges.

Fixes #8192
Depends on #8154
2025-05-27 13:17:47 -07:00
James Renken 3f879ed0b4
Add Identifiers to Authorization & Order structs (#7961)
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
2025-03-26 10:30:24 -07:00
James Renken cb94164b54
policy: Add initial Identifier support (#8064)
Change WillingToIssue and WellFormedDomainNames to use Identifiers, and
(for now) reject non-DNS identifiers.

Part of #7311
2025-03-14 11:34:59 -07:00
James Renken edc3c7fa6d
Shorten "identifier(s)" in variable names & function arguments (#8066)
For consistency, and to prevent confusion with the `identifier` package,
use "ident(s)" instead.

Part of #7311
2025-03-14 10:59:38 -07:00
James Renken 412e959063
Allow CSRs whose CN is longer than acceptable (#7700)
Also rework comments & test names for clarity, add tests for this new CN
handling, and change/remove tests that should indeed no longer fail.

Fixes https://github.com/letsencrypt/boulder/issues/7623
2024-09-10 14:05:32 -04:00
Aaron Gable d58d09615a
Improve how we disable challenge types (#7677)
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
2024-08-29 15:38:50 -07:00
Aaron Gable e1790a5a02
Remove deprecated sapb.NewAuthzRequest fields (#7651)
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
2024-08-15 15:35:10 -07:00
Aaron Gable 8545ea8364
KeyPolicy: add custom constructor and make all fields private (#7543)
Change how goodkey.KeyPolicy keeps track of allowed RSA and ECDSA key
sizes, to make it slightly more flexible while still retaining the very
locked-down allowlist of only 6 acceptable key sizes (RSA 2048, 3076,
and 4092, and ECDSA P256, P384, and P521). Add a new constructor which
takes in a collection of allowed key sizes, so that users of the goodkey
package can customize which keys they accept. Rename the existing
constructor to make it clear that it uses hardcoded default values.

With these new constructors available, make all of the goodkey.KeyPolicy
member fields private, so that a KeyPolicy can only be built via these
constructors.
2024-06-18 17:52:50 -04:00
Aaron Gable 78e4e82ffa
Feature cleanup (#7320)
Remove three deprecated feature flags which have been removed from all
production configs:
- StoreLintingCertificateInsteadOfPrecertificate
- LeaseCRLShards
- AllowUnrecognizedFeatures

Deprecate three flags which are set to true in all production configs:
- CAAAfterValidation
- AllowNoCommonName
- SHA256SubjectKeyIdentifier

IN-9879 tracked the removal of these flags.
2024-02-13 17:42:27 -08:00
Samantha d281702c17
PA: Improve wildcard exact blocklist implementation (#7218)
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
2023-12-19 14:22:18 -05:00
Aaron Gable 82c1763824
Test NamesFromCSR with duplicate SANs (#6826)
Add tests to NamesFromCSR which test additional aspects of its behavior:
merging the CSR's CN and SANs into a single list of SANs, and removing
duplicate SANs.
2023-04-18 09:09:42 -07:00
Aaron Gable 276978ac4a
Lowercase CN in all code paths (#6824)
When returning a CN from csr.NamesFromCSR, ensure that we call
strings.ToLower on that name in all return paths. This prevents us from
running into lint failures (and therefore refusing to issue) when an
applicant submits a CSR containing uppercase SANs and no explicit CN.
2023-04-17 14:56:33 -04:00
Aaron Gable 6d6f3632da
Change SetCommonName to RequireCommonName (#6749)
Change the SetCommonName flag, introduced in #6706, to
RequireCommonName. Rather than having the flag control both whether or
not a name is hoisted from the SANs into the CN *and* whether or not the
CA is willing to issue certs with no CN, this updated flag now only
controls the latter. By default, the new flag is true, and continues our
current behavior of failing issuance if we cannot set a CN in the cert.
When the flag is set to false, then we are willing to issue certificates
for which the CSR contains no CN and there is no SAN short enough to be
hoisted into the CN field.

When we have rolled out this change, we can move on to the next flag in
this series: HoistCommonName, which will control whether or not a SAN is
hoisted at all, effectively giving the CSRs (and therefore the clients)
full control over whether their certificate contains a SAN.

This change is safe because no environment explicitly sets the
SetCommonName flag to false yet.

Fixes #5112
2023-03-21 11:07:06 -07:00
Aaron Gable 9af4871e59
Add SetCommonName feature flag (#6706)
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
2023-03-09 13:31:55 -05:00
Aaron Gable 1b7eb3d978
RA: Simplify FinalizeOrder flow (#6588)
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
2023-01-25 17:59:54 -08:00
Aaron Gable 4f473edfa8
Deprecate 10 feature flags (#6502)
Deprecate these feature flags, which are consistently set in both prod
and staging and which we do not expect to change the value of ever
again:
- AllowReRevocation
- AllowV1Registration
- CheckFailedAuthorizationsFirst
- FasterNewOrdersRateLimit
- GetAuthzReadOnly
- GetAuthzUseIndex
- MozRevocationReasons
- RejectDuplicateCSRExtensions
- RestrictRSAKeySizes
- SHA1CSRs

Move each feature flag to the "deprecated" section of features.go.
Remove all references to these feature flags from Boulder application
code, and make the code they were guarding the only path. Deduplicate
tests which were testing both the feature-enabled and feature-disabled
code paths. Remove the flags from all config-next JSON configs (but
leave them in config ones until they're fully deleted, not just
deprecated). Finally, replace a few testdata CSRs used in CA tests,
because they had SHA1WithRSAEncryption signatures that are now rejected.

Fixes #5171 
Fixes #6476
Part of #5997
2022-11-14 09:24:50 -08:00
Aaron Gable bd608e3b53
Make pa.WillingToIssue private (#6304)
By making this method private, we ensure that all policy
decisions go through WillingToIssueWildcards, which
attaches the failing identifier to each error message.

Fixes #2526
Part of #5816
2022-08-19 08:12:01 -07:00
Jacob Hoffman-Andrews cf9df961ba
Add feature flags for upcoming deprecations (#6043)
This adds three features flags: SHA1CSRs, OldTLSOutbound, and
OldTLSInbound. Each controls the behavior of an upcoming deprecation
(except OldTLSInbound, which isn't yet scheduled for a deprecation
but will be soon). Note that these feature flags take advantage of
`features`' default values, so they can default to "true" (that is, each
of these features is enabled by default), and we set them to "false"
in the config JSON to turn them off when the time comes.

The unittest for OldTLSOutbound requires that `example.com` resolves
to 127.0.0.1. This is because there's logic in the VA that checks
that redirected-to hosts end in an IANA TLD. The unittest relies on
redirecting, and we can't use e.g. `localhost` in it because of that
TLD check, so we use example.com.

Fixes #6036 and #6037
2022-04-15 12:14:00 -07:00
Aaron Gable 8da52f74c3
Remove v1 `NewCertificate` code path from WFE and RA (#5842)
The NewCertificate codepath was the ACME v1 API's equivalent of
the modern Finalize endpoint. Remove the bodies of the WFE's and
the RA's `NewCertificate` functions. Remove the functions which were
called only from those functions. One of the removed functions is the
old `checkAuthorizations`, so update some tests which were calling
that directly to instead use different entry points.

Part of #5681
2021-12-13 17:45:45 -08:00
Aaron Gable e2c8f6743a
Introduce new core.AcmeChallenge type (#5012)
ACME Challenges are well-known strings ("http-01", "dns-01", and
"tlsalpn-01") identifying which kind of challenge should be used
to verify control of a domain. Because they are well-known and
only certain values are valid, it is better to represent them as
something more akin to an enum than as bare strings. This also
improves our ability to ensure that an AcmeChallenge is not
accidentally used as some other kind of string in a different
context. This change also brings them closer in line with the
existing core.AcmeResource and core.OCSPStatus string enums.

Fixes #5009
2020-08-11 15:02:16 -07:00
Roland Bracewell Shoemaker 7853b12cb3
Remove support for issuing certificates with no CN (#5008)
We'd like to issue certs with no CN eventually, but it's not
going to happen any time soon. In the mean time, the existing
code never gets exercised and is rather complex, so this
removes it.
2020-08-05 09:15:30 -07:00
Jacob Hoffman-Andrews 6f4966cc0f
Check email address validity in notify-mailer. (#4841)
This required a refactoring: Move validateEmail from the RA to ValidEmail
in the `policy` package. I also moved `ValidDomain` from a method on
PolicyAuthority to a standalone function so that ValidEmail can call it.

notify-mailer will now log invalid addresses and skip them without
attempting to send mail. Since @example.com addresses are invalid,
I updated the notify-mailer test, which used a lot of such addresses.

Also, now when notify-mailer receives an unrecoverable error sending
mail, it logs the email address and what offset within the list it was.
2020-06-04 18:28:02 -07:00
Roland Bracewell Shoemaker 9df97cbf06
Add a blocked keys table, and use it (#4773)
Fixes #4712 and fixes #4711.
2020-04-15 13:42:51 -07:00
Roland Bracewell Shoemaker 324d92d7c5
goodkey cleanups (#4754)
Fixes #4748 and fixes #4740.
2020-04-08 17:57:23 -07:00
Daniel McCarney fde145ab96
RA: implement stricter email validation. (#4574)
Prev. we weren't checking the domain portion of an email contact address
very strictly in the RA. This updates the PA to export a function that
can be used to validate the domain the same way we validate domain
portions of DNS type identifiers for issuance.

This also changes the RA to use the `invalidEmail` error type in more
places.

A new Go integration test is added that checks these errors end-to-end
for both account creation and account update.
2019-11-22 13:39:31 -05:00
Daniel McCarney ecca3492e9 csr: return berrors in VerifyCSR. (#4473)
This also adds the badCSR error type specified by RFC 8555. It is a natural fit for the errors in VerifyCSR that aren't covered by badPublicKey. The web package function for converting a berror to
a problem is updated for the new badCSR error type.

The callers (RA and CA) are updated to return the berrors from VerifyCSR as is instead of unconditionally wrapping them as a berrors.MalformedError instance. Unit/integration tests are updated accordingly.

Resolves #4418
2019-10-09 17:11:11 -07:00
Roland Bracewell Shoemaker 1c8cfb3cba Fix SAN to CN promotion (#4298)
Before #4275 if a CSR only contained SANs longer than the max CN limit
it would set the CN to one anyway and would cause the 'CN too long'
check to get triggered. After #4275 if all of the SANs were too long
the CN wouldn't get set and we didn't have a check for `forceCNFromSAN
&& cn == ""` which would allow empty CNs despite `forceCNFromSAN`
being set. This adds that check and a test for the corner case.
2019-06-26 15:34:47 -07:00
Daniel McCarney 4fbb90b2d1
csr: be more flexible promoting a SAN to Subj. CN. (#4275)
When configured to force promote a SAN to the Subject CN while
normalizing a CSR without a CN we should try to find a SAN that is <=
maxCNLength.
2019-06-20 14:18:37 -04:00
Daniel McCarney 7dd176e9a4 Implement suberrors for policy blocked names. (#4234)
When validating a CSR's identifiers, or a new order's identifiers there may be more than one identifier that is blocked by policy. We should return an error that has suberrors identifying each bad identifier individually in this case.

Updates https://github.com/letsencrypt/boulder/issues/4193
Resolves https://github.com/letsencrypt/boulder/issues/3727
2019-05-31 15:00:17 -07: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
Jacob Hoffman-Andrews 935df44851 Move "Combinations" support to WFE1. (#4155)
Early ACME drafts supported a notion of "combinations" of challenges
that had to be completed together. This was removed from subsequent
drafts. Boulder has only ever supported "combinations" that exactly map
to the list of challenges, 1 for 1.

This removes all the plumbing for combinations, and adds a list of
combinations to the authz JSON right before marshaling it in WFE1.
2019-04-16 11:31:51 -07:00
Jacob Hoffman-Andrews d1e6d0f190 Remove TLS-SNI-01 (#4114)
* Remove the challenge whitelist
* Reduce the signature for ChallengesFor and ChallengeTypeEnabled
* Some unit tests in the VA were changed from testing TLS-SNI to testing the same behavior
  in TLS-ALPN, when that behavior wasn't already tested. For instance timeouts during connect 
  are now tested.

Fixes #4109
2019-03-15 09:05:24 -04:00
Roland Bracewell Shoemaker a9a0846ee9
Remove checks for deployed features (#3881)
Removes the checks for a handful of deployed feature flags in preparation for removing the flags entirely. Also moves all of the currently deprecated flags to a separate section of the flags list so they can be more easily removed once purged from production configs.

Fixes #3880.
2018-10-17 20:29:18 -07:00
Jacob Hoffman-Andrews 8153b919be
Implement TLSSNIRevalidation (#3361)
This change adds a feature flag, TLSSNIRevalidation. When it is enabled, Boulder
will create new authorization objects with TLS-SNI challenges if the requesting
account has issued a certificate with the relevant domain name, and was the most
recent account to do so*. This setting overrides the configured list of
challenges in the PolicyAuthority, so even if TLS-SNI is disabled in general, it
will be enabled for revalidation.

Note that this interacts with EnforceChallengeDisable. Because
EnforceChallengeDisable causes additional checked at validation time and at
issuance time, we need to update those two places as well. We'll send a
follow-up PR with that.

*We chose to make this work only for the most recent account to issue, even if
there were overlapping certificates, because it significantly simplifies the
database access patterns and should work for 95+% of cases.

Note that this change will let an account revalidate and reissue for a domain
even if the previous issuance on that account used http-01 or dns-01. This also
simplifies implementation, and fits within the intent of the mitigation plan: If
someone previously issued for a domain using http-01, we have high confidence
that they are actually the owner, and they are not going to "steal" the domain
from themselves using tls-sni-01.

Also note: This change also doesn't work properly with ReusePendingAuthz: true.
Specifically, if you attempted issuance in the last couple days and failed
because there was no tls-sni challenge, you'll still have an http-01 challenge
lying around, and we'll reuse that; then your client will fail due to lack of
tls-sni challenge again.

This change was joint work between @rolandshoemaker and @jsha.
2018-01-12 11:00:06 -08:00
Maciej Dębski 44984cd84a Implement regID whitelist for allowed challenge types. (#3352)
This updates the PA component to allow authorization challenge types that are globally disabled if the account ID owning the authorization is on a configured whitelist for that challenge type.
2018-01-10 13:44:53 -05:00
Roland Shoemaker 400ffede3d More fixes 2018-01-09 20:48:16 -08:00
Roland Shoemaker dcd2b438f4 Fix previous impl, add valid authz reuse fix and existing authz validation fix 2018-01-09 19:53:48 -08:00
Daniel McCarney 1c99f91733 Policy based issuance for wildcard identifiers (Round two) (#3252)
This PR implements issuance for wildcard names in the V2 order flow. By policy, pending authorizations for wildcard names only receive a DNS-01 challenge for the base domain. We do not re-use authorizations for the base domain that do not come from a previous wildcard issuance (e.g. a normal authorization for example.com turned valid by way of a DNS-01 challenge will not be reused for a *.example.com order).

The wildcard prefix is stripped off of the authorization identifier value in two places:

When presenting the authorization to the user - ACME forbids having a wildcard character in an authorization identifier.
When performing validation - We validate the base domain name without the *. prefix.
This PR is largely a rewrite/extension of #3231. Instead of using a pseudo-challenge-type (DNS-01-Wildcard) to indicate an authorization & identifier correspond to the base name of a wildcard order name we instead allow the identifier to take the wildcard order name with the *. prefix.
2017-12-04 12:18:10 -08:00
Andriy 72330bbedd Fix `TestNormalizeCSR` test condition (#3245)
Previous to this commit `TestNormalizeCSR` was comparing the expected DNSNames in the CSR to themselves, **not** the names in the CSR.
2017-11-17 08:30:09 -05:00
Jacob Hoffman-Andrews 404e9682b1 Improve error messages. (#2256)
Quote rejected hostnames.
Include term "global" when rejecting based on global rate limit.

Fixes #2252
2016-10-18 10:15:21 -07:00
Jacob Hoffman-Andrews 96fc0b264d Reject IPAddresses and EmailAddresses earlier. (#2213)
Previously, if we received a CSR with IPAddress or EmailAddress SANs, we would
ignore those fields, issuing only for the DNSNames in the CSR. However, we would
later check in MatchesCSR that the CSR's IPAddresses and EmailAddresses matches
those in the issued certificate. This check would fail, serving a 500 to the end
user.

Instead, we now reject the CSR earlier in the process, and send a
meaningful error message.

Fixes #2203
2016-10-03 10:22:44 -07:00
Roland Bracewell Shoemaker 91bfd05127 Revert #2088 (#2137)
* Remove oldx509 usage

* Un-vendor old crypto/x509, crypto/x509/pkix, and encoding/asn1
2016-08-23 14:01:37 -04:00
Jacob Hoffman-Andrews 474b76ad95 Import forked x509 for parsing of CSRs with empty integers (#2088)
Part of #2080.

This change vendors `crypto/x509`, `crypto/x509/pkix`, and `encoding/asn1` from  1d5f6a765d. That commit is a direct child of the Go 1.5.4 release tag, so it contains the same code as the current Go version we are using. In that commit I rewrote imports in those packages so they depend on each other internally rather than calling out to the standard library, which would cause type disagreements.

I changed the imports in each place where we're parsing CSRs, and imported under a different name `oldx509`, both to avoid collisions and make it clear what's going on. Places that only use `x509` to parse certificates are not changed, and will use the current standard library.

This will unblock us from moving to Go 1.6, and subsequently Go 1.7.
2016-07-28 10:38:33 -04:00
Ben Irving d3db851403 remove regID from WillingToIssue (#1957)
The `regID` parameter in the PA's `WillingToIssue` function was originally used for whitelisting purposes, but is not used any longer. This PR removes it.
2016-06-22 12:21:07 -04:00
Jacob Hoffman-Andrews 4e0f96d924 Remove last vestiges of challenge.AccountKey. (#1949)
This is a followup from https://github.com/letsencrypt/boulder/pull/1942. That PR stopped setting challenge.AccountKey. This one removes it entirely.

Fixes #1948
2016-06-21 16:25:58 -07:00
Jacob Hoffman-Andrews 9a4b979397 Move goodkey and nonce out of core (#1869) 2016-06-02 11:29:58 -07:00
Roland Bracewell Shoemaker 5abe7e3cdf Move CSR normalization/verification to their own methods (#1826)
* Split CSR testing and name hoisting into own functions, verify CSR in RA & CA

* Move tests around and various other fixes

* 1.5.3 doesn't have the needed stringer

* Move functions to their own lib

* Remove unused imports

* Move MaxCNLength and BadSignatureAlgorithms to csr package

* Always normalizeCSR in VerifyCSR and de-export it

* Update comments
2016-05-26 14:17:41 -07:00