diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index c2c28003..af63e9ae 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -92,6 +92,63 @@ type certAttributes struct { serialNumber *big.Int keychain string useDataProtectionKeychain bool + + // Subject distinguished name components (cn, o, ou, l, st, c) used to + // filter certificates after retrieving them from the keychain. The + // keychain cannot match individual subject components, and it stores some + // of the subject values in uppercase. + commonName string + organization []string + organizationalUnit []string + locality []string + province []string + country []string +} + +// hasSubjectQuery reports whether at least one subject distinguished name +// component (cn, o, ou, l, st, c) is set. +func (u *certAttributes) hasSubjectQuery() bool { + return u.commonName != "" || len(u.organization) > 0 || + len(u.organizationalUnit) > 0 || len(u.locality) > 0 || + len(u.province) > 0 || len(u.country) > 0 +} + +// matchesSubject reports whether the certificate subject contains all the +// subject distinguished name components in u. Comparisons are exact and +// case-sensitive; for multi-valued components every requested value must be +// present in the certificate subject. It returns true if no components are +// set. +func (u *certAttributes) matchesSubject(cert *x509.Certificate) bool { + if u.commonName != "" && cert.Subject.CommonName != u.commonName { + return false + } + return containsAll(cert.Subject.Organization, u.organization) && + containsAll(cert.Subject.OrganizationalUnit, u.organizationalUnit) && + containsAll(cert.Subject.Locality, u.locality) && + containsAll(cert.Subject.Province, u.province) && + containsAll(cert.Subject.Country, u.country) +} + +// containsAll reports whether values contains all the elements in want. It +// returns true if want is empty. +func containsAll(values, want []string) bool { + for _, w := range want { + if !slices.Contains(values, w) { + return false + } + } + return true +} + +// nonEmptyValues returns the values in vs that are not the empty string. +func nonEmptyValues(vs []string) []string { + var values []string + for _, v := range vs { + if v != "" { + values = append(values, v) + } + } + return values } type algorithmAttributes struct { @@ -376,16 +433,31 @@ func (k *MacKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, er }, nil } -// LoadCertificate returns an [x509.Certificate] by its label and/or serial -// number. If multiple certificates match, it will return the certificate with -// the most recent NotBefore date among those that have already become valid. -// Future certificates maintain their original order. Expiration is not checked. -// By default Apple Keychain will use the certificate common name as the label. +// LoadCertificate returns an [x509.Certificate] by its label, serial number, +// and/or subject distinguished name components. If multiple certificates +// match, it will return the certificate with the most recent NotBefore date +// among those that have already become valid. Future certificates maintain +// their original order. Expiration is not checked. By default Apple Keychain +// will use the certificate common name as the label. // // Valid names (URIs) are: // - mackms:label=test@example.com // - mackms:serial=2c273934eda8454d2595a94497e2395a // - mackms:label=test@example.com;serial=2c273934eda8454d2595a94497e2395a +// - mackms:cn=My+Cert +// - mackms:cn=My+Cert;o=Smallstep;ou=Engineering;ou=Platform +// - mackms:label=test@example.com;ou=Engineering +// +// The "cn" (common name), "o" (organization), "ou" (organizational unit), "l" +// (locality), "st" (state or province), and "c" (country) attributes select +// certificates by their subject. All the given components must match using +// exact, case-sensitive comparison. The "cn" attribute is single-valued; the +// others can be repeated (e.g. "ou=a;ou=b"), and every given value must be +// present in the certificate subject. Values must be URL-encoded; spaces are +// encoded as "+" or "%20". Subject components can be combined with "label" and +// "serial", which narrow the keychain query. Note that a URI with just an +// attribute name and no value (e.g. "mackms:cn") is the shorthand for a label +// with that name, so subject components always require a value. // // On code-signed applications, it is possible to use the Data Protection // Keychain by default if [UseDataProtectionKeychain] is set to true. You can @@ -399,7 +471,7 @@ func (k *MacKMS) LoadCertificate(req *apiv1.LoadCertificateRequest) (*x509.Certi return nil, fmt.Errorf("loadCertificateRequest 'name' cannot be empty") } - // Require label or serial + // Require label, serial, or a subject component u, err := parseCertURI(req.Name, k.useDataProtectionKeychain, true) if err != nil { return nil, fmt.Errorf("mackms LoadCertificate failed: %w", err) @@ -424,6 +496,9 @@ func (k *MacKMS) LoadCertificate(req *apiv1.LoadCertificateRequest) (*x509.Certi // - "mackms:label=my-label" will use "my-label" // - "mackms:my-label" will use the "my-label" // +// The "serial" attribute and the subject components ("cn", "o", "ou", "l", +// "st", "c") are ignored when storing a certificate. +// // On code-signed applications, it is possible to use the Data Protection // Keychain by default if [UseDataProtectionKeychain] is set to true. You can // also select the keychain using the "keychain" attribute: @@ -464,23 +539,29 @@ func (k *MacKMS) StoreCertificate(req *apiv1.StoreCertificateRequest) error { return nil } -// LoadCertificateChain returns the leaf [x509.Certificate] by its label and/or -// serial number and its intermediate certificates. If multiple certificates -// match, it will return the certificate with the most recent NotBefore date -// among those that have already become valid. Future certificates maintain -// their original order. Expiration is not checked. By default Apple Keychain -// will use the certificate common name as the label. +// LoadCertificateChain returns the leaf [x509.Certificate] by its label, +// serial number, and/or subject distinguished name components, together with +// its intermediate certificates. If multiple certificates match, it will +// return the certificate with the most recent NotBefore date among those that +// have already become valid. Future certificates maintain their original +// order. Expiration is not checked. By default Apple Keychain will use the +// certificate common name as the label. // // Valid names (URIs) are: // - mackms:label=test@example.com // - mackms:serial=2c273934eda8454d2595a94497e2395a // - mackms:label=test@example.com;serial=2c273934eda8454d2595a94497e2395a +// - mackms:cn=My+Cert;ou=Engineering +// +// Subject components select the leaf certificate only; intermediates are +// located using the authority key identifier. See [MacKMS.LoadCertificate] for +// the subject component matching rules. func (k *MacKMS) LoadCertificateChain(req *apiv1.LoadCertificateChainRequest) ([]*x509.Certificate, error) { if req.Name == "" { return nil, fmt.Errorf("loadCertificateChainRequest 'name' cannot be empty") } - // Require label or serial + // Require label, serial, or a subject component u, err := parseCertURI(req.Name, k.useDataProtectionKeychain, true) if err != nil { return nil, fmt.Errorf("mackms LoadCertificateChain failed: %w", err) @@ -617,7 +698,18 @@ func (*MacKMS) DeleteKey(req *apiv1.DeleteKeyRequest) error { } // DeleteCertificate deletes the certificate referenced by the URI in the -// request name. +// request name. It deletes at most one certificate. +// +// Valid names (URIs) are: +// - mackms:label=test@example.com +// - mackms:serial=2c273934eda8454d2595a94497e2395a +// - mackms:label=test@example.com;serial=2c273934eda8454d2595a94497e2395a +// - mackms:cn=My+Cert;ou=Engineering +// +// When subject components ("cn", "o", "ou", "l", "st", "c") are present, the +// certificate is first located using the same selection rules as +// [MacKMS.LoadCertificate], and then deleted by its serial number and, when +// available, its subject key identifier. // // # Experimental // @@ -637,6 +729,27 @@ func (k *MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { security.KSecClass: security.KSecClassCertificate, security.KSecMatchLimit: security.KSecMatchLimitOne, } + + serialNumber := u.serialNumber + if u.hasSubjectQuery() { + // The keychain cannot match individual subject components. Find the + // matching certificate first, using the same selection rules as + // LoadCertificate, and delete it by its serial number and, when + // available, its subject key identifier. + cert, err := loadCertificate(u, nil) + if err != nil { + return fmt.Errorf("mackms DeleteCertificate failed: %w", apiv1Error(err)) + } + serialNumber = cert.SerialNumber + if len(cert.SubjectKeyId) > 0 { + cfSubjectKeyID, err := cf.NewData(cert.SubjectKeyId) + if err != nil { + return fmt.Errorf("mackms DeleteCertificate failed: %w", err) + } + defer cfSubjectKeyID.Release() + query[security.KSecAttrSubjectKeyID] = cfSubjectKeyID + } + } if u.label != "" { cfLabel, err := cf.NewString(u.label) if err != nil { @@ -645,8 +758,8 @@ func (k *MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { defer cfLabel.Release() query[security.KSecAttrLabel] = cfLabel } - if u.serialNumber != nil { - cfSerial, err := cf.NewData(encodeSerialNumber(u.serialNumber)) + if serialNumber != nil { + cfSerial, err := cf.NewData(encodeSerialNumber(serialNumber)) if err != nil { return fmt.Errorf("mackms DeleteCertificate failed: %w", err) } @@ -1090,6 +1203,22 @@ func loadCertificate(u *certAttributes, subjectKeyID []byte) (*x509.Certificate, return nil, err } } + + // Filter the certificates by the subject distinguished name components + // (cn, o, ou, l, st, c) in the uri. The keychain cannot match individual + // subject components, and it normalizes the subject values it stores, so + // the parsed certificates are filtered instead. + if u.hasSubjectQuery() { + certs = slices.DeleteFunc(certs, func(cert *x509.Certificate) bool { + return !u.matchesSubject(cert) + }) + if len(certs) == 0 { + return nil, apiv1.NotFoundError{ + Message: "certificate not found", + } + } + } + if len(certs) == 1 { return certs[0], nil } @@ -1283,16 +1412,28 @@ func parseCertURI(rawuri string, useDataProtectionKeychain, requireValue bool) ( if err != nil { return nil, fmt.Errorf("error parsing %q: %w", rawuri, err) } - if requireValue && label == "" && serialNumber == nil { - return nil, fmt.Errorf("error parsing %q: label or serial is required", rawuri) - } - return &certAttributes{ + // Subject distinguished name components (cn, o, ou, l, st, c) are used to + // filter the certificates retrieved from the keychain. Empty values are + // ignored, and o, ou, l, st, and c can be repeated (e.g. "ou=a;ou=b"). + values := uri.Values(u) + attrs := &certAttributes{ label: label, serialNumber: serialNumber, useDataProtectionKeychain: isDataProtectionKeychain(keychain, useDataProtectionKeychain), keychain: keychain, - }, nil + commonName: u.Get("cn"), + organization: nonEmptyValues(values["o"]), + organizationalUnit: nonEmptyValues(values["ou"]), + locality: nonEmptyValues(values["l"]), + province: nonEmptyValues(values["st"]), + country: nonEmptyValues(values["c"]), + } + if requireValue && label == "" && serialNumber == nil && !attrs.hasSubjectQuery() { + return nil, fmt.Errorf("error parsing %q: label, serial, or a subject component (cn, o, ou, l, st, c) is required", rawuri) + } + + return attrs, nil } func parseSearchURI(rawuri string) (*keySearchAttributes, error) { diff --git a/kms/mackms/mackms_test.go b/kms/mackms/mackms_test.go index adca47ec..b6ea3c3d 100644 --- a/kms/mackms/mackms_test.go +++ b/kms/mackms/mackms_test.go @@ -578,6 +578,125 @@ func Test_parseURI(t *testing.T) { } } +func Test_parseCertURI(t *testing.T) { + type args struct { + rawuri string + useDataProtectionKeychain bool + requireValue bool + } + tests := []struct { + name string + args args + want *certAttributes + assertion assert.ErrorAssertionFunc + }{ + {"ok label", args{"mackms:label=the-label", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok bare label", args{"the-label", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok label simple uri", args{"mackms:the-label", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok uppercase scheme", args{"MACKMS:label=the-label", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok serial hex", args{"mackms:serial=0x0102", false, true}, &certAttributes{serialNumber: big.NewInt(0x0102)}, assert.NoError}, + {"ok serial decimal", args{"mackms:serial=123456", false, true}, &certAttributes{serialNumber: big.NewInt(123456)}, assert.NoError}, + {"ok label and serial", args{"mackms:label=the-label;serial=0x01", false, true}, &certAttributes{label: "the-label", serialNumber: big.NewInt(1)}, assert.NoError}, + {"ok cn", args{"mackms:cn=My+Cert", false, true}, &certAttributes{commonName: "My Cert"}, assert.NoError}, + {"ok cn percent encoded", args{"mackms:cn=My%20Cert", false, true}, &certAttributes{commonName: "My Cert"}, assert.NoError}, + {"ok subject components", args{"mackms:cn=leaf;o=Smallstep;ou=Eng;l=San+Francisco;st=California;c=US", false, true}, &certAttributes{ + commonName: "leaf", + organization: []string{"Smallstep"}, + organizationalUnit: []string{"Eng"}, + locality: []string{"San Francisco"}, + province: []string{"California"}, + country: []string{"US"}, + }, assert.NoError}, + {"ok repeated ou", args{"mackms:ou=a;ou=b", false, true}, &certAttributes{organizationalUnit: []string{"a", "b"}}, assert.NoError}, + {"ok label serial and cn", args{"mackms:label=the-label;serial=0x01;cn=leaf", false, true}, &certAttributes{label: "the-label", serialNumber: big.NewInt(1), commonName: "leaf"}, assert.NoError}, + {"ok empty subject values", args{"mackms:label=the-label;o=;ou=", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok empty cn with label", args{"mackms:cn=;label=the-label", false, true}, &certAttributes{label: "the-label"}, assert.NoError}, + {"ok keychain", args{"mackms:label=the-label;keychain=dataProtection", false, true}, &certAttributes{label: "the-label", keychain: "dataProtection", useDataProtectionKeychain: true}, assert.NoError}, + {"ok default data protection", args{"mackms:label=the-label", true, true}, &certAttributes{label: "the-label", useDataProtectionKeychain: true}, assert.NoError}, + {"ok no require value", args{"mackms:", false, false}, &certAttributes{}, assert.NoError}, + {"ok bare cn is a label", args{"mackms:cn", false, true}, &certAttributes{label: "cn"}, assert.NoError}, + {"ok bare cn equal is a label", args{"mackms:cn=", false, true}, &certAttributes{label: "cn"}, assert.NoError}, + {"fail require value", args{"mackms:", false, true}, nil, assert.Error}, + {"fail keychain only", args{"mackms:keychain=login", false, true}, nil, assert.Error}, + {"fail bad serial", args{"mackms:serial=010a020b030z", false, true}, nil, assert.Error}, + {"fail parse", args{"mackms:%label=the-label", false, true}, nil, assert.Error}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseCertURI(tt.args.rawuri, tt.args.useDataProtectionKeychain, tt.args.requireValue) + tt.assertion(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_certAttributes_hasSubjectQuery(t *testing.T) { + tests := []struct { + name string + attrs *certAttributes + want bool + }{ + {"empty", &certAttributes{}, false}, + {"label and serial only", &certAttributes{label: "the-label", serialNumber: big.NewInt(1), keychain: "login"}, false}, + {"cn", &certAttributes{commonName: "leaf"}, true}, + {"o", &certAttributes{organization: []string{"Smallstep"}}, true}, + {"ou", &certAttributes{organizationalUnit: []string{"Eng"}}, true}, + {"l", &certAttributes{locality: []string{"San Francisco"}}, true}, + {"st", &certAttributes{province: []string{"California"}}, true}, + {"c", &certAttributes{country: []string{"US"}}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.attrs.hasSubjectQuery()) + }) + } +} + +func Test_certAttributes_matchesSubject(t *testing.T) { + cert := &x509.Certificate{Subject: pkix.Name{ + CommonName: "leaf", + Organization: []string{"Smallstep"}, + OrganizationalUnit: []string{"Eng", "Platform"}, + Locality: []string{"San Francisco"}, + Province: []string{"California"}, + Country: []string{"US"}, + }} + + tests := []struct { + name string + attrs *certAttributes + cert *x509.Certificate + want bool + }{ + {"ok empty query", &certAttributes{label: "the-label"}, cert, true}, + {"ok cn", &certAttributes{commonName: "leaf"}, cert, true}, + {"ok all components", &certAttributes{ + commonName: "leaf", + organization: []string{"Smallstep"}, + organizationalUnit: []string{"Eng", "Platform"}, + locality: []string{"San Francisco"}, + province: []string{"California"}, + country: []string{"US"}, + }, cert, true}, + {"ok ou subset", &certAttributes{organizationalUnit: []string{"Platform"}}, cert, true}, + {"fail cn", &certAttributes{commonName: "other"}, cert, false}, + {"fail cn case sensitive", &certAttributes{commonName: "Leaf"}, cert, false}, + {"fail ou case sensitive", &certAttributes{organizationalUnit: []string{"eng"}}, cert, false}, + {"fail and semantics", &certAttributes{commonName: "leaf", organizationalUnit: []string{"Marketing"}}, cert, false}, + {"fail ou superset", &certAttributes{organizationalUnit: []string{"Eng", "Marketing"}}, cert, false}, + {"fail o", &certAttributes{organization: []string{"Other"}}, cert, false}, + {"fail l", &certAttributes{locality: []string{"New York"}}, cert, false}, + {"fail st", &certAttributes{province: []string{"New York"}}, cert, false}, + {"fail c", &certAttributes{country: []string{"ES"}}, cert, false}, + {"fail empty subject", &certAttributes{commonName: "leaf"}, &x509.Certificate{}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.attrs.matchesSubject(tt.cert)) + }) + } +} + func Test_parseECDSAPublicKey(t *testing.T) { p256, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) @@ -954,6 +1073,127 @@ func TestMacKMS_LoadCertificate_sort(t *testing.T) { assert.Equal(t, cert2, cert) } +func TestMacKMS_LoadCertificate_bySubject(t *testing.T) { + testName := t.Name() + ca, err := minica.New(minica.WithName(testName)) + require.NoError(t, err) + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + suffix, err := randutil.Alphanumeric(8) + require.NoError(t, err) + label := "test-" + suffix + + now := time.Now().Truncate(time.Second) + cert1, err := ca.Sign(&x509.Certificate{ + Subject: pkix.Name{ + CommonName: testName + "-1-" + suffix, + Organization: []string{"TestOrg"}, + OrganizationalUnit: []string{"Engineering"}, + Locality: []string{"San Francisco"}, + Province: []string{"California"}, + Country: []string{"US"}, + }, + PublicKey: key.Public(), + NotBefore: now, + }) + require.NoError(t, err) + + cert2, err := ca.Sign(&x509.Certificate{ + Subject: pkix.Name{ + CommonName: testName + "-2-" + suffix, + Organization: []string{"TestOrg"}, + OrganizationalUnit: []string{"Engineering", "Platform"}, + Locality: []string{"New York"}, + Province: []string{"New York"}, + Country: []string{"US"}, + }, + PublicKey: key.Public(), + NotBefore: now.Add(-time.Second), + }) + require.NoError(t, err) + + cert3, err := ca.Sign(&x509.Certificate{ + Subject: pkix.Name{ + CommonName: testName + "-3-" + suffix, + Organization: []string{"OtherOrg"}, + OrganizationalUnit: []string{"Sales"}, + Country: []string{"ES"}, + }, + PublicKey: key.Public(), + NotBefore: now.Add(-2 * time.Second), + }) + require.NoError(t, err) + + kms := &MacKMS{} + for _, crt := range []*x509.Certificate{cert1, cert2, cert3} { + require.NoError(t, kms.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: "mackms:label=" + label, Certificate: crt, + })) + t.Cleanup(func() { deleteCertificate(t, label, crt) }) + } + + isNotFound := func(t assert.TestingT, err error, i ...any) bool { + return assert.ErrorIs(t, err, apiv1.NotFoundError{}, i...) + } + + type args struct { + req *apiv1.LoadCertificateRequest + } + tests := []struct { + name string + args args + want *x509.Certificate + assertion assert.ErrorAssertionFunc + }{ + {"ok cn", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";cn=" + cert2.Subject.CommonName, + }}, cert2, assert.NoError}, + {"ok ou", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Platform", + }}, cert2, assert.NoError}, + {"ok ou repeated", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Engineering;ou=Platform", + }}, cert2, assert.NoError}, + {"ok ou multiple matches", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Engineering", + }}, cert1, assert.NoError}, + {"ok o", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";o=OtherOrg", + }}, cert3, assert.NoError}, + {"ok l with spaces", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";l=San+Francisco", + }}, cert1, assert.NoError}, + {"ok st and c", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";st=New+York;c=US", + }}, cert2, assert.NoError}, + {"ok cn only", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:cn=" + cert1.Subject.CommonName, + }}, cert1, assert.NoError}, + {"ok serial and o", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:serial=" + hex.EncodeToString(cert3.SerialNumber.Bytes()) + ";o=OtherOrg", + }}, cert3, assert.NoError}, + {"fail serial and o", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:serial=" + hex.EncodeToString(cert3.SerialNumber.Bytes()) + ";o=TestOrg", + }}, nil, isNotFound}, + {"fail ou", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Marketing", + }}, nil, isNotFound}, + {"fail ou case sensitive", args{&apiv1.LoadCertificateRequest{ + Name: "mackms:label=" + label + ";ou=engineering", + }}, nil, isNotFound}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := &MacKMS{} + got, err := k.LoadCertificate(tt.args.req) + tt.assertion(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + func TestMacKMS_StoreCertificate(t *testing.T) { testName := t.Name() @@ -1151,7 +1391,10 @@ func TestMacKMS_LoadCertificateChain(t *testing.T) { require.NoError(t, err) cert, err := ca.Sign(&x509.Certificate{ - Subject: pkix.Name{CommonName: testName + "@example.com"}, + Subject: pkix.Name{ + CommonName: testName + "@example.com", + OrganizationalUnit: []string{"Engineering"}, + }, EmailAddresses: []string{testName + "@example.com"}, PublicKey: key.Public(), }) @@ -1192,12 +1435,21 @@ func TestMacKMS_LoadCertificateChain(t *testing.T) { {"ok label and serial", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{ Name: "mackms:labeld=" + cert.Subject.CommonName + ";serial=" + hex.EncodeToString(cert.SerialNumber.Bytes()), }}, []*x509.Certificate{cert, ca.Intermediate}, assert.NoError}, + {"ok cn", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{ + Name: "mackms:cn=" + cert.Subject.CommonName, + }}, []*x509.Certificate{cert, ca.Intermediate}, assert.NoError}, + {"ok label and ou", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{ + Name: "mackms:label=" + cert.Subject.CommonName + ";ou=Engineering", + }}, []*x509.Certificate{cert, ca.Intermediate}, assert.NoError}, {"ok self-signed", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{ Name: "mackms:label=" + ca.Root.Subject.CommonName, }}, []*x509.Certificate{ca.Root}, assert.NoError}, {"fail name", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{}}, nil, assert.Error}, {"fail uri", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{Name: "mackms:"}}, nil, assert.Error}, {"fail missing", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{Name: "mackms:label=missing-" + testName}}, nil, assert.Error}, + {"fail ou", &MacKMS{}, args{&apiv1.LoadCertificateChainRequest{ + Name: "mackms:label=" + cert.Subject.CommonName + ";ou=Marketing", + }}, nil, assert.Error}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1446,6 +1698,117 @@ func TestMacKMS_DeleteCertificate(t *testing.T) { } } +func TestMacKMS_DeleteCertificate_bySubject(t *testing.T) { + testName := t.Name() + ca, err := minica.New(minica.WithName(testName)) + require.NoError(t, err) + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + suffix, err := randutil.Alphanumeric(8) + require.NoError(t, err) + label := "test-del-" + suffix + + certA, err := ca.Sign(&x509.Certificate{ + Subject: pkix.Name{ + CommonName: testName + "-a-" + suffix, + OrganizationalUnit: []string{"Platform"}, + }, + PublicKey: key.Public(), + }) + require.NoError(t, err) + require.NotEmpty(t, certA.SubjectKeyId) + + certB, err := ca.Sign(&x509.Certificate{ + Subject: pkix.Name{ + CommonName: testName + "-b-" + suffix, + OrganizationalUnit: []string{"Engineering"}, + }, + PublicKey: key.Public(), + }) + require.NoError(t, err) + + // Create a self-signed certificate without a subject key identifier to + // exercise the delete by serial number only. Go only generates the subject + // key identifier automatically for CAs. + now := time.Now() + serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 64)) + require.NoError(t, err) + template := &x509.Certificate{ + SerialNumber: serial.Add(serial, big.NewInt(1)), + Subject: pkix.Name{ + CommonName: testName + "-c-" + suffix, + OrganizationalUnit: []string{"NoSKID"}, + }, + NotBefore: now.Add(-time.Hour), + NotAfter: now.Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, key.Public(), key) + require.NoError(t, err) + certC, err := x509.ParseCertificate(der) + require.NoError(t, err) + require.Empty(t, certC.SubjectKeyId) + + serialURI := func(cert *x509.Certificate) string { + return "mackms:serial=0x" + hex.EncodeToString(cert.SerialNumber.Bytes()) + } + existsCheck := func(cert *x509.Certificate) { + kms := &MacKMS{} + _, err := kms.LoadCertificate(&apiv1.LoadCertificateRequest{Name: serialURI(cert)}) + assert.NoError(t, err) + } + notExistsCheck := func(cert *x509.Certificate) { + kms := &MacKMS{} + _, err := kms.LoadCertificate(&apiv1.LoadCertificateRequest{Name: serialURI(cert)}) + assert.ErrorIs(t, err, apiv1.NotFoundError{}) + } + + kms := &MacKMS{} + for _, crt := range []*x509.Certificate{certA, certB, certC} { + require.NoError(t, kms.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: "mackms:label=" + label, Certificate: crt, + })) + } + t.Cleanup(func() { + // The certificates are deleted by the test itself; this only removes + // the leftovers if the test fails early. + kms := &MacKMS{} + for _, crt := range []*x509.Certificate{certA, certB, certC} { + _ = kms.DeleteCertificate(&apiv1.DeleteCertificateRequest{Name: serialURI(crt)}) + } + }) + + // Delete by subject removes only the matching certificate. + require.NoError(t, kms.DeleteCertificate(&apiv1.DeleteCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Platform", + })) + notExistsCheck(certA) + existsCheck(certB) + existsCheck(certC) + + // Delete a certificate without a subject key identifier. + require.NoError(t, kms.DeleteCertificate(&apiv1.DeleteCertificateRequest{ + Name: "mackms:label=" + label + ";ou=NoSKID", + })) + notExistsCheck(certC) + existsCheck(certB) + + // Fail to delete if no certificate matches the subject. + err = kms.DeleteCertificate(&apiv1.DeleteCertificateRequest{ + Name: "mackms:label=" + label + ";ou=Marketing", + }) + assert.ErrorIs(t, err, apiv1.NotFoundError{}) + existsCheck(certB) + + // Delete using only subject components. + require.NoError(t, kms.DeleteCertificate(&apiv1.DeleteCertificateRequest{ + Name: "mackms:cn=" + certB.Subject.CommonName, + })) + notExistsCheck(certB) +} + func Test_apiv1Error(t *testing.T) { type args struct { err error