Skip to content
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

Allow old certs to be cross-signed #16494

Merged
merged 4 commits into from
Aug 3, 2022
Merged

Conversation

cipherboy
Copy link
Contributor

In Vault 1.11, we introduced cross-signing support, but the earlier SKID
field change in Vault 1.10 causes problems: notably, certs created on
older versions of Vault (<=1.9) or outside of Vault (with a different
SKID method) cannot be cross-signed and validated in OpenSSL.

In particular, OpenSSL appears to be unique in requiring a SKID/AKID
match for chain building. If AKID and SKID are present on an otherwise
valid client/parent cert pair and the values are different, OpenSSL will
not build a valid path over those two, whereas most other chain
validation implementations will.

Regardless, to have proper cross-signing support, we really aught to
support copying an SKID. This adds such support to the sign-intermediate
endpoint. Support for the /issue endpoint is not added, as cross-signing
leaf certs isn't generally useful and can accept random SKIDs.

Resolves: #16461

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

@cipherboy cipherboy added this to the 1.12.0-rc1 milestone Jul 28, 2022
@cipherboy cipherboy requested a review from a team July 28, 2022 20:41
@cipherboy
Copy link
Contributor Author

@dmitriy-moiseev -- do you want to test this to make sure this satisfies your use case? If you go to the GH test build -> summary page you can fetch a pre-built scratch binary if it is of interest.

@kitography kitography self-requested a review July 29, 2022 15:28
Copy link
Contributor

@kitography kitography left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good!

builtin/logical/pki/cert_util.go Outdated Show resolved Hide resolved
builtin/logical/pki/chain_test.go Show resolved Hide resolved
In Vault 1.11, we introduced cross-signing support, but the earlier SKID
field change in Vault 1.10 causes problems: notably, certs created on
older versions of Vault (<=1.9) or outside of Vault (with a different
SKID method) cannot be cross-signed and validated in OpenSSL.

In particular, OpenSSL appears to be unique in requiring a SKID/AKID
match for chain building. If AKID and SKID are present on an otherwise
valid client/parent cert pair and the values are different, OpenSSL will
not build a valid path over those two, whereas most other chain
validation implementations will.

Regardless, to have proper cross-signing support, we really aught to
support copying an SKID. This adds such support to the sign-intermediate
endpoint. Support for the /issue endpoint is not added, as cross-signing
leaf certs isn't generally useful and can accept random SKIDs.

Resolves: #16461

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-add-skid-cross-signing branch from 8f242ba to 9b684ab Compare August 1, 2022 12:41
@cipherboy cipherboy requested review from kitography, stevendpclark and a team August 1, 2022 12:41
@cipherboy
Copy link
Contributor Author

@kitography @stevendpclark -- updated!

Also adds a known-answer test using LE R3 CA's SKID.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-add-skid-cross-signing branch from 9b684ab to 45d678e Compare August 1, 2022 12:43
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, one minor nit/suggestion but I don't feel strongly about it.

builtin/logical/pki/cert_util.go Outdated Show resolved Hide resolved
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy enabled auto-merge (squash) August 3, 2022 13:18
@cipherboy cipherboy merged commit 74d68e2 into main Aug 3, 2022
@cipherboy cipherboy deleted the cipherboy-add-skid-cross-signing branch December 1, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pki: can't issue a valid cross-signed certificate if vault was too old (SKID calculation)
3 participants