-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Support for CPS URLs in Custom Policy Identifiers. #15751
Support for CPS URLs in Custom Policy Identifiers. #15751
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <steven.clark@hashicorp.com>
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.
Looks good to me!
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the full round trip here!
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.
👍
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" } } ```
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" } } ```
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" } } ```
…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" } } ```
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.