From 262f023e706eccf2054625b07f15bd892b58a6cf Mon Sep 17 00:00:00 2001 From: Kit Haines Date: Fri, 3 Jun 2022 14:50:46 -0400 Subject: [PATCH] Support for CPS URLs in Custom Policy Identifiers. (#15751) * Support for CPS URLs in Custom Policy Identifiers. * go fmt * Add Changelog * Fix panic in test-cases. * Update builtin/logical/pki/path_roles.go Fix intial nil identifiers. Co-authored-by: Steven Clark * Make valid policy OID so don't break ASN parse in test. * Add test cases. * go fmt. Co-authored-by: Steven Clark --- builtin/logical/pki/path_roles.go | 61 ++++++++-- builtin/logical/pki/path_roles_test.go | 148 ++++++++++++++++++++++++- changelog/15751.txt | 3 + sdk/helper/certutil/helpers.go | 22 +++- sdk/helper/certutil/types.go | 114 +++++++++++++++++++ 5 files changed, 332 insertions(+), 16 deletions(-) create mode 100644 changelog/15751.txt diff --git a/builtin/logical/pki/path_roles.go b/builtin/logical/pki/path_roles.go index c057c9aa88..da446d65d1 100644 --- a/builtin/logical/pki/path_roles.go +++ b/builtin/logical/pki/path_roles.go @@ -3,6 +3,7 @@ package pki import ( "context" "crypto/x509" + "encoding/json" "fmt" "strings" "time" @@ -384,8 +385,10 @@ for "generate_lease".`, }, "policy_identifiers": { - Type: framework.TypeCommaStringSlice, - Description: `A comma-separated string or list of policy OIDs.`, + Type: framework.TypeCommaStringSlice, + Description: `A comma-separated string or list of policy OIDs, or a JSON list of qualified policy +information, which must include an oid, and may include a notice and/or cps url, using the form +[{"oid"="1.3.6.1.4.1.7.8","notice"="I am a user Notice"}, {"oid"="1.3.6.1.4.1.44947.1.2.4 ","cps"="https://example.com"}].`, }, "basic_constraints_valid_for_non_ca": { @@ -658,7 +661,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data NoStore: data.Get("no_store").(bool), RequireCN: data.Get("require_cn").(bool), AllowedSerialNumbers: data.Get("allowed_serial_numbers").([]string), - PolicyIdentifiers: data.Get("policy_identifiers").([]string), + PolicyIdentifiers: getPolicyIdentifier(data, nil), BasicConstraintsValidForNonCA: data.Get("basic_constraints_valid_for_non_ca").(bool), NotBeforeDuration: time.Duration(data.Get("not_before_duration").(int)) * time.Second, NotAfter: data.Get("not_after").(string), @@ -747,11 +750,9 @@ func validateRole(b *backend, entry *roleEntry, ctx context.Context, s logical.S } if len(entry.PolicyIdentifiers) > 0 { - for _, oidstr := range entry.PolicyIdentifiers { - _, err := certutil.StringToOid(oidstr) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("%q could not be parsed as a valid oid for a policy identifier", oidstr)), nil - } + _, err := certutil.CreatePolicyInformationExtensionFromStorageStrings(entry.PolicyIdentifiers) + if err != nil { + return nil, err } } @@ -848,7 +849,7 @@ func (b *backend) pathRolePatch(ctx context.Context, req *logical.Request, data NoStore: getWithExplicitDefault(data, "no_store", oldEntry.NoStore).(bool), RequireCN: getWithExplicitDefault(data, "require_cn", oldEntry.RequireCN).(bool), AllowedSerialNumbers: getWithExplicitDefault(data, "allowed_serial_numbers", oldEntry.AllowedSerialNumbers).([]string), - PolicyIdentifiers: getWithExplicitDefault(data, "policy_identifiers", oldEntry.PolicyIdentifiers).([]string), + PolicyIdentifiers: getPolicyIdentifier(data, &oldEntry.PolicyIdentifiers), BasicConstraintsValidForNonCA: getWithExplicitDefault(data, "basic_constraints_valid_for_non_ca", oldEntry.BasicConstraintsValidForNonCA).(bool), NotBeforeDuration: getTimeWithExplicitDefault(data, "not_before_duration", oldEntry.NotBeforeDuration), NotAfter: getWithExplicitDefault(data, "not_after", oldEntry.NotAfter).(string), @@ -1116,3 +1117,45 @@ const pathListRolesHelpDesc = `Roles will be listed by the role name.` const pathRoleHelpSyn = `Manage the roles that can be created with this backend.` const pathRoleHelpDesc = `This path lets you manage the roles that can be created with this backend.` + +const policyIdentifiersParam = "policy_identifiers" + +func getPolicyIdentifier(data *framework.FieldData, defaultIdentifiers *[]string) []string { + policyIdentifierEntry, ok := data.GetOk(policyIdentifiersParam) + if !ok { + // No Entry for policy_identifiers + if defaultIdentifiers != nil { + return *defaultIdentifiers + } + return data.Get(policyIdentifiersParam).([]string) + } + // Could Be A JSON Entry + policyIdentifierJsonEntry := data.Raw[policyIdentifiersParam] + policyIdentifierJsonString, ok := policyIdentifierJsonEntry.(string) + if ok { + policyIdentifiers, err := parsePolicyIdentifiersFromJson(policyIdentifierJsonString) + if err == nil { + return policyIdentifiers + } + } + // Else could Just Be A List of OIDs + return policyIdentifierEntry.([]string) +} + +func parsePolicyIdentifiersFromJson(policyIdentifiers string) ([]string, error) { + var entries []certutil.PolicyIdentifierWithQualifierEntry + var policyIdentifierList []string + err := json.Unmarshal([]byte(policyIdentifiers), &entries) + if err != nil { + return policyIdentifierList, err + } + policyIdentifierList = make([]string, 0, len(entries)) + for _, entry := range entries { + policyString, err := json.Marshal(entry) + if err != nil { + return policyIdentifierList, err + } + policyIdentifierList = append(policyIdentifierList, string(policyString)) + } + return policyIdentifierList, nil +} diff --git a/builtin/logical/pki/path_roles_test.go b/builtin/logical/pki/path_roles_test.go index c7035adadb..97c465b27c 100644 --- a/builtin/logical/pki/path_roles_test.go +++ b/builtin/logical/pki/path_roles_test.go @@ -2,13 +2,19 @@ package pki import ( "context" + "crypto/x509" + "encoding/asn1" + "encoding/base64" + "encoding/pem" "fmt" "testing" "time" + "github.com/go-errors/errors" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -928,8 +934,8 @@ func TestPki_RolePatch(t *testing.T) { }, { Field: "policy_identifiers", - Before: []string{"1.2.3.4.5"}, - Patched: []string{"5.4.3.2.1"}, + Before: []string{"1.3.6.1.4.1.1.1"}, + Patched: []string{"1.3.6.1.4.1.1.2"}, }, { Field: "basic_constraints_valid_for_non_ca", @@ -1019,3 +1025,141 @@ func TestPki_RolePatch(t *testing.T) { } } } + +func TestPKI_RolePolicyInformation_Flat(t *testing.T) { + type TestCase struct { + Input interface{} + ASN interface{} + OidList []string + } + + expectedSimpleAsnExtension := "MBYwCQYHKwYBBAEBATAJBgcrBgEEAQEC" + expectedSimpleOidList := append(*new([]string), "1.3.6.1.4.1.1.1", "1.3.6.1.4.1.1.2") + + testCases := []TestCase{ + { + Input: "1.3.6.1.4.1.1.1,1.3.6.1.4.1.1.2", + ASN: expectedSimpleAsnExtension, + OidList: expectedSimpleOidList, + }, + { + Input: "[{\"oid\":\"1.3.6.1.4.1.1.1\"},{\"oid\":\"1.3.6.1.4.1.1.2\"}]", + ASN: expectedSimpleAsnExtension, + OidList: expectedSimpleOidList, + }, + { + Input: "[{\"oid\":\"1.3.6.1.4.1.7.8\",\"notice\":\"I am a user Notice\"},{\"oid\":\"1.3.6.1.44947.1.2.4\",\"cps\":\"https://example.com\"}]", + ASN: "MF8wLQYHKwYBBAEHCDAiMCAGCCsGAQUFBwICMBQMEkkgYW0gYSB1c2VyIE5vdGljZTAuBgkrBgGC3xMBAgQwITAfBggrBgEFBQcCARYTaHR0cHM6Ly9leGFtcGxlLmNvbQ==", + OidList: append(*new([]string), "1.3.6.1.4.1.7.8", "1.3.6.1.44947.1.2.4"), + }, + } + + b, storage := createBackendWithStorage(t) + + caData := map[string]interface{}{ + "common_name": "myvault.com", + "ttl": "5h", + "ip_sans": "127.0.0.1", + } + caReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: caData, + } + caResp, err := b.HandleRequest(context.Background(), caReq) + if err != nil || (caResp != nil && caResp.IsError()) { + t.Fatalf("bad: err: %v resp: %#v", err, caResp) + } + + for index, testCase := range testCases { + var roleResp *logical.Response + var issueResp *logical.Response + var err error + + // Create/update the role + roleData := map[string]interface{}{} + roleData[policyIdentifiersParam] = testCase.Input + + roleReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/testrole", + Storage: storage, + Data: roleData, + } + + roleResp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (roleResp != nil && roleResp.IsError()) { + t.Fatalf("bad [%d], setting policy identifier %v err: %v resp: %#v", index, testCase.Input, err, roleResp) + } + + // Issue Using this role + issueData := map[string]interface{}{} + issueData["common_name"] = "localhost" + issueData["ttl"] = "2s" + + issueReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "issue/testrole", + Storage: storage, + Data: issueData, + } + + issueResp, err = b.HandleRequest(context.Background(), issueReq) + if err != nil || (issueResp != nil && issueResp.IsError()) { + t.Fatalf("bad [%d], setting policy identifier %v err: %v resp: %#v", index, testCase.Input, err, issueResp) + } + + // Validate the OIDs + policyIdentifiers, err := getPolicyIdentifiersOffCertificate(*issueResp) + if err != nil { + t.Fatalf("bad [%d], getting policy identifier from %v err: %v resp: %#v", index, testCase.Input, err, issueResp) + } + if len(policyIdentifiers) != len(testCase.OidList) { + t.Fatalf("bad [%d], wrong certificate policy identifier from %v len expected: %d got %d", index, testCase.Input, len(testCase.OidList), len(policyIdentifiers)) + } + for i, identifier := range policyIdentifiers { + if identifier != testCase.OidList[i] { + t.Fatalf("bad [%d], wrong certificate policy identifier from %v expected: %v got %v", index, testCase.Input, testCase.OidList[i], policyIdentifiers[i]) + } + } + // Validate the ASN + certificateAsn, err := getPolicyInformationExtensionOffCertificate(*issueResp) + if err != nil { + t.Fatalf("bad [%d], getting extension from %v err: %v resp: %#v", index, testCase.Input, err, issueResp) + } + certificateB64 := make([]byte, len(certificateAsn)*2) + base64.StdEncoding.Encode(certificateB64, certificateAsn) + certificateString := string(certificateB64[:]) + assert.Contains(t, certificateString, testCase.ASN) + } +} + +func getPolicyIdentifiersOffCertificate(resp logical.Response) ([]string, error) { + stringCertificate := resp.Data["certificate"].(string) + block, _ := pem.Decode([]byte(stringCertificate)) + certificate, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, err + } + policyIdentifierStrings := make([]string, len(certificate.PolicyIdentifiers)) + for index, asnOid := range certificate.PolicyIdentifiers { + policyIdentifierStrings[index] = asnOid.String() + } + return policyIdentifierStrings, nil +} + +func getPolicyInformationExtensionOffCertificate(resp logical.Response) ([]byte, error) { + stringCertificate := resp.Data["certificate"].(string) + block, _ := pem.Decode([]byte(stringCertificate)) + certificate, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, err + } + for _, extension := range certificate.Extensions { + if extension.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 32}) { + return extension.Value, nil + } + } + return *new([]byte), errors.New("No Policy Information Extension Found") +} diff --git a/changelog/15751.txt b/changelog/15751.txt new file mode 100644 index 0000000000..d753db82d3 --- /dev/null +++ b/changelog/15751.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Add support for CPS URLs and User Notice to Policy Information +``` \ No newline at end of file diff --git a/sdk/helper/certutil/helpers.go b/sdk/helper/certutil/helpers.go index 27d056854c..457de9b022 100644 --- a/sdk/helper/certutil/helpers.go +++ b/sdk/helper/certutil/helpers.go @@ -446,18 +446,30 @@ func ParsePublicKeyPEM(data []byte) (interface{}, error) { return nil, errors.New("data does not contain any valid public keys") } -// addPolicyIdentifiers adds certificate policies extension -// +// AddPolicyIdentifiers adds certificate policies extension, based on CreationBundle func AddPolicyIdentifiers(data *CreationBundle, certTemplate *x509.Certificate) { - for _, oidstr := range data.Params.PolicyIdentifiers { - oid, err := StringToOid(oidstr) + oidOnly := true + for _, oidStr := range data.Params.PolicyIdentifiers { + oid, err := StringToOid(oidStr) if err == nil { certTemplate.PolicyIdentifiers = append(certTemplate.PolicyIdentifiers, oid) } + if err != nil { + oidOnly = false + } + } + if !oidOnly { // Because all policy information is held in the same extension, when we use an extra extension to + // add policy qualifier information, that overwrites any information in the PolicyIdentifiers field on the Cert + // Template, so we need to reparse all the policy identifiers here + extension, err := CreatePolicyInformationExtensionFromStorageStrings(data.Params.PolicyIdentifiers) + if err == nil { + // If this errors out, don't add it, rely on the OIDs parsed into PolicyIdentifiers above + certTemplate.ExtraExtensions = append(certTemplate.ExtraExtensions, *extension) + } } } -// addExtKeyUsageOids adds custom extended key usage OIDs to certificate +// AddExtKeyUsageOids adds custom extended key usage OIDs to certificate func AddExtKeyUsageOids(data *CreationBundle, certTemplate *x509.Certificate) { for _, oidstr := range data.Params.ExtKeyUsageOIDs { oid, err := StringToOid(oidstr) diff --git a/sdk/helper/certutil/types.go b/sdk/helper/certutil/types.go index 76587826ef..a5caa2e440 100644 --- a/sdk/helper/certutil/types.go +++ b/sdk/helper/certutil/types.go @@ -17,7 +17,10 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" + "encoding/json" "encoding/pem" + "errors" "fmt" "math/big" "net" @@ -894,3 +897,114 @@ func (p *KeyBundle) ToPrivateKeyPemString() (string, error) { return "", errutil.InternalError{Err: "No Private Key Bytes to Wrap"} } + +// PolicyIdentifierWithQualifierEntry Structure for Internal Storage +type PolicyIdentifierWithQualifierEntry struct { + PolicyIdentifierOid string `json:"oid",mapstructure:"oid"` + CPS string `json:"cps,omitempty",mapstructure:"cps"` + Notice string `json:"notice,omitempty",mapstructure:"notice"` +} + +// GetPolicyIdentifierFromString parses out the internal structure of a Policy Identifier +func GetPolicyIdentifierFromString(policyIdentifier string) (*PolicyIdentifierWithQualifierEntry, error) { + if policyIdentifier == "" { + return nil, nil + } + entry := &PolicyIdentifierWithQualifierEntry{} + // Either a OID, or a JSON Entry: First check OID: + _, err := StringToOid(policyIdentifier) + if err == nil { + entry.PolicyIdentifierOid = policyIdentifier + return entry, nil + } + // Now Check If JSON Entry + jsonErr := json.Unmarshal([]byte(policyIdentifier), &entry) + if jsonErr != nil { // Neither, if we got here + return entry, errors.New(fmt.Sprintf("Policy Identifier %q is neither a valid OID: %s, Nor JSON Policy Identifier: %s", policyIdentifier, err.Error(), jsonErr.Error())) + } + return entry, nil +} + +// Policy Identifier with Qualifier Structure for ASN Marshalling: + +var policyInformationOid = asn1.ObjectIdentifier{2, 5, 29, 32} + +type policyInformation struct { + PolicyIdentifier asn1.ObjectIdentifier + Qualifiers []interface{} `asn1:"tag:optional,omitempty"` +} + +var cpsPolicyQualifierID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 2, 1} + +type cpsUrlPolicyQualifier struct { + PolicyQualifierID asn1.ObjectIdentifier + Qualifier string `asn1:"tag:optional,ia5"` +} + +var userNoticePolicyQualifierID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 2, 2} + +type userNoticePolicyQualifier struct { + PolicyQualifierID asn1.ObjectIdentifier + Qualifier userNotice +} + +type userNotice struct { + ExplicitText string `asn1:"tag:optional,utf8"` +} + +func createPolicyIdentifierWithQualifier(entry PolicyIdentifierWithQualifierEntry) (*policyInformation, error) { + // Each Policy is Identified by a Unique ID, as designated here: + policyOid, err := StringToOid(entry.PolicyIdentifierOid) + if err != nil { + return nil, err + } + pi := policyInformation{ + PolicyIdentifier: policyOid, + } + if entry.CPS != "" { + qualifier := cpsUrlPolicyQualifier{ + PolicyQualifierID: cpsPolicyQualifierID, + Qualifier: entry.CPS, + } + pi.Qualifiers = append(pi.Qualifiers, qualifier) + } + if entry.Notice != "" { + qualifier := userNoticePolicyQualifier{ + PolicyQualifierID: userNoticePolicyQualifierID, + Qualifier: userNotice{ + ExplicitText: entry.Notice, + }, + } + pi.Qualifiers = append(pi.Qualifiers, qualifier) + } + return &pi, nil +} + +// CreatePolicyInformationExtensionFromStorageStrings parses the stored policyIdentifiers, which might be JSON Policy +// Identifier with Qualifier Entries or String OIDs, and returns an extension if everything parsed correctly, and an +// error if constructing +func CreatePolicyInformationExtensionFromStorageStrings(policyIdentifiers []string) (*pkix.Extension, error) { + var policyInformationList []policyInformation + for _, policyIdentifierStr := range policyIdentifiers { + policyIdentifierEntry, err := GetPolicyIdentifierFromString(policyIdentifierStr) + if err != nil { + return nil, err + } + if policyIdentifierEntry != nil { // Okay to skip empty entries if there is no error + policyInformationStruct, err := createPolicyIdentifierWithQualifier(*policyIdentifierEntry) + if err != nil { + return nil, err + } + policyInformationList = append(policyInformationList, *policyInformationStruct) + } + } + asn1Bytes, err := asn1.Marshal(policyInformationList) + if err != nil { + return nil, err + } + return &pkix.Extension{ + Id: policyInformationOid, + Critical: false, + Value: asn1Bytes, + }, nil +}