diff --git a/smx509/verify.go b/smx509/verify.go index 29e781a..9766d53 100644 --- a/smx509/verify.go +++ b/smx509/verify.go @@ -488,25 +488,6 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V leaf = currentChain[0] } - if (len(c.ExtKeyUsage) > 0 || len(c.UnknownExtKeyUsage) > 0) && len(opts.KeyUsages) > 0 { - acceptableUsage := false - um := make(map[ExtKeyUsage]bool, len(opts.KeyUsages)) - for _, u := range opts.KeyUsages { - um[u] = true - } - if !um[ExtKeyUsageAny] { - for _, u := range c.ExtKeyUsage { - if u == ExtKeyUsageAny || um[u] { - acceptableUsage = true - break - } - } - if !acceptableUsage { - return CertificateInvalidError{Cert: c.asX509(), Reason: IncompatibleUsage, Detail: ""} - } - } - } - if (certType == intermediateCertificate || certType == rootCertificate) && c.hasNameConstraints() { toCheck := []*Certificate{} @@ -683,26 +664,45 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e } } - if len(opts.KeyUsages) == 0 { - opts.KeyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth} - } - err = c.isValid(leafCertificate, nil, &opts) if err != nil { return } - if len(opts.DNSName) > 0 { - err = c.VerifyHostname(opts.DNSName) + var candidateChains [][]*Certificate + if opts.Roots.contains(c) { + candidateChains = [][]*Certificate{{c}} + } else { + candidateChains, err = c.buildChains([]*Certificate{c}, nil, &opts) if err != nil { - return + return nil, err } } - if opts.Roots.contains(c) { - return [][]*Certificate{{c}}, nil + if len(opts.KeyUsages) == 0 { + opts.KeyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth} } - return c.buildChains([]*Certificate{c}, nil, &opts) + + for _, eku := range opts.KeyUsages { + if eku == ExtKeyUsageAny { + // If any key usage is acceptable, no need to check the chain for + // key usages. + return candidateChains, nil + } + } + + chains = make([][]*Certificate, 0, len(candidateChains)) + for _, candidate := range candidateChains { + if checkChainForKeyUsage(candidate, opts.KeyUsages) { + chains = append(chains, candidate) + } + } + + if len(chains) == 0 { + return nil, CertificateInvalidError{c.asX509(), IncompatibleUsage, ""} + } + + return chains, nil } func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate { diff --git a/smx509/verify_test.go b/smx509/verify_test.go index 22161d0..f94ca1c 100644 --- a/smx509/verify_test.go +++ b/smx509/verify_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "encoding/pem" "errors" "fmt" @@ -2323,6 +2324,54 @@ func TestPathBuilding(t *testing.T) { "CN=leaf -> CN=inter b -> CN=inter c -> CN=root", }, }, + { + // Build a basic graph with two paths from leaf to root, but the path passing + // through C should be ignored, because it has invalid EKU nesting. + name: "ignore invalid EKU path", + graph: trustGraphDescription{ + Roots: []string{"root"}, + Leaf: "leaf", + Graph: []trustGraphEdge{ + { + Issuer: "root", + Subject: "inter a", + Type: intermediateCertificate, + }, + { + Issuer: "root", + Subject: "inter c", + Type: intermediateCertificate, + }, + { + Issuer: "inter c", + Subject: "inter b", + Type: intermediateCertificate, + MutateTemplate: func(t *Certificate) { + t.ExtKeyUsage = []ExtKeyUsage{ExtKeyUsageCodeSigning} + }, + }, + { + Issuer: "inter a", + Subject: "inter b", + Type: intermediateCertificate, + MutateTemplate: func(t *Certificate) { + t.ExtKeyUsage = []ExtKeyUsage{ExtKeyUsageServerAuth} + }, + }, + { + Issuer: "inter b", + Subject: "leaf", + Type: leafCertificate, + MutateTemplate: func(t *Certificate) { + t.ExtKeyUsage = []ExtKeyUsage{ExtKeyUsageServerAuth} + }, + }, + }, + }, + expectedChains: []string{ + "CN=leaf -> CN=inter b -> CN=inter a -> CN=root", + }, + }, } for _, tc := range tests { @@ -2342,3 +2391,194 @@ func TestPathBuilding(t *testing.T) { }) } } + +func TestEKUEnforcement(t *testing.T) { + type ekuDescs struct { + EKUs []ExtKeyUsage + Unknown []asn1.ObjectIdentifier + } + tests := []struct { + name string + root ekuDescs + inters []ekuDescs + leaf ekuDescs + verifyEKUs []ExtKeyUsage + err string + }{ + { + name: "valid, full chain", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + inters: []ekuDescs{ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + }, + { + name: "valid, only leaf has EKU", + root: ekuDescs{}, + inters: []ekuDescs{ekuDescs{}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + }, + { + name: "invalid, serverAuth not nested", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageClientAuth}}, + inters: []ekuDescs{ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + err: "x509: certificate specifies an incompatible key usage", + }, + { + name: "valid, two EKUs, one path", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + inters: []ekuDescs{ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}, + }, + { + name: "invalid, ladder", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + inters: []ekuDescs{ + ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}, + ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageClientAuth}}, + ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}}, + ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + }, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}, + err: "x509: certificate specifies an incompatible key usage", + }, + { + name: "valid, intermediate has no EKU", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + inters: []ekuDescs{ekuDescs{}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + }, + { + name: "invalid, intermediate has no EKU and no nested path", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageClientAuth}}, + inters: []ekuDescs{ekuDescs{}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth, ExtKeyUsageClientAuth}, + err: "x509: certificate specifies an incompatible key usage", + }, + { + name: "invalid, intermediate has unknown EKU", + root: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + inters: []ekuDescs{ekuDescs{Unknown: []asn1.ObjectIdentifier{{1, 2, 3}}}}, + leaf: ekuDescs{EKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + err: "x509: certificate specifies an incompatible key usage", + }, + } + + k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate test key: %s", err) + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + rootPool := NewCertPool() + root := genCertEdge(t, "root", k, func(c *Certificate) { + c.ExtKeyUsage = tc.root.EKUs + c.UnknownExtKeyUsage = tc.root.Unknown + }, rootCertificate, nil, k) + rootPool.AddCert(root) + + parent := root + interPool := NewCertPool() + for i, interEKUs := range tc.inters { + inter := genCertEdge(t, fmt.Sprintf("inter %d", i), k, func(c *Certificate) { + c.ExtKeyUsage = interEKUs.EKUs + c.UnknownExtKeyUsage = interEKUs.Unknown + }, intermediateCertificate, parent, k) + interPool.AddCert(inter) + parent = inter + } + + leaf := genCertEdge(t, "leaf", k, func(c *Certificate) { + c.ExtKeyUsage = tc.leaf.EKUs + c.UnknownExtKeyUsage = tc.leaf.Unknown + }, intermediateCertificate, parent, k) + + _, err := leaf.Verify(VerifyOptions{Roots: rootPool, Intermediates: interPool, KeyUsages: tc.verifyEKUs}) + if err == nil && tc.err != "" { + t.Errorf("expected error") + } else if err != nil && err.Error() != tc.err { + t.Errorf("unexpected error: want %q, got %q", err.Error(), tc.err) + } + }) + } +} + +func TestVerifyEKURootAsLeaf(t *testing.T) { + k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate key: %s", err) + } + + for _, tc := range []struct { + rootEKUs []ExtKeyUsage + verifyEKUs []ExtKeyUsage + succeed bool + }{ + { + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + succeed: true, + }, + { + rootEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + succeed: true, + }, + { + rootEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + succeed: true, + }, + { + rootEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageAny}, + succeed: true, + }, + { + rootEKUs: []ExtKeyUsage{ExtKeyUsageAny}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + succeed: true, + }, + { + rootEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth}, + verifyEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth}, + succeed: false, + }, + } { + t.Run(fmt.Sprintf("root EKUs %#v, verify EKUs %#v", tc.rootEKUs, tc.verifyEKUs), func(t *testing.T) { + tmpl := &Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "root"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + DNSNames: []string{"localhost"}, + ExtKeyUsage: tc.rootEKUs, + } + rootDER, err := CreateCertificate(rand.Reader, tmpl.asX509(), tmpl.asX509(), k.Public(), k) + if err != nil { + t.Fatalf("failed to create certificate: %s", err) + } + root, err := ParseCertificate(rootDER) + if err != nil { + t.Fatalf("failed to parse certificate: %s", err) + } + roots := NewCertPool() + roots.AddCert(root) + + _, err = root.Verify(VerifyOptions{Roots: roots, KeyUsages: tc.verifyEKUs}) + if err == nil && !tc.succeed { + t.Error("verification succeed") + } else if err != nil && tc.succeed { + t.Errorf("verification failed: %q", err) + } + }) + } + +}