New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check cnf claim with CSR or SSH public key fingerprint #1660
base: master
Are you sure you want to change the base?
Conversation
This commit allows tying tokens with the provided CSR or SSH public key. Tokens with a confirmation claim kid (cnf.kid) will validate that the provided fingerprint (kid) matches the CSR or SSH public key. This check will only be present in JWK and X5C provisioners. Fixes #1637
// fingerprintValidator is a CertificateRequestValidator that checks the | ||
// fingerprint of the certificate with the provided one. | ||
type fingerprintValidator string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// fingerprintValidator is a CertificateRequestValidator that checks the | |
// fingerprint of the certificate with the provided one. | |
type fingerprintValidator string | |
// fingerprintValidator is a CertificateRequestValidator that checks the | |
// fingerprint of the certificate request with the provided one. | |
type fingerprintValidator string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, to clarify what it's operating on, maybe this can be named csrFingerprintValidator
or certificateRequestFingerprintValidator
.
// sshFingerprintValidator is a SSHPublicKeyValidator that checks the | ||
// fingerprint of the public key with the provided one. | ||
type sshFingerprintValidator string | ||
|
||
func (s sshFingerprintValidator) Valid(key ssh.PublicKey) error { | ||
if s != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little surprising to see the SSH fingerprint being a slightly different thing than an X509 fingerprint. I know we're using fingerprint for both full certificate hashes as well as derivations of just the public key in several places already, but in this case different logic seems to be used for a single functionality, whereas for the other places we refer to fingerprints, the context makes usage more clear.
IMO it would be better if we were to operate on the canonical contents of an SSH certificate request instead, so that the semantics are more similar for these checks. That would also keep the option open for an actual public key equality validator. It would require us to define that canonical format, because I think there's no existing definition for it, except for the final certificates.
IMO it's an option to skip (temporarily, or indefinitely) the SSH part of this PR if we don't find a good way for the SSH certificate request representation. I think that's preferred over having this logic with different semantics.
This commit allows tying tokens with the provided CSR or SSH public key. Tokens with a confirmation claim kid (
cnf.kid
) will validate that the provided fingerprint (kid) matches the CSR or SSH public key.This check will only be present in JWK and X5C provisioners.
Fixes #1637
Related PRs:
cnf
claim cli#1092