Commit Graph

34 Commits

Author SHA1 Message Date
Aaron Gable 5e1bc3b501
Simplify the features package (#7204)
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
2023-12-12 15:51:57 -05:00
Phil Porada 3366be50f1
Use RFC 7093 truncated SHA256 hash for Subject Key Identifier (#7179)
- Adds a feature flag to gate rollout for SHA256 Subject Key Identifiers
for end-entity certificates.
- The ceremony tool will now use the RFC 7093 section 2 option 1 method
for generating Subject Key Identifiers for future root CA, intermediate
CA, and cross-sign ceremonies.

- - - -

[RFC 7093 section 2 option
1](https://datatracker.ietf.org/doc/html/rfc7093#section-2) provides a
method for generating a truncated SHA256 hash for the Subject Key
Identifier field in accordance with Baseline Requirement [section
7.1.2.11.4 Subject Key
Identifier](90a98dc7c1/docs/BR.md (712114-subject-key-identifier)).

> [RFC5280] specifies two examples for generating key identifiers from
>    public keys.  Four additional mechanisms are as follows:
> 
>    1) The keyIdentifier is composed of the leftmost 160-bits of the
>       SHA-256 hash of the value of the BIT STRING subjectPublicKey
>       (excluding the tag, length, and number of unused bits).

The related [RFC 5280 section
4.2.1.2](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.2)
states:
>   For CA certificates, subject key identifiers SHOULD be derived from
>   the public key or a method that generates unique values.  Two common
>   methods for generating key identifiers from the public key are:
>   ...
>   Other methods of generating unique numbers are also acceptable.
2023-12-06 13:44:17 -05:00
Aaron Gable 124d2eb5d3
Rely on crypto/x509 to set AuthorityKeyID (#7181)
The go crypto/x509 library correctly sets the AuthorityKeyID from the
issuer's SubjectKeyID, overriding any value provided in the template:

https://pkg.go.dev/crypto/x509@go1.21.4#CreateCertificate:
> The AuthorityKeyId will be taken from the SubjectKeyId of parent, if
any, unless the resulting certificate is self-signed. Otherwise the
value from template will be used.

https://cs.opensource.google/go/go/+/refs/tags/go1.21.4:src/crypto/x509/x509.go;l=1584-1587;drc=82c713feb05da594567631972082af2fcba0ee4f

We shouldn't attempt to second-guess this functionality by setting the
AKID ourselves.
2023-12-04 16:35:10 -08:00
Aaron Gable e59f59c21b
Split issuance.go into issuer.go and cert.go (#7167)
This will make it easier to add a crl.go, holding functionality similar
to cert.go, without making any single file overly complex.

This introduces no functionality changes.

Part of https://github.com/letsencrypt/boulder/issues/7159
2023-11-28 11:34:04 -08:00
Aaron Gable 2bf5b26397
Remove ability to configure policy OIDs (#6992)
Completely remove the ability to configure Certificate Policy OIDs in
the Boulder CA. Instead, hard-code the Baseline Requirements Domain
Validated Reserved Policy Identifier. Boulder will never perform OV or
EV validation, so this is the only identifier that will be necessary.

In the ceremony tool, introduce additional checks that assert that Root
certificates do not have policies, and Intermediate certificates have
exactly the one Baseline Requirements Domain Validated Reserved Policy
Identifier.
2023-07-19 10:38:59 -04:00
Aaron Gable 158f62bd0c
Remove policy qualifiers from all issuance paths (#6980)
The inclusion of Policy Qualifiers inside Policy Information elements of
a Certificate Policies extension is now NOT RECOMMENDED by the Baseline
Requirements. We have already removed these fields from all of our
Boulder configuration, and ceased issuing certificates with Policy
Qualifiers.

Remove all support for configuring and including Policy Qualifiers in
our certificates, both in Boulder's main issuance path and in our
ceremony tool. Switch from using the policyasn1 library to manually
encode these extensions, to using the crypto/x509's
Certificate.PolicyIdentifiers field. Delete the policyasn1 package as it
is no longer necessary.

Fixes https://github.com/letsencrypt/boulder/issues/6880
2023-07-13 10:37:05 -07:00
Jacob Hoffman-Andrews f6a005bc25
ca: check correspondence between precertificate and final linting certificate (#6953)
This introduces a small new package, `precert`, with one function
`Correspond` that checks a precertificate against a final certificate to
see if they correspond in the relationship described in RFC 6962.

This also modifies the `issuance` package so that RequestFromPrecert
generates an IssuanceRequest that keeps a reference to the
precertificate's bytes. The allows `issuance.Prepare` to do a
correspondence check when preparing to sign the final certificate. Note
in particular that the correspondence check is done against the
_linting_ version of the final certificate. This allows us to catch
correspondence problems before the real, trusted signature is actually
made.

Fixes #6945
2023-06-26 15:35:06 -07:00
Samantha 43bb293d6f
CA: Only increment lintErrorCount for true lint violations (#6843)
Return the sentinel error indicative of lint violation from
`linter.ProcessResultSet()` instead of `issuance`. This removes a
potential source of false-positives.
2023-04-24 14:30:50 -07:00
Samantha 914e971f15
CA: Emit count of precertificate lint failures (#6839)
Fixes #6825
2023-04-21 17:00:27 -04:00
Jacob Hoffman-Andrews b4bdd035ad
issuance: split linting and issuing (#6788)
It's useful to be able to get a copy of the linting certificate out of
the process, so we can store it in the database for use by processes
that want a certificate-shaped object (for instance, scanning
possibly-issued public keys for a newly discovered weakness in key
generation).

To do this, while still ensuring that it's impossible to issue a
certificate without linting it, return an IssuanceToken from the linting
process. The IssuanceToken has a private field containing the template
that was used for linting. To issue a final certificate, the
IssuanceToken has to be redeemed, and only the template stored in it can
be used.
2023-04-06 13:24:19 -07:00
Samantha b2224eb4bc
config: Add validation tags to all configuration structs (#6674)
- Require `letsencrypt/validator` package.
- Add a framework for registering configuration structs and any custom
validators for each Boulder component at `init()` time.
- Add a `validate` subcommand which allows you to pass a `-component`
name and `-config` file path.
- Expose validation via exported utility functions
`cmd.LookupConfigValidator()`, `cmd.ValidateJSONConfig()` and
`cmd.ValidateYAMLConfig()`.
- Add unit test which validates all registered component configuration
structs against test configuration files.

Part of #6052
2023-03-21 14:08:03 -04: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
Matthew McPherrin 391a59921b
Move cmd.ConfigDuration to config.Duration (#6705)
We rely on the ratelimit/ package in CI to validate our ratelimit
configurations. However, because that package relies on cmd/ just for
cmd.ConfigDuration, many additional dependencies get pulled in.

This refactors just that struct to a separate config package. This was
done using Goland's automatic refactoring tooling, which also organized
a few imports while it was touching them, keeping standard library,
internal and external dependencies grouped.
2023-02-28 08:11:49 -08:00
Aaron Gable 0340b574d9
Add unparam linter to CI (#6312)
Enable the "unparam" linter, which checks for unused function
parameters, unused function return values, and parameters and
return values that always have the same value every time they
are used.

In addition, fix many instances where the unparam linter complains
about our existing codebase. Remove error return values from a
number of functions that never return an error, remove or use
context and test parameters that were previously unused, and
simplify a number of (mostly test-only) functions that always take the
same value for their parameter. Most notably, remove the ability to
customize the RSA Public Exponent from the ceremony tooling,
since it should always be 65537 anyway.

Fixes #6104
2022-08-23 12:37:24 -07:00
Aaron Gable 9c197e1f43
Use io and os instead of deprecated ioutil (#6286)
The iotuil package has been deprecated since go1.16; the various
functions it provided now exist in the os and io packages. Replace all
instances of ioutil with either io or os, as appropriate.
2022-08-10 13:30:17 -07:00
Aaron Gable 733bcec941
Standardize on 'shardIdx' to identify crl shards (#6263)
Realized that "ShardID" is a bad name, because a real unique
identifier of a shard would include the issuer, crl number, and
shard number. Switching to "ShardIdx" makes it clearer that
shards within a full and complete CRL are identified by a
zero-indexed integer.
2022-08-02 13:21:26 -07:00
Aaron Gable 11544756bb
Support new Google CT Policy (#6082)
Add a new code path to the ctpolicy package which enforces Chrome's new
CT Policy, which requires that SCTs come from logs run by two different
operators, rather than one Google and one non-Google log. To achieve
this, invert the "race" logic: rather than assuming we always have two
groups, and racing the logs within each group against each other, we now
race the various groups against each other, and pick just one arbitrary
log from each group to attempt submission to.

Ensure that the new code path does the right thing by adding a new zlint
which checks that the two SCTs embedded in a certificate come from logs
run by different operators. To support this lint, which needs to have a
canonical mapping from logs to their operators, import the Chrome CT Log
List JSON Schema and autogenerate Go structs from it so that we can
parse a real CT Log List. Also add flags to all services which run these
lints (the CA and cert-checker) to let them load a CT Log List from disk
and provide it to the lint.

Finally, since we now have the ability to load a CT Log List file
anyway, use this capability to simplify configuration of the RA. Rather
than listing all of the details for each log we're willing to submit to,
simply list the names (technically, Descriptions) of each log, and look
up the rest of the details from the log list file.

To support this change, SRE will need to deploy log list files (the real
Chrome log list for prod, and a custom log list for staging) and then
update the configuration of the RA, CA, and cert-checker. Once that
transition is complete, the deletion TODOs left behind by this change
will be able to be completed, removing the old RA configuration and old
ctpolicy race logic.

Part of #5938
2022-05-25 15:14:57 -07:00
Jacob Hoffman-Andrews e0605aadab
issuance: improve error message when load chain fails (#6087) 2022-05-05 15:41:07 -07:00
Aaron Gable 0a22f83c0b
Major OCSP refactor (#5863)
Completely refactor the way we organize our code related to OCSP.

- Move it all into one `//ocsp/` package, rather than having multiple
  top-level packages.
- Merge the OCSP updater's config sub-package with its parent
  (since it isn't necessary to break it out to avoid cyclic imports).
- Remove all `Source` logic from ocsp-responder's `main.go`, because
  it was difficult to mentally trace the control flow there.
- Replace that logic with a set of composable `Source`s in the
  `//ocsp/responder/` package, each of which is good at just one thing.
- Update the way the filters work to make sure that the request's
  `IssuerKeyHash` and the response's `ResponderName` can both
  be derived from the same issuer certificate, ensuring that the req and
  resp are correctly matched.
- Split the metrics into a separate metric for each `Source`, so we can
  tell what all of them are doing, not just aggregate behavior.
- Split the tests into individual files for each `Source`, and update them
  for the new public interfaces.
2022-02-04 12:01:34 -08:00
Aaron Gable 305ef9cce9
Improve error checking paradigm (#5920)
We have decided that we don't like the if err := call(); err != nil
syntax, because it creates confusing scopes, but we have not cleaned up
all existing instances of that syntax. However, we have now found a
case where that syntax enables a bug: It caused readers to believe that
a later err = call() statement was assigning to an already-declared err
in the local scope, when in fact it was assigning to an
already-declared err in the parent scope of a closure. This caused our
ineffassign and staticcheck linters to be unable to analyze the
lifetime of the err variable, and so they did not complain when we
never checked the actual value of that error.

This change standardizes on the two-line error checking syntax
everywhere, so that we can more easily ensure that our linters are
correctly analyzing all error assignments.
2022-02-01 14:42:43 -07:00
Samantha 83a7220f4e
admin-revoker: Block and revoke by private key (#5878)
Incidents of key compromise where proof is supplied in the form of a private key
have historically been labor intensive for SRE. This PR seeks to automate the
process of embedded public key validation , query for issuance, revocation, and
blocking by SPKI hash.

For an example of private keys embedding a mismatched public key, see:
https://blog.hboeck.de/archives/888-How-I-tricked-Symantec-with-a-Fake-Private-Key.html.

Adds two new sub-commands (private-key-block and private-key-revoke) and one new
flag (-dry-run) to admin-revoker. Both new sub-commands validate that the
provided private key and provide the operator with an issuance count. Any
blocking and revocation actions are gated by the new '-dry-run' flag, which is
'true' by default.

private-key-block: if -dry-run=false, will immediately block issuance for the
provided key. The operator is informed that bad-key-revoker will eventually
revoke any certificates using the provided key.

private-key-revoke: if -dry-run=false, will revoke all certificates using the
provided key and then blocks future issuance. This avoids a race with the
bad-key-revoker. This command will execute successfully even if issuance for the
provided key is already blocked.

- Add support for blocking issuance by private key to admin-revoker
- Add support for revoking certificates by private key to admin-revoker
- Create new package called 'privatekey'
- Move private key loading logic from 'issuance' to 'privatekey'
- Add embedded public key verification to 'privatekey'
- Add new field `skipBlockKey` to `AdministrativelyRevokeCertificate` protobuf
- Add check in RA to ensure that only KeyCompromise revocations use
  `skipBlockKey`

Fixes #5785
2022-01-21 10:29:12 -08:00
Aaron Gable ab79f96d7b
Fixup staticcheck and stylecheck, and violations thereof (#5897)
Add `stylecheck` to our list of lints, since it got separated out from
`staticcheck`. Fix the way we configure both to be clearer and not
rely on regexes.

Additionally fix a number of easy-to-change `staticcheck` and
`stylecheck` violations, allowing us to reduce our number of ignored
checks.

Part of #5681
2022-01-20 16:22:30 -08:00
Aaron Gable 99035226d8
Add name and key hashes to issuance.Certificate (#5812)
These hashes are useful for OCSP computations, as they are the two
values that are used to uniquely identify the issuer of the given cert in
an OCSP request. Here, they are restricted to SHA1 only, as Boulder
only supports SHA1 for OCSP, as per RFC 5019.

In addition, because the `ID`, `NameID`, `NameHash`, and `KeyHash`
are relatively expensive to compute, introduce a new constructor for
`issuance.Certificate` that computes all four values at startup time and
then simply returns the precomputed values when asked.
2021-12-01 12:11:10 -08:00
Jacob Hoffman-Andrews 25ef9c3bfc
Shorten maximum serial length. (#5694)
Fixes #5690
2021-10-14 16:49:27 -07:00
Aaron Gable 3492a996ab
Update TODOs for Issue #5152 (#5591)
Changing how we're going to finally handle #5152: rather
than changing everything to use IssuerNameIDs, we're going
to change the meaning of IssuerID. This will allow us to avoid
renaming database columns and protobuf message fields.
2021-08-19 14:31:09 -07:00
Aaron Gable d405f9e616
Refactor lint library for go1.17 support (#5513)
In go1.17, the `x509.CreateCertificate()` method fails if the provided
Signer (private key) and Parent (cert with public key) do not match.
This change both updates the lint library to create and use an issuer
cert whose public key matches the throwaway private key used for lint
signatures, and overhauls its public interface for readability and
simplicity.

Rename the `lint` library to `linter`, to allow other methods to be
renamed to reduce word repetition. Reduce the linter library interface
to three functions: `Check()`, `New()`, and `Linter.Check()` by making
all helper functions private. Refactor the top-level `Check()` method to
rely on `New()` and `Linter.Check()` behind the scenes. Finally, create
a new helper method for creating a lint issuer certificate, call this
new method from `New()`, and store the result in the `Linter` struct.

Part of #5480
2021-07-09 10:29:10 -07:00
Aaron Gable 20f1bf1d0d
Compute validity periods inclusive of notAfter second (#5494)
In the CA, compute the notAfter timestamp such that the cert is actually
valid for the intended duration, not for one second longer. In the
Issuance library, compute the validity period by including the full
length of the final second indicated by the notAfter date when
determining if the certificate request matches our profile. Update tests
and config files to match.

Fixes #5473
2021-06-24 13:17:29 -07:00
Aaron Gable 54b697d51b
Remove CFSSL helpers and dependency (#5349)
Replace the few instances where we were relying on CFSSL utilities: for
OIDs and "helper" methods (parsing private keys and parsing SCT lists)
with our own code. Then delete all vendored CFSSL code.

Based on #5347
Fixes #5115
2021-03-18 17:28:00 -07:00
Samantha 82b200b8e9
Move core loadChain functionality from boulder-wfe to issuance (#5271)
loadChain is an unexported utility function recently added to
boulder-wfe to support the loading and validating of PEM files that
represent a certificate chain

This change moves the core loadChain functionality out of boulder-wfe to
a new exported LoadChain function in the Issuance package. All
boulder-wfe unit tests have been preserved and most of them have been
pared down and added to the Issuance package as well.

Blocks #1669
Fixes #5270
2021-02-04 16:41:25 -08:00
Aaron Gable 16c7a21a57
RA: Multi-issuer support for OCSP purging (#5160)
The RA is responsible for contacting Akamai to purge cached OCSP
responses when a certificate is revoked and fresh OCSP responses need to
be served ASAP. In order to do so, it needs to construct the same OCSP
URLs that clients would construct, and that Akamai would cache. In order
to do that, it needs access to the issuing certificate to compute a hash
across its Subject Info and Public Key.

Currently, the RA holds a single issuer certificate in memory, and uses
that cert to compute all OCSP URLs, on the assumption that all certs
we're being asked to revoke were issued by the same issuer.

In order to support issuance from multiple intermediates at the same
time (e.g. RSA and ECDSA), and to support rollover between different
issuers of the same type (we may need to revoke certs issued by two
different issuers for the 90 days in which their end-entity certs
overlap), this commit changes the configuration to provide a list of
issuer certificates instead.

In order to support efficient lookup of issuer certs, this change also
introduces a new concept, the Chain ID. The Chain ID is a truncated hash
across the raw bytes of either the Issuer Info or the Subject Info of a
given cert. As such, it can be used to confirm issuer/subject
relationships between certificates. In the future, this may be a
replacement for our current IssuerID (a truncated hash over the whole
issuer certificate), but for now it is used to map revoked certs to
their issuers inside the RA.

Part of #5120
2020-11-06 13:58:32 -08:00
Jacob Hoffman-Andrews cfe943fea5
Factor out idForIssuer. (#5102)
This was already part done: There is an ID() method in issuance. This
change extends that by:
 - Defining a type alias indicating something is an IssuerID.
 - Defining issuance.Certificate, which also has an ID() method,
   so that components that aren't the CA can load certificates and
   use the type system to mark them as issuers (and get their IDs).
 - Converting akamai-purger and ca to use the new types.
 - Removing idForIssuer from ca.go.
2020-10-06 15:04:43 -07:00
Aaron Gable 7672d9bc99
CA: Verify digitalSignature and certSign key usages (#5091)
When the CA loads new issuers (both their certificates and their
private keys), it performs a variety of sanity checks, such as
ensuring that the profile's signature algorithm matches the key
type.

With this change, we also check that the issuer's certificate has
the appropriate key usage bits set:
`certSign`, if it is going to be issuing end-entity certs; and
`digitalSignature`, because it will be signing OCSP responses for
previously-issued certificates.

Fixes #5068
2020-09-18 16:10:12 -07:00
Aaron Gable 2d10cce1a3
Refactor CA configs for more modularity (#5087)
The CA is the only service which still defines its json config format
in the package itself, rather than in its corresponding boulder-ca cmd
package. This has allowed the CA's constructor interface to hide
arbitrary complexity inside its first argument, the whole config blob.

This change moves the CA's config to boulder-ca/main.go, to match
the other Boulder components. In the process, it makes a host of
other improvements:

It refactors the issuance package to have a cleaner configuration
interface. It also separates the config into a high-level profile (which
applies equally to all issuers), and issuer-level profiles (which apply
only to a single issuer). This does involve some code duplication,
but that will be removed when CFSSL goes away.

It adds helper functions to the issuance package to make it easier
to construct a new issuer, and takes advantage of these in the
boulder-ca package. As a result, the CA now receives fully-formed
Issuers at construction time, rather than constructing them from
nearly-complete configs during its own initialization.

It adds a Linter struct to the lint package, so that an issuer can
simply carry around a Linter, rather than a separate lint signing
key and registry of lints to run.

It makes CFSSL-specific code more clearly marked as such,
making future removal easier and cleaner.

Fixes #5070
Fixes #5076
2020-09-14 18:38:12 -07:00
Aaron Gable d8a786ea08
Unify usage of 'issuer' and 'signer' as nouns (#5085)
We define a "signer" to be a private key, or something that satisfies the
crypto.Signer interface. We define an "issuer" to be an object which has
both a signer (so it can sign things) and a certificate (so that the things
it signs can have appropriate issuer fields set).

As a result, this change:
- moves the new "signer" library to be called "issuance" instead
- renames several "signers" to instead be "issuers", as defined above
- renames several "issuers" to instead be "certs", to reduce confusion more

There are some further cleanups which could be made, but most of them
will be made irrelevant by the removal of the CFSSL code, so I'm leaving
them be for now.
2020-09-10 17:18:42 -07:00