Support for CPS URLs in Custom Policy Identifiers. #15751
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
For something that doesn't have breaking tests (needs to use a second parameter): |
| if err == nil { | ||
| certTemplate.PolicyIdentifiers = append(certTemplate.PolicyIdentifiers, oid) | ||
| } | ||
| if err != nil { | ||
| oidOnly = false |
There was a problem hiding this comment.
I may be way off the mark here but I would have expected us to store non-oid parseable values into a separate list that would then be fed into the CreatePolicyInformationExtensionFromStorageStrings call below?
Is there not a chance that we could have some of the same elements now within certTemplate.PolicyIdentifiers and within certTemplate.ExtraExtensions?
There was a problem hiding this comment.
Theoretically (though not practically) possible. If someone had in their policy identifier extensions, before validation was added, the fields {"oid":"2.5.1.4.8"}, and managed to build this with an old version of go, that might be trimmed to numbers, and possibly successfully parsed as a oid, and could be successfully parsed as a policy json object now.
It's not a problem if some of the same elements are in the certTemplate.PolicyIdentifiers and certTemplate.ExtraExtensions[policyInformationExtension]. Everything in certTemplate.PolicyIdentifiers is overwritten when there is a policy extension in extra extensions. Since the extra extension is only created if it can parse all the policy identifiers, it will include everything in certTemplate.PolicyIdentifiers (no one will look there and be misled).
Fix intial nil identifiers. Co-authored-by: Steven Clark <[email protected]>
sgmiller
approved these changes
Jun 3, 2022
| certificateB64 := make([]byte, len(certificateAsn)*2) | ||
| base64.StdEncoding.Encode(certificateB64, certificateAsn) | ||
| certificateString := string(certificateB64[:]) | ||
| assert.Contains(t, certificateString, testCase.ASN) |
There was a problem hiding this comment.
Appreciate the full round trip here!
kitography
deleted the
VAULT-5436-pki-support-for-cps-url-in-custom-policy-identifiers
branch
swenson
pushed a commit
to hashicorp/terraform-provider-vault
that referenced
this pull request
Jun 9, 2022
Update the `vault_resource_pki_secret_backend_role` to support specifying the CPS URL when specifying policy identifiers in line with the recent changes to the PKI Secrets Engine in Vault 1.11: hashicorp/vault#15751 We do this by deprecating the existing `policy_identifiers` argument and creating a new block, `policy_identifier`, which can be specified multiple times. If both `policy_identifiers` and `policy_identifier` blocks are present, then `policy_identifier` is ignored. (Otherwise, refreshing would delete one or the other, and the state wouldn't have round trip stability.) This was also tested locally with, for example, a terraform file like: ```hcl provider "vault" { } resource "vault_mount" "pki" { path = "pki" type = "pki" default_lease_ttl_seconds = 3600 max_lease_ttl_seconds = 86400 } resource "vault_pki_secret_backend_role" "role" { name = "example-dot-com" backend = vault_mount.pki.path allowed_domains = ["example.com"] allow_subdomains = true allow_bare_domains = true allow_glob_domains = true allow_ip_sans = true allow_localhost = "true" generate_lease = true organization = ["Hashi test"] country = ["USA"] locality = ["Area 51"] province = ["NV"] max_ttl = "720h" policy_identifiers = ["2.5.29.32","1.2.3"] // or policy_identifier { oid = "2.5.29.32" cps = "https://example.com/cps" notice = "Some notice" } policy_identifier { oid = "1.2.3" } } ```
swenson
pushed a commit
to hashicorp/terraform-provider-vault
that referenced
this pull request
Jun 9, 2022
Update the `vault_resource_pki_secret_backend_role` to support specifying the CPS URL when specifying policy identifiers in line with the recent changes to the PKI Secrets Engine in Vault 1.11: hashicorp/vault#15751 We do this by deprecating the existing `policy_identifiers` argument and creating a new block, `policy_identifier`, which can be specified multiple times. If both `policy_identifiers` and `policy_identifier` blocks are present, then `policy_identifier` is ignored. (Otherwise, refreshing would delete one or the other, and the state wouldn't have round trip stability.) This was also tested locally with, for example, a terraform file like: ```hcl provider "vault" { } resource "vault_mount" "pki" { path = "pki" type = "pki" default_lease_ttl_seconds = 3600 max_lease_ttl_seconds = 86400 } resource "vault_pki_secret_backend_role" "role" { name = "example-dot-com" backend = vault_mount.pki.path allowed_domains = ["example.com"] allow_subdomains = true allow_bare_domains = true allow_glob_domains = true allow_ip_sans = true allow_localhost = "true" generate_lease = true organization = ["Hashi test"] country = ["USA"] locality = ["Area 51"] province = ["NV"] max_ttl = "720h" policy_identifiers = ["2.5.29.32","1.2.3"] // or policy_identifier { oid = "2.5.29.32" cps = "https://example.com/cps" notice = "Some notice" } policy_identifier { oid = "1.2.3" } } ```
swenson
added a commit
to hashicorp/terraform-provider-vault
that referenced
this pull request
Jun 10, 2022
PKI: Add support for CPS URL in custom policy identifiers Update the `vault_resource_pki_secret_backend_role` to support specifying the CPS URL when specifying policy identifiers in line with the recent changes to the PKI Secrets Engine in Vault 1.11: hashicorp/vault#15751 We do this by deprecating the existing `policy_identifiers` argument and creating a new block, `policy_identifier`, which can be specified multiple times. If both `policy_identifiers` and `policy_identifier` blocks are present, then `policy_identifier` is ignored. (Otherwise, refreshing would delete one or the other, and the state wouldn't have round trip stability.) This was also tested locally with, for example, a terraform file like: ```hcl provider "vault" { } resource "vault_mount" "pki" { path = "pki" type = "pki" default_lease_ttl_seconds = 3600 max_lease_ttl_seconds = 86400 } resource "vault_pki_secret_backend_role" "role" { name = "example-dot-com" backend = vault_mount.pki.path allowed_domains = ["example.com"] allow_subdomains = true allow_bare_domains = true allow_glob_domains = true allow_ip_sans = true allow_localhost = "true" generate_lease = true organization = ["Hashi test"] country = ["USA"] locality = ["Area 51"] province = ["NV"] max_ttl = "720h" policy_identifiers = ["2.5.29.32","1.2.3"] // or policy_identifier { oid = "2.5.29.32" cps = "https://example.com/cps" notice = "Some notice" } policy_identifier { oid = "1.2.3" } } ```
marcboudreau
pushed a commit
to marcboudreau/terraform-provider-vault
that referenced
this pull request
Nov 6, 2022
…1495) PKI: Add support for CPS URL in custom policy identifiers Update the `vault_resource_pki_secret_backend_role` to support specifying the CPS URL when specifying policy identifiers in line with the recent changes to the PKI Secrets Engine in Vault 1.11: hashicorp/vault#15751 We do this by deprecating the existing `policy_identifiers` argument and creating a new block, `policy_identifier`, which can be specified multiple times. If both `policy_identifiers` and `policy_identifier` blocks are present, then `policy_identifier` is ignored. (Otherwise, refreshing would delete one or the other, and the state wouldn't have round trip stability.) This was also tested locally with, for example, a terraform file like: ```hcl provider "vault" { } resource "vault_mount" "pki" { path = "pki" type = "pki" default_lease_ttl_seconds = 3600 max_lease_ttl_seconds = 86400 } resource "vault_pki_secret_backend_role" "role" { name = "example-dot-com" backend = vault_mount.pki.path allowed_domains = ["example.com"] allow_subdomains = true allow_bare_domains = true allow_glob_domains = true allow_ip_sans = true allow_localhost = "true" generate_lease = true organization = ["Hashi test"] country = ["USA"] locality = ["Area 51"] province = ["NV"] max_ttl = "720h" policy_identifiers = ["2.5.29.32","1.2.3"] // or policy_identifier { oid = "2.5.29.32" cps = "https://example.com/cps" notice = "Some notice" } policy_identifier { oid = "1.2.3" } } ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allows a new method of adding Policy Information that enables the use of CPS URLs and (explicit text) user notices. This is done in one field (in the same field as previously).
Eg:
On the backend each policy_identifier is stored separately - those might be an oid or string_json form of policy identifier with qualifiers.