Replace "mocks.StorageAuthority" with "sapb.StorageAuthorityClient" in
our test mocks. The improves them by removing implementations of the
methods the tests don't actually need, instead of inheriting lots of
extraneous methods from the huge and cumbersome mocks.StorageAuthority.
This reduces our usage of mocks.StorageAuthority to only the WFE tests
(which create one in the frequently-used setup() function), which will
make refactoring those mocks in the pursuit of
https://github.com/letsencrypt/boulder/issues/7476 much easier.
Part of https://github.com/letsencrypt/boulder/issues/7476
In #6478, we stopped passing through Redis errors to the top-level
Responder object, preferring instead to live-sign. As part of that
change, we logged the Redis errors so they wouldn't disappear. However,
the sample rate for those errors was hard coded to 1-in-1000, instead of
following the LogSampleRate configured in the JSON.
This adds a field to redisSource for logSampleRate, and passes it
through from the JSON config in ocsp-responder/main.go.
Part of #7091
This change replaces [gorp] with [borp].
The changes consist of a mass renaming of the import and comments / doc
fixups, plus modifications of many call sites to provide a
context.Context everywhere, since gorp newly requires this (this was one
of the motivating factors for the borp fork).
This also refactors `github.com/letsencrypt/boulder/db.WrappedMap` and
`github.com/letsencrypt/boulder/db.Transaction` to not embed their
underlying gorp/borp objects, but to have them as plain fields. This
ensures that we can only call methods on them that are specifically
implemented in `github.com/letsencrypt/boulder/db`, so we don't miss
wrapping any. This required introducing a `NewWrappedMap` method along
with accessors `SQLDb()` and `BorpDB()` to get at the internal fields
during metrics and logging setup.
Fixes#6944
Enable the errcheck linter. Update the way we express exclusions to use
the new, non-deprecated, non-regex-based format. Fix all places where we
began accidentally violating errcheck while it was disabled.
When the ocsp-responder queries the database for a certificate status,
we want to return a 404 if we don't find a certificate status row for
the serial in question. This is because we often receive requests for
serials which we never issued, and for very old (expired) serials whose
status data we may have purged from the database.
Previously, we did this by checking whether the error returned by the
database was the "ErrNoRows" used by Go's SQL library. However, when the
ocsp-responder uses the SA to get this information, rather than querying
the database directly, the SA's gRPC service returns berrors.NotFound
instead. The code was not checking for this error, and therefore turned
some requests that should have been 404s into 500s.
Check for both kinds of "not found" error, and return a 404 for both.
Add tests to ensure that we return responder.ErrNotFound in both cases.
In #6293, we gave the ocsp-responder the ability to use a gRPC
connection to the SA to get status information for certificates, rather
than using a database connection directly. However, that change
neglected to make the database connection configuration optional: an
ocsp-responder with an SA gRPC client configured would never use its
database connection, but if it wasn't configured it would refuse to
start. Fix this oversight by making the DBConfig stanza optional.
In the WFE, ocsp-responder, and crl-updater, switch from using
StorageAuthorityClients to StorageAuthorityReadOnlyClients. This ensures
that these services cannot call methods which write to our database.
Fixes#6454
Previously, the live-signing routine was lookking for
`rocsp.ErrRedisNotFound` errors in order to increment the
`certificate_not_found` metrics. But this was a bug, copy-pasted from
code higher in the file that does a similar check. The live-signing code
actually returns `responder.ErrNotFound`. Check for that error instead,
to properly increment our metrics.
In live.Source, translate berrors.NotFound (returned by RA when the
certificate is expired) into responder.NotFound (which causes an
Unauthorized response rather than a 5xx).
In the Redis source, remove the special case that will return a stale
response if live signing fails, and simply pass through the error from
the live source.
Before this fix, if we found a stale response in Redis, tried to get a
fresh response, and found that the certificate was expired, we would
have served the stale response rather than our usual 404 for expired
certificates. Since that messes with our metrics, we don't want to do
it.
Also, fix an incorrect use of `%w` in log.Warningf.
Give less confusing names to the metrics in checked_redis_source, e.g.
"revocation_re_sign_success" instead of "sign_and_save_success".
Also use a new enum type as the `cause` parameter to signAndSave, to
make it clear what should be passed.
Finally, in redis_source, split `counter` into two separate Prometheus
counters: one for requests in general, and a separate one for
signAndSave. The counter for signAndSave has two labels: cause and
result.
Fixes#6339
The third argument to signAndSave is intended to be a "cause", to
provide a description of why we are doing a fresh signing that can
be included in our metric labels.
It was mistakenly being set to the serial number of the cert whose
new OCSP response is being generated, causing the number of
unique labels on this metric to explode.
Part of #6339
Add a new `GetRevocationStatus` gRPC method to the SA which retrieves
only the subset of the certificate status metadata relevant to
revocation, namely whether the certificate has been revoked, when it was
revoked, and the revocation reason. Notably, this method is our first
use of the `goog.protobuf.Timestamp` type in a message, which is more
ergonomic and less prone to errors than using unix nanoseconds.
Use this new method in ocsp-responder's checked_redis_source, to avoid
having to send many other pieces of metadata and the full ocsp response
bytes over the network. It provides all the information necessary to
determine if the response from Redis is up-to-date.
Within the checked_redis_source, use this new method in two different
ways: if only a database connection is configured (as is the case today)
then get this information directly from the db; if a gRPC connection to
the SA is available then prefer that instead. This may make requests
slower, but will allow us to remove database access from the hosts which
run the ocsp-responder today, simplifying our network.
The new behavior consists of two pieces, each locked behind a config
gate:
- Performing the smaller database query is only enabled if the
ocsp-responder has the `ROCSPStage3` feature flag enabled.
- Talking to the SA rather than the database directly is only enabled if
the ocsp-responder has an `saService` gRPC stanza in its config.
Fixes#6274
Add checkedRedisSource, a new OCSP Source which gets
responses from Redis, gets metadata from the database, and
only serves the Redis response if it matches the authoritative
metadata. If there is a mismatch, it requests a new OCSP
response from the CA, stores it in Redis, and serves the new
response.
This behavior is locked behind a new ROCSPStage3 feature flag.
Part of #6079
For multiSource, split out checkSecondary's metrics into their own
counter. Treat NotFound as a separate error type (so we can more
clearly distinguish the half-hourly pattern of fetches for expired
certificates).
In redisSource, add a histogram for the ages of responses fetched from
cache (regardless of whether they are served or not). This parallels
ocsp_respond_ages in ocsp/responder.go, but may show ages beyond the
compliance limit, even under normal operations, because it is checked
before signAndServe is called.
Previously we used "ExpectedFreshness" to control how frequently the
Redis source would request re-signing of stale entries. But that field
also controls whether multi_source is willing to serve a MariaDB
response. It's better to split these into two values.
This enables ocsp-responder to talk to the RA and request freshly signed
OCSP responses.
ocsp/responder/redis_source is moved to ocsp/responder/redis/redis_source.go
and significantly modified. Instead of assuming a response is always available
in Redis, it wraps a live-signing source. When a response is not available,
it attempts a live signing.
If live signing succeeds, the Redis responder returns the result right away
and attempts to write a copy to Redis on a goroutine using a background
context.
To make things more efficient, I eliminate an unneeded ocsp.ParseResponse
from the storage path. And I factored out a FakeResponse helper to make
the unittests more manageable.
Commits should be reviewable one-by-one.
Fixes#6191
Aaron Gable
George Ma
Jacob Hoffman-Andrews
Samantha