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 issuer usage restrictions #15255

Merged

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented May 2, 2022

Built on top of #15254; will be rebased when that merges.

This handles adding usage restrictions to issuers, limiting if they can be used for signing certs and CRLs (separately).

We updated the legacy bundle shim (showing it can be used for both), the issuer update/fetch paths, and mostly enforce the constraint in fetchCAInfo. When fetching a cert (not for signing certs or CRLs), we use false, false as arguments.

I'm wondering if we want to make this an actual usage vector set? Or if separate flags are better? Seems like its all just about the same amount of work, minus the arguments to fetchCAInfo, which is an internal function.

@cipherboy cipherboy force-pushed the cipherboy-fix-delete-stuff branch from 8967ca9 to 75632ef Compare May 2, 2022 15:43
@cipherboy cipherboy force-pushed the cipherboy-add-issuer-usage-restrictions branch from b0f1641 to 19faacd Compare May 2, 2022 15:44
@cipherboy cipherboy changed the base branch from cipherboy-fix-delete-stuff to pki-pod-rotation May 3, 2022 12:42
@cipherboy cipherboy force-pushed the cipherboy-add-issuer-usage-restrictions branch from 19faacd to 0456808 Compare May 3, 2022 12:42
@cipherboy cipherboy marked this pull request as ready for review May 3, 2022 12:42
builtin/logical/pki/backend_test.go Outdated Show resolved Hide resolved
builtin/logical/pki/path_fetch_issuers.go Outdated Show resolved Hide resolved
@cipherboy cipherboy force-pushed the cipherboy-add-issuer-usage-restrictions branch from 0456808 to ae9ff5c Compare May 3, 2022 16:14
@cipherboy
Copy link
Contributor Author

Testing:

source devvault
vault secrets enable pki
vault write pki/root/generate/internal common_name="root x1" issuer_name=x1
vault write pki/root/generate/internal common_name="root x2" issuer_name=x2

vault write pki/issuer/default issuer_name=x1 usage="read only,issuing certificates"
vault write pki/roles/testing enforce_hostnames=false client_flag=true allow_any_name=true require_cn=false organization=hashicorp key_type=rsa
vault write pki/issue/testing common_name=localhost ttl=1s

vault write pki/issuer/default issuer_name=x1 usage="read only"
vault write pki/issue/testing common_name=localhost ttl=1s # should fail

@cipherboy cipherboy force-pushed the cipherboy-add-issuer-usage-restrictions branch from ae9ff5c to 36d31c0 Compare May 3, 2022 19:44
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.

Small nit on docs, but 👍

builtin/logical/pki/path_fetch_issuers.go Outdated Show resolved Hide resolved
This allows issuers to have usage restrictions, limiting whether they
can be used to issue certificates or if they can generate CRLs. This
allows certain issuers to not generate a CRL (if the global config is
with the CRL enabled) or allows the issuer to not issue new certificates
(but potentially letting the CRL generation continue).

Setting both fields to false effectively forms a soft delete capability.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-add-issuer-usage-restrictions branch from 36d31c0 to 15bc598 Compare May 3, 2022 20:40
@cipherboy cipherboy merged commit 15bc598 into pki-pod-rotation May 3, 2022
@cipherboy
Copy link
Contributor Author

Thank you!

@cipherboy cipherboy deleted the cipherboy-add-issuer-usage-restrictions branch May 17, 2022 14:34
@cipherboy
Copy link
Contributor Author

This PR was merged in #15277. See that PR and the relevant docs PR #15238 for more information about this change.

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.

3 participants