Commit Graph

13 Commits

Author SHA1 Message Date
Jacob Hoffman-Andrews 692bd53ae5
ca: unsplit issuance flow (#8014)
Add a new RPC to the CA: `IssueCertificate` covers issuance of both the
precertificate and the final certificate. In between, it calls out to
the RA's new method `GetSCTs`.

The RA calls the new `CA.IssueCertificate` if the `UnsplitIssuance`
feature flag is true.

The RA had a metric that counted certificates by profile name and hash.
Since the RA doesn't receive a profile hash in the new flow, simply
record the total number of issuances.

Fixes https://github.com/letsencrypt/boulder/issues/7983
2025-02-24 11:37:17 -08:00
Aaron Gable ff851f7107
WFE: Include profile name in returned Order json (#7626)
Integration testing revealed that the WFE was not rendering the profile
name in the Order JSON object. Fix the one spot where it was missed.

Part of https://github.com/letsencrypt/boulder/issues/7332
2024-07-24 14:30:24 -07:00
Aaron Gable b92581d620
Better compile-time type checking for gRPC server implementations (#7504)
Replaced our embeds of foopb.UnimplementedFooServer with
foopb.UnsafeFooServer. Per the grpc-go docs this reduces the "forwards
compatibility" of our implementations, but that is only a concern for
codebases that are implementing gRPC interfaces maintained by third
parties, and which want to be able to update those third-party
dependencies without updating their own implementations in lockstep.
Because we update our protos and our implementations simultaneously, we
can remove this safety net to replace runtime type checking with
compile-time type checking.

However, that replacement is not enough, because we never pass our
implementation objects to a function which asserts that they match a
specific interface. So this PR also replaces our reflect-based unittests
with idiomatic interface assertions. I do not view this as a perfect
solution, as it relies on people implementing new gRPC servers to add
this line, but it is no worse than the status quo which relied on people
adding the "TestImplementation" test.

Fixes https://github.com/letsencrypt/boulder/issues/7497
2024-05-28 09:26:29 -07:00
Aaron Gable e05d47a10a
Replace explicit int loops with range-over-int (#7434)
This adopts modern Go syntax to reduce the chance of off-by-one errors
and remove unnecessary loop variable declarations.

Fixes https://github.com/letsencrypt/boulder/issues/7227
2024-04-22 10:34:51 -07:00
Jacob Hoffman-Andrews 521eb55d1e
test: better message for different empty slices (#6920)
Given two empty slices, one that is equal to nil and one that is not,
AssertDeepEquals used to produce this confusing output:

    [[]] !(deep)= [[]]

After this change, it produces:

    [[]string(nil)] !(deep)= [[]string{}]
2023-05-26 09:41:23 -07:00
Aaron Gable 9262ca6e3f
Add grpc implementation tests to all services (#6782)
As a follow-up to #6780, add the same style of implementation test to
all of our other gRPC services. This was not included in that PR just to
keep it small and single-purpose.
2023-03-31 09:52:26 -07:00
Aaron Gable 0d0116dd3f
Implement GetSerialMetadata on StorageAuthorityRO (#6780)
When external clients make POST requests to our ARI endpoint, they're
getting 404s even when a GET request with the same exact CertID
succeeds. Logs show that this is because the SA is returning "method
GetSerialMetadata not implemented" when the WFE attempts that gRPC
request. This is due to an oversight: the GetSerialMetadata method is
not implemented on the SQLStorageAuthorityRO object, only on the
SQLStorageAuthority object. The unit tests did not catch this bug
because they supply a mock SA, which does implement the method in
question.

Update the receiver and add a wrapper so that GetSerialMetadata is
implemented on both the read-write and read-only SA implementation
types. Add a new kind of test assertion which helps ensure this won't
happen again. Add a TODO for an integration test covering the ARI POST
codepath to prevent a regression.

Fixes #6778
2023-03-30 12:32:14 -07:00
Phil Porada fdb9c543b7
Remove ReuseValidAuthz code (#6686)
Removes all code related to the `ReuseValidAuthz` feature flag. The
Boulder default is to now always reuse valid authorizations.

Fixes a panic in `test.AssertErrorIs` when `err` is unexpectedly `nil`
that was found this while reworking the
`TestPerformValidationAlreadyValid` test. The go stdlib `func Is`[1]
does not check for this.

1. https://go.dev/src/errors/wrap.go

Part 2/2, fixes https://github.com/letsencrypt/boulder/issues/2734
2023-02-28 17:57:16 -05:00
Phil Porada c0e158ed93
Limit input fields during new authz creation in sa.NewOrderAndAuthz (#6622)
A `core.Authorization` object has lots of fields (e.g. `status`, 
`attempted`, `attemptedAt`) which are not relevant to a 
newly-created authorization: a brand new authz can only be in 
the "pending" state, cannot have been attempted already or have 
been validated.

Fix a nil pointer dereference in `sa.NewOrderAndAuthzs` if a 
`req *sapb.NewOrderAndAuthzsRequest` is passed into the 
function with an inner nil `req.NewOrder`.

Add new tests. 
- TestNewOrderAndAuthzs_MissingInnerOrder 
  - Checks that
the nil pointer dereference no longer occurs 
- TestNewOrderAndAuthzs_NewAuthzExpectedFields 
  - Checks that the `Attempted`, `AttemptedAt`, `ValidationRecords`,
     and `ValidationErrors` fields for a brand new authz in the 
    `pending` state are correctly defaulted to `nil` in 
    `sa.NewOrderAndAuthzs`.

Add a new test assertion `AssertBoxedNil` that returns true for the
existence of a "boxed nil" - a nil value wrapped in a non-nil interface
type.

Fixes #6535

---------

Co-authored-by: Samantha <hello@entropy.cat>
2023-02-03 15:38:51 -05:00
Aaron Gable 4ad66729d2
Tests: use reflect.IsNil() to avoid boxed nil issues (#6305)
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
2022-08-19 14:47:34 -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 f5769c0967
Fix comment on AssertMetricWithLabelsEquals (#6099)
Also tag it as a helper.
2022-05-10 15:52:19 -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