admin: revoke certs or block keys based on cert PEM #7431
Conversation
aarongable
requested review from
beautifulentropy,
jsha and
pgporada
and removed request for
beautifulentropy
There was a problem hiding this comment.
Looks good in general. The original commented-out line talked about multiple certificates in a single PEM file, but the implementation here supports only one. Did you decide that's not a use case worth supporting? I don't have a strong feeling either way, though probably the flag docs should say explicitly that only a single cert per file is allowed (and maybe we should error if there's extra junk after that first cert).
cmd/admin/key.go
Outdated
| @@ -43,6 +44,7 @@ func (s *subcommandBlockKey) Flags(flag *flag.FlagSet) { | |||
| // Flags specifying the input method for the keys to be blocked. | |||
| flag.StringVar(&s.privKey, "private-key", "", "Block issuance for the pubkey corresponding to this private key") | |||
| flag.StringVar(&s.spkiFile, "spki-file", "", "Block issuance for all keys listed in this file as SHA256 hashes of SPKI, hex encoded, one per line") | |||
| flag.StringVar(&s.certFile, "cert-file", "", "Block issuance for the public key of the PEM-formatted certificate in this file") | |||
There was a problem hiding this comment.
It's potentially confusing to have -cert-file do a different thing for the block-key subcommand vs the revoke-cert subcommand. How about -key-from-cert-file?
There was a problem hiding this comment.
It feels (to me) like it does the same thing? In both, it's "extract the necessary information from the PEM certificate in this file". It's the subcommand that's doing something different, not the flag, if that makes sense?
Naming it -key-from-cert-file makes it feel like all the others should be -serials-from-text-file to me?
Yeah. Nowhere else in Boulder do we load multiple PEM blocks from a single file, and I didn't think it was worth reinventing the wheel here. It seems unlikely to me that we'll be in a situation where it's easier to concatenate a bunch of cert PEMs together into a file than... have any other way to list the things we want to block/revoke.
Updated! |
Boulder can though for creating a CA root store in several places such as here and the mailer. I went through this discovery last week and was surprised. |
Unfortunately |
Add a new "-cert-file" input mode to both `admin revoke-cert` and `admin block-key` which operates on the serial or pubkey found in the PEM-encoded certificate in the supplied file. Fixes letsencrypt#7267
Add a new "-cert-file" input mode to both
admin revoke-certandadmin block-keywhich operates on the serial or pubkey found in the PEM-encoded certificate in the supplied file.Fixes #7267