Drop the reversedName_renewal_notBefore_Idx from the issuedNames table.
This index was added to facilitate rate limit queries, but we now use
the certificatesPerName table for rate limits instead.
Keep the reversedName_notBefore_Idx in place, as it is still useful for
gathering stats on how many hostnames have active certificates.
Fixes#3180
Historically the only database/sql driver setting exposed via JSON
config was maxDBConns. This change adds support for maxIdleConns,
connMaxLifetime, connMaxIdleTime, and renames maxDBConns to
maxOpenConns. The addition of these settings will give our SRE team a
convenient method for tuning the reuse/closure of database connections.
A new struct, DBSettings, has been added to SA. The struct, and each of
it's fields has been commented.
All new fields have been plumbed through to the relevant Boulder
components and exported as Prometheus metrics. Tests have been
added/modified to ensure that the fields are being set. There should be
no loss in coverage
Deployability concerns for the migration from maxDBConns to maxOpenConns
have been addressed with the temporary addition of the helper method
cmd.DBConfig.GetMaxOpenConns(). This method can be removed once
test/config is defaulted to using maxOpenConns. Relevant sections of the
code have TODOs added that link back to an newly opened issue.
Fixes#5199
This change refactors the sa.NewOrder method to be more readable.
Previously, the outer method both modified and returned the request
object, a confusion violation of abstraction. Simultaneously, the inner
transaction function modified its input order object, but returned the
original request object which it hadn't modified.
Now, the NewOrder method does not modify its input, and instead
returns an all-new Order object. Additionally, the inner transaction
function does not modify its input, and returns an order model with
relevant fields set.
Part of #5166
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.
errors.As checks for a specific error in a wrapped error chain
(see https://golang.org/pkg/errors/#As) as opposed to asserting
that an error is of a specific type.
Part of #5010
This change adds `req.IssuerID` to the set of fields that the SA's
`AddPrecertificate` method requires be non-zero.
As a result, this also updates many tests, both unit and integration,
to ensure that they supply a value (usually just 1) for that field. The
most complex part of the test changes is a slight refactoring to the
orphan-finder code, which makes it easier to reason about the
separation between log line parsing and building and sending the
request.
Based on #5096Fixes#5097
The previous implementation of `IsAnyNilOrZero` did not in fact work,
and its tests did not catch this fact. Within the numeric clause, the
compiler would only instantiate the comparison literal 0 to be one
of the eight possible types. Comparisons against any of the other
seven types would always be false, no matter what value that type
held.
The tests did not catch this because they only tested two literal
values: `0` and `-12.345`, both of which can be `float64`s.
This change updates the utility function to use the `reflect` package,
to ensure that it works correctly. It also updates the test to test
multiple different kinds of numeric values, and removes the code
for handling pointer-to- types, as all of our proto2 code has been
removed.
Finally, it updates the SA wrapper's `RevokeCertificate` method to
correctly not require that `req.Reason` be non-zero: this field can
and often is zero, as that value represents `Unspecified`.
Using the reflect package is a conscious tradeoff. It will be slower
than manually writing out every single case, but it will also be less
prone to error.
Part of #5097
It's not vital that this row be strongly consistent with the other
updates. And updating it inside the transaction means we hold a lock on
this row while doing a bunch of other expensive inserts, which is likely
creating lock contention.
These were used during the transition to authzv2. The SA side of these
RPCs already ignores these booleans. This is just cleaning up the
protobufs and call sites.
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.
As part of the migration to proto3, any fields in requests that may be
zero should also be allowed to be nil. That's because proto3 will
represent those fields as absent when they have their zero value.
This is based on a manual review of the wrappers for the SA, plus
a pair of integration test runs. For the integration test runs I took these
steps:
1. Copy sa/proto to sa/proto2
2. Change sa/proto to use proto3 and regenerate.
3. In sa/*.go and cmd/boulder-sa/main.go, update the imports to use the
proto2 version.
4. Split grpc/sa-wrappers.go into sa-server-wrappers.go and sa-wrappers.go
(containing the client code)
5. In sa-server-wrappers.go, change the import to use sa/proto2.
6. In sa-server-wrappers.go, make a local copy of the core.StorageAuthority
interface that uses the sa/proto2 types. This was necessary as
a temporary kludge because of how the server wrapper internally
uses the core.StorageAuthority interface.
7. Fix all the pointer-vs-value build errors in every other package.
8. Run integration tests.
I also performed those steps with proto2 and proto3 swapped, to confirm the
behavior when a proto2 client talks to a proto3 SA.
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
Our proto files had a variety of indentation styles: 2 spaces,
4 spaces, 8 spaces, and tabs; sometimes mixed within the same
file. The proto3 style guide[1] says to use 2-space indents,
so this change standardizes on that.
[1] https://developers.google.com/protocol-buffers/docs/style
Previously, this limit was bucketed by hour, but that created too much
sudden traffic at the beginning of each hour as accounts' rate limits
expired. Chunking by the minute should make it possible to smooth out
traffic more.
Simplify database interactions
This change is a result of an audit of all places where
Go code directly constructs SQL queries and executes them
against a dbMap, with the goal of eliminating all instances
of constructing a well-known object type (such as a
core.CertificateStatus) from explicitly-listed database columns.
Instead, we should be relying on helper functions defined in the
sa itself to determine which columns are relevant for the
construction of any given object.
This audit did not find many places where this was occurring. It
did reveal a few simplifications, which are contained in this
change:
1) Greater use of existing SelectFoo methods provided by models.go
2) Streamlining of various SelectSingularFoo methods to always
select by serial string, rather than user-provided WHERE clause
3) One spot (in ocsp-responder) where using a well-known type seemed
better than using a more minimal custom type
Addresses #4899
The StoreKeyHashes feature flag controls whether rows are added to the
keyHashToSerial table. This feature is now enabled everywhere, so the
flag-protected code can be turned on unconditionally and the flag
removed from configs.
Related to #4895
This copies over a number of features flags and other settings from
test/config-next that have been applied in prod.
Also, remove the config-next gate on various tests.
And use it in ocsp-updater. This was cleaned up in #4546 because it was
unused, but it should have been in use in ocsp-updater now that we can
make a straightforward query here instead of a JOIN.
This makes the SA the single source of truth for what columns are in the
certificateStatus table.
This commit consists of three classes of changes:
1) Changing various command main.go files to always behave as they
would have when features.BlockedKeyTable was true. Also changing
one test in the same manner.
2) Removing the BlockedKeyTable flag from configuration in config-next,
because the flag is already live.
3) Moving the BlockedKeyTable flag to the "deprecated" section of
features.go, and regenerating featureflag_strings.go.
A future change will remove the BlockedKeyTable flag (and other
similarly deprecated flags) from features.go entirely.
Fixes#4873
Now that the migration has been applied, we can reference the issuerID
field unconditionally. Also remove the migration file. It had already
been copied to sa/_db/migrations, but not removed from _db-next.
Part of a multi-PR changeset removing the StoreIssuerInfo flag.
There are some changes to the code generated in the latest version, so
this modifies every .pb.go file.
Also, the way protoc-gen-go decides where to put files has changed, so
each generate.go gets the --go_opt=paths=source_relative flag to
tell protoc to continue placing output next to the input.
Remove staticcheck from build.sh; we get it via golangci-lint now.
Pass --no-document to gem install fpm; this is recommended in the fpm docs.
Adds a daemon which monitors the new blockedKeys table and checks for any unexpired, unrevoked certificates that are associated with the added SPKI hashes and revokes them, notifying the user that issued the certificates.
Fixes#4772.
In addition to base64(sha256(spki)).
As part of that, change KeyDigest to return [32]byte, and add KeyDigestB64 which provides the base64-encoded output that KeyDigest used to provide. Also update all call sites.
This cleans up after the authzv2 migration and makes names a little
easier to read, since there is no longer a v1/v2 distinction. This
leaves the names of tables the same since they would require a migration
to change.
Aaron Gable