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.

Sign up for GitHub

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

Jump to bottom

fix: add check for unsupported subject fields #275

Merged
merged 6 commits into from Feb 16, 2023
Merged

fix: add check for unsupported subject fields #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
91fe728
Add check for unsupported subject fields
byronchien Feb 10, 2023
c06473c
Move check to pkix.ParseDistinguishedName
byronchien Feb 11, 2023
1ecda12
Improve error message
byronchien Feb 14, 2023
dee9625
Update internal/pkix/pkix.go
byronchien Feb 15, 2023
cac9f1a
Remove comment
byronchien Feb 15, 2023
3f534dc
Merge branch 'main' into cert-subject-check
byronchien Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions internal/pkix/pkix.go
View file
Open in desktop
Expand Up @@ -2,12 +2,17 @@ package pkix

import (
"fmt"
"strings"

ldapv3 "github.com/go-ldap/ldap/v3"
)

// ParseDistinguishedName parses a DN name and validates Notary V2 rules
func ParseDistinguishedName(name string) (map[string]string, error) {
if strings.Contains(name, "=#") {
return nil, fmt.Errorf("unsupported distinguished name (DN) %q: notation does not support x509.subject identities containing \"=#\"", name)
}

mandatoryFields := []string{"C", "ST", "O"}
attrKeyValue := make(map[string]string)
dn, err := ldapv3.ParseDN(name)
Expand Down
31 changes: 31 additions & 0 deletions verifier/testdata/verifier/bad-cert.pem
View file
Open in desktop
@@ -0,0 +1,31 @@
-----BEGIN CERTIFICATE-----
MIIFPjCCAyYCCQCUhFkkjvs/QzANBgkqhkiG9w0BAQsFADBhMQswCQYDVQQGEwJV
UzELMAkGA1UECAwCV0ExEDAOBgNVBAcMB1NlYXR0bGUxEDAOBgNVBAMMB0JhZD0j
Q04xDzANBgNVBAsMBlNvbWVPVTEQMA4GA1UECgwHU29tZU9yZzAeFw0yMzAyMTAy
MjU4MjZaFw0yNDAyMTAyMjU4MjZaMGExCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJX
QTEQMA4GA1UEBwwHU2VhdHRsZTEQMA4GA1UEAwwHQmFkPSNDTjEPMA0GA1UECwwG
U29tZU9VMRAwDgYDVQQKDAdTb21lT3JnMIICIjANBgkqhkiG9w0BAQEFAAOCAg8A
MIICCgKCAgEA3eCwQ3snC1akVZKRFTqzoJ9HldLtzhmIbzaQeofKRn1HG18Dxbfw
XGtL0kqFL7ew9C7Qg0JpSo4tp0r3TdMsykFOPf9nkMjGQOA4TTPdYCGtKYLP0UiE
yDeLita5VkXABuwSGG5wi1tcuPQXVCXvnyzTPVj9eRGMBvFNop5P8y8cY4Jv5PJc
PLlgnMskdTqElKyIqk5E6KP9NeVdiJW1c6JZ+TvICfjcwhinAVVE00dgMidDRk/+
LorlWFJoXLw4i6b8uLXar8Xh7kn83LMOFM217WiW8h78ANPjtDxBnzw9BiCEQXBo
ScpIdmyhmm4TBiXJHOOTwhZPUtRFmAsk/apP+OkNI20ENQUvu7MHD0Rjy1ZHCI2W
VDAQt7/8PeDKsVeM8HNpehN9Rjom5mF9PE21Y2meZKhbU3vlWNO+MAXm4yitp5m3
1gS+cLnh88YPkRPlJVNv/HbJerqh+9sFB94IxRbpgOP2np5XyR+1yTKSbUZt6kIg
Zoo9vsOjCZ2Mgy67dVMJ7mqMeRKw/v3LapGuUizv8XlTF3Sg2LBkzrPe0+PVOw85
Oymprd0rasM0CoxQt3uI+shGUA7ZwEUwg6cjvV4VqvbXsRAgJyvNedp2fawrNC01
EPP63c8zXmS9j/PDfS9zSAgsNJnBcuoqWCFXQmbI2+FHjzYgPdjtYEECAwEAATAN
BgkqhkiG9w0BAQsFAAOCAgEAXnOIleUM6unIJGpsCQkaVBG2bqIkbTZRkO85ekQG
GeU7J9RMF8w+qO5zqcK6X5iUlN+w/eLbu7oLPK0ST4NIV9cXxf+mWrX2Tceee65e
qLPbtinPm9oSVcc7TGTx0scmHaOTmh6v01zGo/oQMVah2uCeTbanuRyoH9Qa/rOv
1o8/JmbYqDrNP9/Lm6c8+iOPBab0MmR17Vp6zs2gAimGD+30at8nm4uEMHGbT4XL
8HVxI6Qn/jUJTKq6XpWSLYI2g2L0Sr0vGmqhnNKb2fPQJsYGL5dO4RTuQOoKGrnQ
LMNQZuh2ifDI2Eri9PZTCUJ/T1Iqenko6GxTfLcCe1nVB3bUqvS+fDKWri4Vdf2P
w9hCdnRycL6llfGGjMMYvRx/HK8mzGrfMUhqa6/raRiK7REzZpFt9walbAtdi/o4
iIh7Tb4ju7pj0GfzsqGJXEGZQyv/RNfkhXBJdFuSsB7ysGIPMzf5lcpNbVBVdahi
hI2BJ0x+1JtJ1YqM1bQtyaikieEkj5PrhK1Uugev3zdo9wtuQPXo9mLX86aBNIBB
v8+lWu6Y1vWinDYiztdZAiK2P7Nn9V3EbORvf2r5F4lKCYEJbUl42+ANwir6fMBb
u+gbgN7ueDcsd3MQ2VJ4mVvaA2UQspbZdpoDIwnBchy8IDbwtx8aARzw8NrAXjrl
F/Y=
-----END CERTIFICATE-----
3 changes: 1 addition & 2 deletions verifier/trustpolicy/trustpolicy_test.go
View file
Open in desktop
Expand Up @@ -146,8 +146,7 @@ func TestValidateTrustedIdentities(t *testing.T) {
validDN1 := "x509.subject:C=US,ST=WA,O=MyOrg"
validDN2 := "x509.subject:C=US,ST=WA,O= My. Org"
validDN3 := "x509.subject:C=US,ST=WA,O=My \"special\" Org \\, \\; \\\\ others"
validDN4 := "x509.subject:C=US,ST=WA,O=My Org,1.3.6.1.4.1.1466.0=#04024869"
policyStatement.TrustedIdentities = []string{validDN1, validDN2, validDN3, validDN4}
policyStatement.TrustedIdentities = []string{validDN1, validDN2, validDN3}
policyDoc.TrustPolicies = []TrustPolicy{policyStatement}
err = policyDoc.Validate()
if err != nil {
Expand Down
27 changes: 16 additions & 11 deletions verifier/verifier_test.go
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ package verifier

import (
"context"
"crypto/x509"
"errors"
"fmt"
"path/filepath"
Expand Down Expand Up @@ -573,20 +574,24 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {

func TestVerifyX509TrustedIdentities(t *testing.T) {

certs, _ := corex509.ReadCertificateFile(filepath.FromSlash("testdata/verifier/signing-cert.pem")) // cert's subject is "CN=SomeCN,OU=SomeOU,O=SomeOrg,L=Seattle,ST=WA,C=US"
certs, _ := corex509.ReadCertificateFile(filepath.FromSlash("testdata/verifier/signing-cert.pem")) // cert's subject is "CN=SomeCN,OU=SomeOU,O=SomeOrg,L=Seattle,ST=WA,C=US"
unsupportedCerts, _ := corex509.ReadCertificateFile(filepath.FromSlash("testdata/verifier/bad-cert.pem")) // cert's subject is "CN=bad=#CN,OU=SomeOU,O=SomeOrg,L=Seattle,ST=WA,C=US"

tests := []struct {
certs []*x509.Certificate
x509Identities []string
wantErr bool
}{
{[]string{"x509.subject:C=US,O=SomeOrg,ST=WA"}, false},
{[]string{"x509.subject:C=US,O=SomeOrg,ST=WA", "nonX509Prefix:my-custom-identity"}, false},
{[]string{"x509.subject:C=US,O=SomeOrg,ST=WA", "x509.subject:C=IND,O=SomeOrg,ST=TS"}, false},
{[]string{"nonX509Prefix:my-custom-identity"}, true},
{[]string{"*"}, false},
{[]string{"x509.subject:C=IND,O=SomeOrg,ST=TS"}, true},
{[]string{"x509.subject:C=IND,O=SomeOrg,ST=TS", "nonX509Prefix:my-custom-identity"}, true},
{[]string{"x509.subject:C=IND,O=SomeOrg,ST=TS", "x509.subject:C=LOL,O=LOL,ST=LOL"}, true},
{certs, []string{"x509.subject:C=US,O=SomeOrg,ST=WA"}, false},
{certs, []string{"x509.subject:C=US,O=SomeOrg,ST=WA", "nonX509Prefix:my-custom-identity"}, false},
{certs, []string{"x509.subject:C=US,O=SomeOrg,ST=WA", "x509.subject:C=IND,O=SomeOrg,ST=TS"}, false},
{certs, []string{"nonX509Prefix:my-custom-identity"}, true},
{certs, []string{"*"}, false},
{certs, []string{"x509.subject:C=IND,O=SomeOrg,ST=TS"}, true},
{certs, []string{"x509.subject:C=IND,O=SomeOrg,ST=TS", "nonX509Prefix:my-custom-identity"}, true},
{certs, []string{"x509.subject:C=IND,O=SomeOrg,ST=TS", "x509.subject:C=LOL,O=LOL,ST=LOL"}, true},
{certs, []string{"x509.subject:C=bad=#identity,O=LOL,ST=LOL"}, true},
{unsupportedCerts, []string{"x509.subject:C=US,O=SomeOrg,ST=WA", "nonX509Prefix:my-custom-identity"}, true},
}
for i, tt := range tests {
t.Run(strconv.Itoa(i), func(t *testing.T) {
Expand All @@ -597,7 +602,7 @@ func TestVerifyX509TrustedIdentities(t *testing.T) {
TrustStores: []string{"ca:test-store"},
TrustedIdentities: tt.x509Identities,
}
err := verifyX509TrustedIdentities(certs, &trustPolicy)
err := verifyX509TrustedIdentities(tt.certs, &trustPolicy)

if tt.wantErr != (err != nil) {
t.Fatalf("TestVerifyX509TrustedIdentities Error: %q WantErr: %v", err, tt.wantErr)
Expand Down Expand Up @@ -648,7 +653,7 @@ func TestVerifyUserMetadata(t *testing.T) {
t.Fatalf("TestVerifyUserMetadata Error: %q WantErr: %v", err, tt.wantErr)
}
})
}
}
}

func TestPluginVersionCompatibility(t *testing.T) {
Expand Down