add certificate extract command

[6293]

Fixes #487

Description

Add step certificate extract command to extract a certificate and private key from a .p12 file, with corresponding integration tests.

[CLAassistant]

All committers have signed the CLA.

[dopey]

Hey @z8674558 thanks for the PR! Love that you added unit tests. You'll notice that we're not great at that ourselves.

The first bit of feedback is that we'd like this command to be under the step certificate format command group. So p12 would be an option under format (the way that PEM and DER are). Is that a change you wouldn't mind making?

[6293]

hi @dopey, that makes sense. I updated my code.

do we want to also move step certificate p12 command to format?

[6293]

To avoid breaking backward compatibility, I added --crt, --key, --ca option to format command. Presence of these options means that the positional argument is .p12 file.

[maraino]

We don't want to move step certificate p12 but we might want to add support for it in format too. My proposal would be something like:

# Current functionality:
## Format PEM to DER
step certificate format internal.pem [-out internal.der]
## Format DER to PEM
step certificate format internal.der [-out internal.pem]

# New functionality:
## Format PEM to P12
## We can default to p12 if --key or --ca are passed
step certificate format internal.pem [--format p12] --key internal.key --ca ca.pem [-out internal.p12]
## Format P12 to PEM
## We can default to PEM if the input is p12
### Print crt, ca, and key to stdout or file (in that order)
step certificate format internal.p12 [--format pem] [--out internal.pem]
### Print crt, ca, and key to different files
step certificate format internal.p12 --crt internal.pem --key internal.key --ca ca.pem
step certificate format internal.p12 --out internal.pem --key internal.key --ca ca.pem
### If this is confusing we might want to do
step certificate format internal.p12 --out internal.pem --out-key internal.key --out-ca ca.pem

If you don't pass the out it will print in standard out. And of course, you can also do --format der, but in this case, you cannot print to stdout multiple files, just one.

@mmalone @dopey @z8674558 What do you think about this?

[6293]

@maraino Looks nice, I updated my code. One concern from me is that this approach makes hard to print a single file to stdout. e.g. one have to do the following to print only a certificate from a p12 file:

step certificate format foo.p12 --out-key /tmp/out.key --out-ca /tmp/out.ca

I would propose that instead of printing to stdout when the flags are absent, we allow empty value in the flags. In the above example,

step certificate format foo.p12 --out

will print the certificate to stdout, ignoring the key and the intermediate CAs. With this approach, And of course, you can also do --format der, but in this case, you cannot print to stdout multiple files, just one. would be simpler, too.

[mmalone]

Yea, so, to state the obvious this is a little clunky because P12s can have CAs & keys while PEM/DER is typically just one cert. I think having both --out <file> and meaning "send everything to <file>" and --crt <crt-file> --key <key-file> --ca <ca-file> to split stuff out is fine.

If you combine these flags (e.g., --out foo --key bar --ca baz I'd be ok with either 1) failing with an error (--out is incompatible with the other three flags) or writing everything to foo (crt, key, ca) and writing the key to bar and the CA to baz. In other words, it feels more reasonable to keep the semantics of --out <file> as "write everything you have to file" even when combined with --crt, --key, or --ca rather than changing the semantics of the --out flag depending on whether those other flags are present.

@z8674558, for your use case (writing an individual file to stdout) could we support - as a value to the various flags. For example, step certificate format foo.p12 --crt - would write just the key to stdout? Or is that weird? Does - usually mean "read from stdin" or can it also mean "write to stdout"?

[6293]

@mmalone Thank you for the insight.

If you combine these flags (e.g., --out foo --key bar --ca baz I'd be ok with either 1) failing with an error (--out is incompatible with the other three flags) or writing everything to foo (crt, key, ca) and writing the key to bar and the CA to baz. In other words, it feels more reasonable to keep the semantics of --out as "write everything you have to file" even when combined with --crt, --key, or --ca rather than changing the semantics of the --out flag depending on whether those other flags are present.

I adopted the strategy 1) for I don't think many people want to save crt, key, and ca into a single file. And they can just cat when they want to.

@z8674558, for your use case (writing an individual file to stdout) could we support - as a value to the various flags. For example, step certificate format foo.p12 --crt - would write just the key to stdout? Or is that weird? Does - usually mean "read from stdin" or can it also mean "write to stdout"?

This looks nice and I tried it. It turned out, however, urfave/cli does not support a hyphen right after an option flag (this is a bug, see urfave/cli#1092). Maybe we have to get by with the workaround for now.

[dopey]

Hey @z8674558 we really appreciate the contribution!

We're still reviewing, but in the mean time we wanted to send you some swag to show our appreciation. If you're interested, ping me ([email protected]). Cheers!

[dopey]

Let's leave the first arg as <crt-file>. That name is consistent across the other commands, and I'm changing the other commands to use - instead of _.

[6293]

done

[dopey]

No description provided.

[dopey]

Created a new branch / PR because I wanted to commit some documentation changes - #589

[dopey]

Going to close this one in favor of the other if that's all right. Let me know if I should reopen