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.
Running an older version (v0.0.1-2020.1.4) of `staticcheck` in
whole-program mode (`staticcheck --unused.whole-program=true -- ./...`)
finds various instances of unused code which don't normally show up
as CI issues. I've used this to find and remove a large chunk of the
unused code, to pave the way for additional large deletions accompanying
the WFE1 removal.
Part of #5681
In #4992, we refactored NewSigner to look keys up by public key and by
label. However, we didn't correctly incorporate the label check into
the new code. This fixes that and adds a test.
This moves x509Signer from cmd/ceremony into pkcs11helpers. It also
adds helper functions getPublicKeyID and getPrivateKey, copied and
adapted from pkcs11key. These act as counterparts to the existing
GetRSAPublicKey and GetECDSAPublicKey, which go from an object handle
to a Go public key object (and are used after key generation).
Fixes#4918
Previously we were relying on a "more" boolean returned from
FindObjects. But according to
https://pkg.go.dev/github.com/miekg/pkcs11?tab=doc#Ctx.FindObjects,
> The returned boolean value is deprecated and should be ignored.
Instead, we ask for more objects than we need and error if we get more
than 1.
Add a test, and in the process split up the relevant test into
multiple smaller test cases.
Merges gen-ca and gen-key into a single tool that can be used to complete a key/certificate generation ceremony. The driving idea here is that instead of having to write out multiple long commands in a specific order in order to complete a ceremony a configuration file is fed to a single binary. This config file contains all of the information needed to complete the ceremony, and can be easily tested outside of the secure environment before hand without fear of later typing a command/flag incorrectly etc.
The tooling works against the test hardware I have (there are minimal changes to the actual PKCS#11 code behind the scenes). Specific attention should be given to the documentation, and the general UX of the tool.
Fixes#4639 and fixes#4667.
Tested against relevant hardware for generating both RSA and ECDSA roots and intermediates with keys generated using `gen-key`.
Also this makes a few changes to the `gen-key` tool after further experience with the HSM and more reading of the PCKS#11 specification. Main change is the removal of `compatMode`, which was intended to provide support for two naming schemes for EC used in subsequent PKCS#11 drafts. It turns out these schemes were changes in name only and the underlying structs/ints were the exact same (i.e. `CKA_ECDSA_PARAMS == CKA_EC_PARAMS` and `CKM_ECDSA_KEY_PAIR_GEN == CKM_EC_KEY_PAIR_GEN`) and just allowed using one of the two names based on preference. This meant with `compatMode` enabled or disabled the tool did the exact same thing.
Fixes#3697.
James Renken
Matthew McPherrin
Aaron Gable
Jacob Hoffman-Andrews
Roland Bracewell Shoemaker
Roland Bracewell Shoemaker