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

Add BYOC-based revocation to PKI secrets engine #16564

Merged
merged 7 commits into from
Aug 15, 2022

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Aug 3, 2022

This PR adds support for post-hoc revocation of certificates either signed externally (prior to Vault owning the CA) or of certificates issued from a no_store=true role. This allows all certificates to be revoked, regardless of origin.

This is done by specifying the certificate parameter (taking a PEM-encoded certificate) on the /revoke API endpoint, instead of the serial_number argument. Vault verifies this against the mount's issuers and adds it to the local storage before also revoking it.

Currently there is no way to import active certificates for storage without revoking them as well.


  • Needs magical length check.
  • Tests for BYOC revocation
  • Change log entry
Siteproxy
@cipherboy cipherboy added this to the 1.12.0-rc1 milestone Aug 3, 2022
@cipherboy cipherboy requested review from kitography, stevendpclark and a team August 3, 2022 20:26
@cipherboy cipherboy requested a review from taoism4504 as a code owner August 3, 2022 20:26
@cipherboy cipherboy changed the title Add BYOC-based revocation Add BYOC-based revocation to PKI secrets engine Aug 3, 2022
@cipherboy cipherboy force-pushed the cipherboy-add-byoc-revocation branch from bdd24d3 to b553c3f Compare August 3, 2022 20:37
@cipherboy cipherboy force-pushed the cipherboy-add-byoc-revocation branch from b553c3f to 128978c Compare August 4, 2022 17:15
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, one small nit/question

builtin/logical/pki/path_revoke.go Show resolved Hide resolved
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This allows operators to revoke certificates via a PEM blob passed to
Vault. In particular, Vault verifies the signature on the certificate
from an existing issuer within the mount, ensuring that one indeed
issued this certificate. The certificate is then added to storage and
its serial submitted for revocation.

This allows certificates generated with no_store=true to be submitted
for revocation afterwards, given a full copy of the certificate. As a
consequence, all roles can now safely move to no_store=true (if desired
for performance) and revocation can be done on a case-by-case basis.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This prevents usage of the BYOC cert on a hybrid 1.10/1.12 cluster with
an non-upgraded CA issuer bundle.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-add-byoc-revocation branch from 128978c to 1385e44 Compare August 15, 2022 13:37
@cipherboy
Copy link
Contributor Author

@stevendpclark Updated!

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.

👍

@cipherboy cipherboy enabled auto-merge (squash) August 15, 2022 13:42
@cipherboy cipherboy merged commit 23ad52f into main Aug 15, 2022
@cipherboy cipherboy deleted the cipherboy-add-byoc-revocation branch December 1, 2022 14:58
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.

None yet

2 participants