This was mostly unused. The only caller was orphan-finder, which used it
to determine if a certificate was already in the database. But this is
not particularly important functionality, so I've removed it.
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.
Use constants from the go stdlib time package, such as time.DateTime and
time.RFC3339, when parsing and formatting timestamps. Additionally,
simplify or remove some of our uses of parsing timestamps, such as to
set fake clocks in tests.
Remove the GenerateOCSP and GenerateCRL methods from the
CertificateAuthority gRPC service. These methods are no longer called by
any clients; all clients use their respective OCSPGenerator and
CRLGenerator gRPC services instead.
In addition, remove the CRLGeneratorServer field from the caImpl, as it
no longer needs it to serve as a backing implementation for the
GenerateCRL pass-through method. Unfortunately, we can't remove the
OCSPGeneratorServer field until after ROCSPStage7 is complete, and the
CA is no longer generating an OCSP response during initial certificate
issuance.
Part of #6448
The `digest` value in AddCertificate's response message is never used by
any callers. Remove it, replacing the whole response message with
google.protobuf.Empty, to mirror the AddPrecertificate method.
This swap is safe, because message names are not sent on the network,
and empty message fields are omitted from the wire format entirely, so
sending the predefined Empty message is identical to sending an empty
AddCertificateResponse message. Since no client is inspecting the
response to access the digest field, sending an empty response will not
break any clients.
Fixes#6498
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#5171Fixes#6476
Part of #5997
Add the Issuing Distribution Point extension to all of our end-entity
CRLs. The extension contains the Distribution Point, the URL from
which this CRL is meant to be downloaded. Because our CRLs are
sharded, this URL prevents an on-path attacker from substituting a
different shard than the client expected in order to hide a revocation.
The extension also contains the OnlyContainsUserCerts boolean,
because our CRLs only contain end-entity certificates.
The Distribution Point url is constructed from a configurable base URI,
the issuer's NameID, the shard index, and the suffix ".crl". The base
URI must use the "http://" scheme and must not end with a slash.
openssl displays the IDP extension as:
```
X509v3 Issuing Distribution Point: critical
Full Name:
URI:http://c.boulder.test/66283756913588288/0.crl Only User Certificates
```
Fixes#6410
Right now, Boulder expects to be able to connect to syslog, and panics
if it's not available. We'd like to be able to log to stdout/stderr as a
replacement for syslog.
- Add a detailed timestamp (down to microseconds, same as we collect in
prod via syslog).
- Remove the escape codes for colorizing output.
- Report the severity level numerically rather than with a letter prefix.
Add locking for stdout/stderr and syslog logs. Neither the [syslog] package
nor the [os] package document concurrency-safety, and the Go rule is: if
it's not documented to be concurrent-safe, it's not. Notably the [log.Logger]
package is documented to be concurrent-safe, and a look at its implementation
shows it uses a Mutex internally.
Remove places that use the singleton `blog.Get()`, and instead pass through
a logger from main in all the places that need it.
[syslog]: https://pkg.go.dev/log/syslog
[os]: https://pkg.go.dev/os
[log.Logger]: https://pkg.go.dev/log#Logger
Add a new `test.AssertNil()` helper to facilitate asserting that a given
unit test result is a non-boxed nil. Update `test.AssertNotNil()` to use
the reflect package's `.IsNil()` method to catch boxed nils.
In Go, variables whose type is constrained to be an interface type (e.g.
a function parameter which takes an interface, or the return value of a
function which returns `error`, itself an interface type) should
actually be thought of as a (T, V) tuple, where T is their underlying
concrete type and V is their underlying value. Thus, there are two ways
for such a variable to be nil-like: it can be truly nil where T=nil and
V is uninitialized, or it can be a "boxed nil" where T is a nillable
type such as a pointer or a slice and V=nil.
Unfortunately, only the former of these is == nil. The latter is the
cause of frequent bugs, programmer frustration, a whole entry in the Go
FAQ, and considerable design effort to remove from Go 2.
Therefore these two test helpers both call `t.Fatal()` when passed a
boxed nil. We want to avoid passing around boxed nils whenever possible,
and having our tests fail whenever we do is a good way to enforce good
nil hygiene.
Fixes#3279
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.
Add a new CA gRPC method named `GenerateCRL`. In the
style of the existing `GenerateOCSP` method, this new endpoint
is implemented as a separate service, for which the CA binary
spins up an additional gRPC service.
This method uses gRPC streaming for both its input and output.
For input, the stream must contain exactly one metadata message
identifying the crl number, issuer, and timestamp, and then any
number of messages identifying a single certificate which should
be included in the CRL. For output, it simply streams chunks of
bytes.
Fixes#6161
Profiling showed that the unit tests were spending almost all of
their time in key generation, because the tests were generating
new fake keys for the linter with every call to `setup()`. Instead,
create the linters just once at startup time.
This decreases the runtime of `go test ./ca/...` from ~9.7s to less
than one second.
These new linters are almost all part of golangci-lint's collection
of default linters, that would all be running if we weren't setting
`disable-all: true`. By adding them, we now have parity with the
default configuration, as well as the additional linters we like.
Adds the following linters:
* unconvert
* deadcode
* structcheck
* typecheck
* varcheck
* wastedassign
Use the T.TempDir method from the testing package to create temporary
directories for tests. Directories created by T.TempDir are automatically
removed when tests complete.
Remove the last of the gRPC wrapper files. In order to do so:
- Remove the `core.StorageGetter` interface. Replace it with a new
interface (whose methods include the `...grpc.CallOption` arg)
inside the `sa/proto/` package.
- Remove the `core.StorageAdder` interface. There's no real use-case
for having a write-only interface.
- Remove the `core.StorageAuthority` interface, as it is now redundant
with the autogenerated `sapb.StorageAuthorityClient` interface.
- Replace the `certificateStorage` interface (which appears in two
different places) with a single unified interface also in `sa/proto/`.
- Update all test mocks to include the `_ ...grpc.CallOption` arg in
their method signatures so they match the gRPC client interface.
- Delete many methods from mocks which are no longer necessary (mostly
because they're mocking old authz1 methods that no longer exist).
- Move the two `test/inmem/` wrappers into their own sub-packages to
avoid an import cycle.
- Simplify the `satest` package to satisfy one of its TODOs and to
avoid an import cycle.
- Add many methods to the `test/inmem/sa/` wrapper, to accommodate all
of the methods which are called in unittests.
Fixes#5600
Make the gRPC wrappers for the SA's `AddCertificate`,
`AddPrecertificate`, `AddSerial`, and `RevokeCertificate`
methods simple pass-throughs.
Fixup a couple tests that were passing only because their
requests to in-memory SA objects were not passing through
the wrapper's consistency checks.
Part of #5532
Make the gRPC wrappers for sa.GetCertificate and
sa.GetPrecertificate bare passthroughs. The latter of
these already took and returned appropriate protobufs,
so this change mostly just makes the former look like the
latter.
Part of #5532
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
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
- Remove field `ECDSAAllowedAccounts` from CA
- Remove `ECDSAAllowedAccounts` from CA tests
- Replace `ECDSAAllowedAccounts` with `ECDSAAllowListFilename` in
`test/config/ca-a.json` and `test/config/ca-b.json`
- Add YAML allow list file at `test/config/ecdsaAllowList.yml`
Fixes#5394
Replace `core.Empty` with `google.protobuf.Empty` in all of our gRPC
methods which consume or return an empty protobuf. The golang core
proto libraries provide an empty message type, so there is no need
for us to reinvent the wheel.
This change is backwards-compatible and does not require a special
deploy. The protobuf message descriptions of `core.Empty` and
`google.protobuf.Empty` are identical, so their wire-formats are
indistinguishable and therefore interoperable / cross-compatible.
Fixes#5443
- Add field `ECDSAAllowListFilename` to `config.CA`
- Move ECDSA allow list logic from `boulder-ca/main.go` to new file
`ca/ecdsa_allow_list.go`
- Add field `ecdsaAllowList` to `certificateAuthorityImpl`
- Update units test to account for changes to `certificateAuthorityImpl`
- Move previous allow list unit tests to `TestDeprecatedECDSAAllowList`
- Add `TestECDSAAllowList` units tests
Fixes#5361
Create a new `ocspImpl` struct which satisfies the interface required
by the `OCSPGenerator` gRPC service. Move the `GenerateOCSP`
method from the `certificateAuthorityImpl` to this new type. To support
existing gRPC clients, keep a reference to the new OCSP service in
the CA impl, and maintain a pass-through `GenerateOCSP` method.
Simplify some of the CA setup code, and make the CA implementation
non-exported because it doesn't need to be.
In order to maintain our existing signature and sign error metrics,
they now need to be initialized outside the CA and OCSP constructors.
This complicates the tests slightly, but seems like a worthwhile
tradeoff.
Fixes#5226Fixes#5086
Only process OCSP generation requests which are identified
by the certificate's serial number and the ID (not NameID,
unfortunately) of its issuer. Delete the code path which handled
OCSP generation for requests identified by the full DER of
the certificate in question.
Update existing tests to use serial+id to request OCSP, and
move test cases from the old `TestGenerateOCSPWithIssuerID`
into the default test method.
Part of #5079
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.
Add a new label, "issuer", to the `signatureCount` prometheus metric
which is exported by the CA. This allows us to separately monitor
signatures by the R3 and E1 intermediates, for all three kinds of
issuance we perform: precertificate, certificate, and ocsp. Add the
appropriate label value when incrementing this metric.
Also add a new `AssertMetricWithLabelsEquals` method to our test
helpers. This allows us to make assertions about the total value of
all metrics with a particular subset of labels set, rather than only
being able to make assertions about metrics which are parameterized by
only a single label. A follow-up change will migrate our usages of
CountCounterVec, CountCounter, and GaugeValueWithLabels to use this new
helper instead.
Fixes#5354
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 #5347Fixes#5115
Make the `NonCFSSLSigner` code path the only code path through the CA.
Remove all code related to the old, CFSSL-based code path. Update tests
to supply (or mock) issuers of the new kind. Remove or simplify a few
tests that were testing for behavior only exhibited by the old code
path, such as incrementing certain metrics. Remove code from `//cmd/`
for initializing the CFSSL library. Finally, mark the `NonCFSSLSigner`
feature flag itself as deprecated.
Delete the portions of the vendored CFSSL code which were only used
by these deleted code paths. This does not remove the CFSSL library
entirely, the rest of the cleanup will follow shortly.
Part of #5115
Currently, the CA is configured with a set of `internalIssuer`s,
and a mapping of public key algorithms (e.g. `x509.RSA`) to which
internalIssuer to use. In operation today, we use the same issuer
for all kinds of public key algorithms. In the future, we will use
different issuers for different algorithms (in particular, we will
use R3 to issue for RSA keys, and E1 to issue for ECDSA keys). But
we want to roll that out slowly, continuing to use our RSA issuer
to issue for all types of public keys, except for ECDSA keys which
are presented by a specific set of allowed accounts.
This change adds a new config field to the CA, which lets us specify
a small list of registration IDs which are allowed to have issuance
from our ECDSA issuer. If the config list is empty, then all accounts
are allowed. The CA checks to see if the key being issued for is
ECDSA: if it is, it then checks to make sure that the associated
registration ID is in the allowlist. If the account is not allowed,
it then overrides the issuance algorithm to use RSA instead,
mimicking our old behavior. It also adds a new feature flag, which
can be enabled to skip the allowlist entirely (effectively allowing
all registered accounts). This feature flag will be enabled when
we're done with our testing and confident in our ECDSA issuance.
Fixes#5259
This adds a new component to the CA, ocspLogQueue, which batches up
OCSP generation events for audit logging. It will log accumulated
events when it reaches a certain line length, or when a maximum amount
of times has passed.
This change adds two new test assertion helpers, `AssertErrorIs`
and `AssertErrorWraps`. The former is a wrapper around `errors.Is`,
and asserts that the error's wrapping chain contains a specific (i.e.
singleton) error. The latter is a wrapper around `errors.As`, and
asserts that the error's wrapping chain contains any error which is
of the given type; it also has the same unwrapping side effect as
`errors.As`, which can be useful for further assertions about the
contents of the error.
It also makes two small changes to our `berrors` package, namely
making `berrors.ErrorType` itself an error rather than just an int,
and giving `berrors.BoulderError` an `Unwrap()` method which
exposes that inner `ErrorType`. This allows us to use the two new
helpers above to make assertions about berrors, rather than
having to hand-roll equality assertions about their types.
Finally, it takes advantage of the two changes above to greatly
simplify many of the assertions in our tests, removing conditional
checks and replacing them with simple assertions.
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.
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#5070Fixes#5076
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.
There are two code paths which end up calling the SA's AddPrecertificate
RPC: one "normal" path from inside the CA's IssuePrecertificate
method, and one "exceptional" path from inside the orphan-integration
path which only gets executed if the initial attempt to store the cert
failed for some reason.
The first of these paths always sets the IssuerID in the RPC's
AddCertificateRequest. The latter of these paths never does.
Historically, this has been fine, as the SA has stored NULL in the
certificateStatus table when given a nil IssuerID, which matches the
behavior prior to the StoreIssuerInfo flag (when the IssuerID was
always nil, and cert status was tracked by full cert DER instead).
However, with the switch to proto3, the SA now interprets a nil
IssuerID as a 0, and stores that value in the table instead. The
ocsp-updater code which consumes rows of the certificateStatus table
is not able to properly handle IssuerIDs of 0, leading to fun times.
This change ensures that the orphan handling code also sets the
IssuerID when asking the SA to create a new certificateStatus row,
so we should stop producing new rows with a 0 IssuerID.
The ca's configuration already has support for containing multiple
issuers. However, when it comes time to actually sign a (pre)cert,
it always uses the defaultIssuer.
This change has the ca instead choose which issuer to use based
on the PublicKeyAlgorithm requested in the CSR (or, for final cert
issuances, based on the PublicKeyAlgorithm in the precert).
This will allow us to use our RSA issuers to sign certificates for
users who aren't ready to switch to ECDSA, while immediately switching
to our new ECDSA chain for subscribers who want to use it.
Fixed#5027
One slightly surprising / interesting thing: Since core types like
Order and Registration are still proto2 and have pointer fields,
there are actually some places in this PR where I had to add
a `*` rather than delete an `&`, because I was taking a pointer
field from one of those core types and passing it as a field in
an SA RPC request.
Fixes#5037.
Adds a replacement issuance library that replaces CFSSL. Usage of the
new library is gated by a feature, meaning until we fully deploy the
new signer we need to support both the new one and CFSSL, which makes
a few things a bit complicated.
One Big follow-up change is that once CFSSL is completely gone we'll
be able to stop using CSRs as the internal representation of issuance
requests (i.e. instead of passing a CSR all the way through from the
WFE -> CA and then converting it to the new signer.IssuanceRequest,
we can just construct a signer.IssuanceRequest at the WFE (or RA) and
pass that through the backend instead, making things a lot less opaque).
Fixes#4906.
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.
This is the only method on the ca which uses a non-proto
type as its request or response value. Changing this to
use a proto removes the last logic from the wrappers,
allowing them to be removed in a future CL. It also makes
the interface more uniform and easier to reason about.
Issue: #4940
We previously used mixed case names for proto imports
(e.g. both `caPB` and `rapb`), sometimes in the same file.
This change standardizes on the all-lowercase spelling,
which was predominant throughout the codebase.
This updates the ca.proto to use proto3 syntax, and updates
all clients of the autogenerated code to use the new types. In
particular, it removes indirection from built-in types (proto3
uses ints, rather than pointers to ints, for example).
It also updates a few instances where tests were being
conducted to see if various object fields were nil to instead
check for those fields' new zero-value.
Fixes#4940
When StoreIssuerInfo is enabled the CA loses its ability to verify that the certificate we are requesting an OCSP response for is real directly (previously we sent the cert DER and checked the signature on it). In order to prevent the ocsp-updater from sending a request for a serial that doesn't exist we added a check that the serial we were being asked to generate a response for did actually exist. This introduced a significant amount of database pressure as it requires a DB query for every single OCSP response we generate. It also provides a minimal level of security, we already trust the ocsp-updater and creating a response for a certificate that doesn't exist doesn't actually accomplish much (if the ocsp-updater was compromised the more realistic attack would be asking to generate a good response for a revoked certificate).
This change removes the check that the serial exists from the CA.
Fixes#4935.
The `KeyPolicy.GoodKey` method is used to validate both public keys
used to sign JWK messages, and public keys contained inside CSR
messages.
According to RFC8555 section 6.7, validation failure in the former
case should result in `badPublicKey`, while validation failure in
the latter case should result in `badCSR`. In either case, a failure
due to reasons other than the key itself should result in
`serverInternal`.
However, the GoodKey method returns a variety of different errors
which are not all applicable depending on the context in which it is
called. In addition, the `csr.VerifyCSR` method passes these errors
through verbatim, resulting in ACME clients receiving confusing and
incorrect error message types.
This change causes the GoodKey method to always return either a
generic error or a KeyError. Calling methods should treat a `KeyError`
as either a `badPublicKey` or a `badCSR` depending on their context,
and may treat a generic error however they choose (though likely as a
serverInternal error).
Fixes#4930
This reverts commit 6454513ded.
We actually need to wait 90 days to ensure the issuerID field of the
certificateStatus table is non-nil for all extant certificates.
As part of that, add support for issuer IDs in orphan-finder's
and RA's calls to GenerateOCSP.
This factors out the idForIssuer logic from ca/ca.go into a new
issuercerts package.
orphan-finder refactors:
Add a list of issuers in config.
Create an orphanFinder struct to hold relevant fields, including the
newly added issuers field.
Factor out a storeDER function to reduce duplication between the
parse-der and parse-ca-log cases.
Use test certificates generated specifically for orphan-finder tests.
This was necessary because the issuers of these test certificates have
to be configured for the orphan finder.
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.
The `orphans` Prometheus `CounterVec` is used to count orphans that
couldn't be confirmed saved by the SA and were queued by the CA.
The `adopted_orphans` `CounterVec` is used to count orphans pulled from
the queue by the CA and successfully integrated through to the SA.
Both counter stats are labelled by "type", e.g. "precert" or "cert".
This avoids needing to send the entire certificate in OCSP generation
RPCs.
Ended up including a few cleanups that made the implementation easier.
Initially I was struggling with how to derive the issuer identification info.
We could just stick the full SPKI hash in certificateStatus, but that takes a
significant amount of space, we could configure unique issuer IDs in the CA
config, but that would require being very careful about keeping the IDs
constant, and never reusing an ID, or we could store issuers in a table in the
database and use that as a lookup table, but that requires figuring out how to
get that info into the table etc. Instead I've just gone with what I found to
be the easiest solution, deriving a stable ID from the cert hash. This means we
don't need to remember to configure anything special and the CA config stays
the same as it is now.
Fixes#4469.
In the process, rename generateOCSPAndStoreCertificate to just
storeCertificate, because that function doesn't generate OCSP anymore;
instead the OCSP is generated (and stored) at precertificate issuance
time.
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
This change adds two tables and two methods in the SA, to store precertificates
and serial numbers.
In the CA, when the feature flag is turned on, we generate a serial number, store it,
sign a precertificate and OCSP, store them, and then return the precertificate. Storing
the serial as an additional step before signing the certificate adds an extra layer of
insurance against duplicate serials, and also serves as a check on database availability.
Since an error storing the serial prevents going on to sign the precertificate, this decreases
the chance of signing something while the database is down.
Right now, neither table has read operations available in the SA.
To make this work, I needed to remove the check for duplicate certificateStatus entry
when inserting a final certificate and its OCSP response. I also needed to remove
an error that can occur when expiration-mailer processes a precertificate that lacks
a final certificate. That error would otherwise have prevented further processing of
expiration warnings.
Fixes#4412
This change builds on #4417, please review that first for ease of review.
The `test/config-next` CA configs are both updated to use `zlint` to lint TBS
pre-certificates with a throw-away key and treat any lint findings >=
`lints.Pass` as an error, blocking the CA from signing the TBS pre-cert with its
private key.
The CA `issuePrecertificateInner` function is updated to specifically catch
linting related errors from CFSSL to marshal the linting findings to the audit
log. A small unit test for this change is included.
The CA `IssueCertificateForPrecertificate` function remains unchanged: the CFSSL
interface that defines `SignFromPrecert` doesn't facilitate linting. We still
lint final certificates post-issuance with `cert-checker` and accept the
possibility there may be some compliance issues that could occur between the
precertificate passing linting and the final certificate being signed.
Resolves https://github.com/letsencrypt/boulder/issues/4255
Precertificate issuance has been the only supported mode for a while now. This
cleans up the remaining flags in the CA code. The same is true of must staple.
This also removes the IssueCertificate RPC call and its corresponding wrappers,
and removes a lot of plumbing in the CA unittests that was used to test the
situation where precertificate issuance was not enabled.
* in boulder-ra we connected to the publisher and created a publisher gRPC client twice for no apparent reason
* in the SA we ignored errors from `getChallenges` in `GetAuthorizations` which could result in a nil challenge being returned in an authorization
Use a boulder error type to indicate duplicate rows instead of a normal untyped error (as gRPC mangles this type of error but understands how to properly handle a boulder error).
Retains the existing logging of orphaned certs until we are confident that this
solution can fully replace it (even then we may want to keep it just for auditing etc).
Fixes#3636.
Fixes#3836.
```
$ ./test.sh
ok github.com/cloudflare/cfssl/api 1.023s coverage: 81.1% of statements
ok github.com/cloudflare/cfssl/api/bundle 1.464s coverage: 87.2% of statements
ok github.com/cloudflare/cfssl/api/certadd 16.766s coverage: 86.8% of statements
ok github.com/cloudflare/cfssl/api/client 1.062s coverage: 51.9% of statements
ok github.com/cloudflare/cfssl/api/crl 1.075s coverage: 75.0% of statements
ok github.com/cloudflare/cfssl/api/gencrl 1.038s coverage: 72.5% of statements
ok github.com/cloudflare/cfssl/api/generator 1.478s coverage: 33.3% of statements
ok github.com/cloudflare/cfssl/api/info 1.085s coverage: 84.1% of statements
ok github.com/cloudflare/cfssl/api/initca 1.050s coverage: 90.5% of statements
ok github.com/cloudflare/cfssl/api/ocsp 1.114s coverage: 93.8% of statements
ok github.com/cloudflare/cfssl/api/revoke 3.063s coverage: 75.0% of statements
ok github.com/cloudflare/cfssl/api/scan 2.988s coverage: 62.1% of statements
ok github.com/cloudflare/cfssl/api/sign 2.680s coverage: 83.3% of statements
ok github.com/cloudflare/cfssl/api/signhandler 1.114s coverage: 26.3% of statements
ok github.com/cloudflare/cfssl/auth 1.010s coverage: 68.2% of statements
ok github.com/cloudflare/cfssl/bundler 22.078s coverage: 84.5% of statements
ok github.com/cloudflare/cfssl/certdb/dbconf 1.013s coverage: 84.2% of statements
ok github.com/cloudflare/cfssl/certdb/ocspstapling 1.302s coverage: 69.2% of statements
ok github.com/cloudflare/cfssl/certdb/sql 1.223s coverage: 70.5% of statements
ok github.com/cloudflare/cfssl/cli 1.014s coverage: 62.5% of statements
ok github.com/cloudflare/cfssl/cli/bundle 1.011s coverage: 0.0% of statements [no tests to run]
ok github.com/cloudflare/cfssl/cli/crl 1.086s coverage: 57.8% of statements
ok github.com/cloudflare/cfssl/cli/gencert 7.927s coverage: 83.6% of statements
ok github.com/cloudflare/cfssl/cli/gencrl 1.064s coverage: 73.3% of statements
ok github.com/cloudflare/cfssl/cli/gencsr 1.058s coverage: 70.3% of statements
ok github.com/cloudflare/cfssl/cli/genkey 2.718s coverage: 70.0% of statements
ok github.com/cloudflare/cfssl/cli/ocsprefresh 1.077s coverage: 64.3% of statements
ok github.com/cloudflare/cfssl/cli/revoke 1.033s coverage: 88.2% of statements
ok github.com/cloudflare/cfssl/cli/scan 1.014s coverage: 36.0% of statements
ok github.com/cloudflare/cfssl/cli/selfsign 2.342s coverage: 73.2% of statements
ok github.com/cloudflare/cfssl/cli/serve 1.076s coverage: 38.2% of statements
ok github.com/cloudflare/cfssl/cli/sign 1.070s coverage: 54.8% of statements
ok github.com/cloudflare/cfssl/cli/version 1.011s coverage: 100.0% of statements
ok github.com/cloudflare/cfssl/cmd/cfssl 1.028s coverage: 0.0% of statements [no tests to run]
ok github.com/cloudflare/cfssl/cmd/cfssljson 1.012s coverage: 3.4% of statements
ok github.com/cloudflare/cfssl/cmd/mkbundle 1.011s coverage: 0.0% of statements [no tests to run]
ok github.com/cloudflare/cfssl/config 1.023s coverage: 67.7% of statements
ok github.com/cloudflare/cfssl/crl 1.054s coverage: 68.3% of statements
ok github.com/cloudflare/cfssl/csr 8.473s coverage: 89.6% of statements
ok github.com/cloudflare/cfssl/errors 1.014s coverage: 79.6% of statements
ok github.com/cloudflare/cfssl/helpers 1.216s coverage: 80.6% of statements
ok github.com/cloudflare/cfssl/helpers/derhelpers 1.017s coverage: 48.0% of statements
ok github.com/cloudflare/cfssl/helpers/testsuite 7.826s coverage: 65.8% of statements
ok github.com/cloudflare/cfssl/initca 151.314s coverage: 73.2% of statements
ok github.com/cloudflare/cfssl/log 1.013s coverage: 59.3% of statements
ok github.com/cloudflare/cfssl/multiroot/config 1.258s coverage: 77.4% of statements
ok github.com/cloudflare/cfssl/ocsp 1.353s coverage: 75.1% of statements
ok github.com/cloudflare/cfssl/revoke 1.149s coverage: 75.0% of statements
ok github.com/cloudflare/cfssl/scan 1.023s coverage: 1.1% of statements
skipped github.com/cloudflare/cfssl/scan/crypto/md5
skipped github.com/cloudflare/cfssl/scan/crypto/rsa
skipped github.com/cloudflare/cfssl/scan/crypto/sha1
skipped github.com/cloudflare/cfssl/scan/crypto/sha256
skipped github.com/cloudflare/cfssl/scan/crypto/sha512
skipped github.com/cloudflare/cfssl/scan/crypto/tls
ok github.com/cloudflare/cfssl/selfsign 1.098s coverage: 70.0% of statements
ok github.com/cloudflare/cfssl/signer 1.020s coverage: 19.4% of statements
ok github.com/cloudflare/cfssl/signer/local 4.886s coverage: 77.9% of statements
ok github.com/cloudflare/cfssl/signer/remote 2.500s coverage: 70.0% of statements
ok github.com/cloudflare/cfssl/signer/universal 2.228s coverage: 67.7% of statements
ok github.com/cloudflare/cfssl/transport 1.012s
ok github.com/cloudflare/cfssl/transport/ca/localca 1.046s coverage: 94.9% of statements
ok github.com/cloudflare/cfssl/transport/kp 1.050s coverage: 37.1% of statements
ok github.com/cloudflare/cfssl/ubiquity 1.037s coverage: 88.3% of statements
ok github.com/cloudflare/cfssl/whitelist 3.519s coverage: 100.0% of statements
...
$ go test ./... (master✱)
ok golang.org/x/crypto/acme 2.782s
ok golang.org/x/crypto/acme/autocert 2.963s
? golang.org/x/crypto/acme/autocert/internal/acmetest [no test files]
ok golang.org/x/crypto/argon2 0.047s
ok golang.org/x/crypto/bcrypt 4.694s
ok golang.org/x/crypto/blake2b 0.056s
ok golang.org/x/crypto/blake2s 0.050s
ok golang.org/x/crypto/blowfish 0.015s
ok golang.org/x/crypto/bn256 0.460s
ok golang.org/x/crypto/cast5 4.204s
ok golang.org/x/crypto/chacha20poly1305 0.560s
ok golang.org/x/crypto/cryptobyte 0.014s
? golang.org/x/crypto/cryptobyte/asn1 [no test files]
ok golang.org/x/crypto/curve25519 0.025s
ok golang.org/x/crypto/ed25519 0.073s
? golang.org/x/crypto/ed25519/internal/edwards25519 [no test files]
ok golang.org/x/crypto/hkdf 0.012s
ok golang.org/x/crypto/internal/chacha20 0.047s
ok golang.org/x/crypto/internal/subtle 0.011s
ok golang.org/x/crypto/md4 0.013s
ok golang.org/x/crypto/nacl/auth 9.226s
ok golang.org/x/crypto/nacl/box 0.016s
ok golang.org/x/crypto/nacl/secretbox 0.012s
ok golang.org/x/crypto/nacl/sign 0.012s
ok golang.org/x/crypto/ocsp 0.047s
ok golang.org/x/crypto/openpgp 8.872s
ok golang.org/x/crypto/openpgp/armor 0.012s
ok golang.org/x/crypto/openpgp/clearsign 16.984s
ok golang.org/x/crypto/openpgp/elgamal 0.013s
? golang.org/x/crypto/openpgp/errors [no test files]
ok golang.org/x/crypto/openpgp/packet 0.159s
ok golang.org/x/crypto/openpgp/s2k 7.597s
ok golang.org/x/crypto/otr 0.612s
ok golang.org/x/crypto/pbkdf2 0.045s
ok golang.org/x/crypto/pkcs12 0.073s
ok golang.org/x/crypto/pkcs12/internal/rc2 0.013s
ok golang.org/x/crypto/poly1305 0.016s
ok golang.org/x/crypto/ripemd160 0.034s
ok golang.org/x/crypto/salsa20 0.013s
ok golang.org/x/crypto/salsa20/salsa 0.013s
ok golang.org/x/crypto/scrypt 0.942s
ok golang.org/x/crypto/sha3 0.140s
ok golang.org/x/crypto/ssh 0.939s
ok golang.org/x/crypto/ssh/agent 0.529s
ok golang.org/x/crypto/ssh/knownhosts 0.027s
ok golang.org/x/crypto/ssh/terminal 0.016s
ok golang.org/x/crypto/tea 0.010s
ok golang.org/x/crypto/twofish 0.019s
ok golang.org/x/crypto/xtea 0.012s
ok golang.org/x/crypto/xts 0.016s
```
Things removed:
* features.EmbedSCTs (and all the associated RA/CA/ocsp-updater code etc)
* ca.enablePrecertificateFlow (and all the associated RA/CA code)
* sa.AddSCTReceipt and sa.GetSCTReceipt RPCs
* publisher.SubmitToCT and publisher.SubmitToSingleCT RPCs
Fixes#3755.
The Boulder orphan-finder command uses the SA's AddCertificate RPC to add orphaned certificates it finds back to the DB. Prior to this commit this RPC always set the core.Certificate.Issued field to the
current time. For the orphan-finder case this meant that the Issued date would incorrectly be set to when the certificate was found, not when it was actually issued. This could cause cert-checker to alarm based on the unusual delta between the cert NotBefore and the core.Certificate.Issued value.
This PR updates the AddCertificate RPC to accept an optional issued timestamp in the request arguments. In the SA layer we address deployability concerns by setting a default value of the current time when none is explicitly provided. This matches the classic behaviour and will let an old RA communicate with a new SA.
This PR updates the orphan-finder to provide an explicit issued time to sa.AddCertificate. The explicit issued time is calculated using the found certificate's NotBefore and the configured backdate.
This lets the orphan-finder set the true issued time in the core.Certificate object, avoiding any cert-checker alarms.
Resolves#3624
Adds SCT embedding to the certificate issuance flow. When a issuance is requested a precertificate (the requested certificate but poisoned with the critical CT extension) is issued and submitted to the required CT logs. Once the SCTs for the precertificate have been collected a new certificate is issued with the poison extension replace with a SCT list extension containing the retrieved SCTs.
Fixes#2244, fixes#3492 and fixes#3429.
We removed most of the component-specific configs out of cmd a long time ago. We
left CAConfig in, because it is used directly in the NewCertificateAuthority
constructor. That means that moving CAConfig into cmd/boulder-ca would have
resulted in a circular dependency.
Eventually we probably want to decompose CAConfig so it's a set of arguments to
NewCertificateAuthority, but as a short term improvement, move the config into
its own package to break the dependency. This has the advantage of removing a
couple of big dependencies from cmd.
Add a logging statement that fires when a remote VA fail causes
overall failure. Also change remoteValidationFailures into a
counter that counts the same thing, instead of a histogram. Since
the histogram had the default bucket sizes, it failed to collect
what we needed, and produced more metrics than necessary.
Stub out IssueCertificateForPrecertificate() enough so that we can continue with the PRs that implement & test it in parallel with PRs that implement and test the calling side (via mock implementations of the CA side).
The notBefore date in certificates is set based on the current system time,
not based on ca.clk. Work around that problem in the issuance tests by
syncing the test ca.clk with the system time. This doesn't affect any
current tests but is required for upcoming tests to work correctly.
* CA: Stub IssuePrecertificate gPRC method.
* CA: Implement IssuePrecertificate.
* CA: Test Precertificate flow in TestIssueCertificate().
move verification of certificate storage
IssuePrecertificate tests
Add CT precertificate poison extension to CFSSL whitelist.
CFSSL won't allow us to add an extension to a certificate unless that
certificate is in the whitelist.
According to its documentation, "Extensions requested in the CSR are
ignored, except for those processed by ParseCertificateRequest (mainly
subjectAltName)." Still, at least we need to add tests to make sure a
poison extension in a CSR isn't copied into the final certificate.
This allows us to avoid making invasive changes to CFSSL.
* CA: Test precertificate issuance in TestInvalidCSRs().
* CA: Only support IssuePrecertificate() if it is explicitly enabled.
* CA: Test that we produce CT poison extensions in the valid form.
The poison extension must be critical in order to work correctly. It probably wouldn't
matter as much what the value is, but the spec requires the value to be ASN.1 NULL, so
verify that it is.
The test previously used an invalid encoding of the CT poison extension
(the value was empty, but a valid CT poison extension has a NULL value).
In preparation for testing specifically how the CT poison extension is
handled, change the test to use a different extension instead.
Split the profile issuance tests such that there is one call to IssueCertificate per test, like
the other certificate issuance tests. This will make it easier to later move the calls to
IssueCertificate() into TestIssueCertificate(), which will make it much easier to test the
precertificate-based flow in addition to the current issuance flow.
Take a step towards enabling the testing of precertificate issuance by
enabling the logic in these tests to be used for testing all forms
of Certificate and Precertificate issuance.
An early design mistake meant that some fields of our services were exported
unnecessarily. In particular, fields storing handles of other services (e.g.
"SA" or "PA") were exported. This introduces the possibility of race conditions,
though in practice these fields are set at startup and never modified
concurrently.
We'd like to go through our codebase and change these all to unexported fields,
set at construction time. This is one step in that process.
This is a step towards the long-term goal of eliminating wrappers and a step
towards the short-term goal of making it easier to refactor ca/ca_test.go to
add testing of precertificate-based issuance.
This will make it easier to add new tests of this form and will also
make it easier to adapt the tests to also test the precertificate +
certificate issuance flow.
Remove the use of mocks for stats in ca_test.go in order to make refactoring
those tests easier. To do so, switch to the same pattern used by the
signature metrics.
Following up on #2752, we don't need to use global vars for our Prometheus stats. We already have a custom registry plumbed through using Scope objects. In this PR, expose the MustRegister method of that registry through the Scope interface, and move existing global vars to be fields of objects. This should improve testability somewhat.
Note that this has a bit of an unfortunate side effect: two instances of the same stats-using class (e.g. VA) can't use the same Scope object, because their MustRegister calls will conflict. In practice this is fine since we never instantiate duplicates of the the classes that use stats, but it's something we should keep an eye on.
Updates #2733
Jacob Hoffman-Andrews
Matthew McPherrin
Aaron Gable
Eng Zer Jun
Samantha
Jacob Hoffman-Andrews
Roland Bracewell Shoemaker
Daniel McCarney
Roland Bracewell Shoemaker
Daniel McCarney
Brian Smith