Repository: zmap/zlint
Pull request #293 (2019-07-25 20:21:09)

lints: add Notice level lint for EE ECDSA KeyUsages.
[RFC 5480 Section 3](https://tools.ietf.org/html/rfc5480#section-3) "Key Usage Bits" indicates that end-entity certificates using a EC public key MAY include the `digitalSignature`, `nonRepudiation`, and `keyAgreement` Key Usages.

If such a certificate contains other Key Usages the new `n_ecdsa_ee_invalid_ku` lint will return a `Notice` level `LintResult` indicating the unexpected Key Usage bits that were included. Depending on the adoption of [a clarification document](https://datatracker.ietf.org/doc/draft-turner-5480-ku-clarifications/) and respective CABF BR updates it may be possible to increase the severity of this `LintResult` in the future. See https://github.com/zmap/zlint/issues/291 for more background discussion.

There are probably similar lints that could be written to add related coverage for other key types and KUs. I only had time to implement this more narrowly scoped lint this week but would be available to review follow-up PRs from other contributors. 

Resolves https://github.com/zmap/zlint/issues/291


Patch:
diff --git a/lints/base.go b/lints/base.go
index f4357115..a7d453c9 100644
--- a/lints/base.go
+++ b/lints/base.go
@@ -50,6 +50,7 @@ const (
 	UnknownLintSource LintSource = iota
 	CABFBaselineRequirements
 	RFC5280
+	RFC5480
 	RFC5891
 	ZLint
 	AWSLabs
diff --git a/lints/lint_ecdsa_ee_invalid_ku.go b/lints/lint_ecdsa_ee_invalid_ku.go
new file mode 100644
index 00000000..03c46448
--- /dev/null
+++ b/lints/lint_ecdsa_ee_invalid_ku.go
@@ -0,0 +1,98 @@
+/*
+ * ZLint Copyright 2019 Regents of the University of Michigan
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package lints
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/zmap/zcrypto/x509"
+	"github.com/zmap/zlint/util"
+)
+
+type ecdsaInvalidKU struct{}
+
+// Initialize is a no-op for this lint.
+func (l *ecdsaInvalidKU) Initialize() error {
+	return nil
+}
+
+// CheckApplies returns true when the certificate is a subscriber cert using an
+// ECDSA public key algorithm.
+func (l *ecdsaInvalidKU) CheckApplies(c *x509.Certificate) bool {
+	return util.IsSubscriberCert(c) && c.PublicKeyAlgorithm == x509.ECDSA
+}
+
+// Execute returns a Notice level LintResult if the ECDSA end entity certificate
+// being linted has Key Usage bits set other than digitalSignature,
+// nonRepudiation/contentCommentment, and keyAgreement.
+func (l *ecdsaInvalidKU) Execute(c *x509.Certificate) *LintResult {
+	// RFC 5480, Section 3 "Key Usage Bits" says:
+	//
+	//   If the keyUsage extension is present in an End Entity (EE)
+	//   certificate that indicates id-ecPublicKey in SubjectPublicKeyInfo,
+	//   then any combination of the following values MAY be present:
+	//
+	//     digitalSignature;
+	//     nonRepudiation; and
+	//     keyAgreement.
+	//
+	// So we set up `allowedKUs` to match. Note that per RFC 5280: recent editions
+	// of X.509 renamed "nonRepudiation" to "contentCommitment", which is the name
+	// of the Go x509 constant we use here alongside the digitalSignature and
+	// keyAgreement constants.
+	allowedKUs := map[x509.KeyUsage]bool{
+		x509.KeyUsageDigitalSignature:  true,
+		x509.KeyUsageContentCommitment: true,
+		x509.KeyUsageKeyAgreement:      true,
+	}
+
+	var invalidKUs []string
+	for ku, kuName := range util.KeyUsageToString {
+		if c.KeyUsage&ku != 0 {
+			if !allowedKUs[ku] {
+				invalidKUs = append(invalidKUs, kuName)
+			}
+		}
+	}
+
+	if len(invalidKUs) > 0 {
+		// Sort the invalid KUs to allow consistent ordering of Details messages for
+		// unit testing
+		sort.Strings(invalidKUs)
+		return &LintResult{
+			Status: Notice,
+			Details: fmt.Sprintf(
+				"Certificate had unexpected key usage(s): %s",
+				strings.Join(invalidKUs, ", ")),
+		}
+	}
+
+	return &LintResult{
+		Status: Pass,
+	}
+}
+
+func init() {
+	RegisterLint(&Lint{
+		Name:          "n_ecdsa_ee_invalid_ku",
+		Description:   "ECDSA end-entity certificates MAY have key usages: digitalSignature, nonRepudiation and keyAgreement",
+		Citation:      "RFC 5480 Section 3",
+		Source:        RFC5480,
+		EffectiveDate: util.CABEffectiveDate,
+		Lint:          &ecdsaInvalidKU{},
+	})
+}
diff --git a/util/ku.go b/util/ku.go
new file mode 100644
index 00000000..31a828fb
--- /dev/null
+++ b/util/ku.go
@@ -0,0 +1,18 @@
+package util
+
+import "github.com/zmap/zcrypto/x509"
+
+var (
+	// KeyUsageToString maps an x509.KeyUsage bitmask to its name.
+	KeyUsageToString = map[x509.KeyUsage]string{
+		x509.KeyUsageDigitalSignature:  "KeyUsageDigitalSignature",
+		x509.KeyUsageContentCommitment: "KeyUsageContentCommitment",
+		x509.KeyUsageKeyEncipherment:   "KeyUsageKeyEncipherment",
+		x509.KeyUsageDataEncipherment:  "KeyUsageDataEncipherment",
+		x509.KeyUsageKeyAgreement:      "KeyUsageKeyAgreement",
+		x509.KeyUsageCertSign:          "KeyUsageCertSign",
+		x509.KeyUsageCRLSign:           "KeyUsageCRLSign",
+		x509.KeyUsageEncipherOnly:      "KeyUsageEncipherOnly",
+		x509.KeyUsageDecipherOnly:      "KeyUsageDecipherOnly",
+	}
+)


Test patch:
diff --git a/lints/lint_ecdsa_ee_invalid_ku_test.go b/lints/lint_ecdsa_ee_invalid_ku_test.go
new file mode 100644
index 00000000..ffd1edb3
--- /dev/null
+++ b/lints/lint_ecdsa_ee_invalid_ku_test.go
@@ -0,0 +1,51 @@
+package lints
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestECDSAInvalidKU(t *testing.T) {
+	testCases := []struct {
+		name            string
+		filename        string
+		expectedStatus  LintStatus
+		expectedDetails string
+	}{
+		{
+			name:           "non-ecdsa ee cert",
+			filename:       "rsaKeyWithParameters.pem",
+			expectedStatus: NA,
+		},
+		{
+			name:           "ecdsa ee cert, valid key usage",
+			filename:       "ecdsaP256ValidKUs.pem",
+			expectedStatus: Pass,
+		},
+		{
+			name:            "ecdsa ee cert, invalid key usage",
+			filename:        "ecdsaP384InvalidKUs.pem",
+			expectedStatus:  Notice,
+			expectedDetails: "Certificate had unexpected key usage(s): KeyUsageKeyEncipherment",
+		},
+		{
+			name:            "ecdsa ee cert, multiple invalid key usages",
+			filename:        "ecdsaP256.pem",
+			expectedStatus:  Notice,
+			expectedDetails: "Certificate had unexpected key usage(s): KeyUsageCRLSign, KeyUsageCertSign",
+		},
+	}
+
+	for _, tc := range testCases {
+		inputPath := fmt.Sprintf("%s%s", testCaseDir, tc.filename)
+		result := Lints["n_ecdsa_ee_invalid_ku"].Execute(ReadCertificate(inputPath))
+		if result.Status != tc.expectedStatus {
+			t.Errorf("expected result %v. actual result was %v",
+				tc.expectedStatus, result.Status)
+		}
+		if result.Details != tc.expectedDetails {
+			t.Errorf("expected details %q. actual result was %q",
+				tc.expectedDetails, result.Details)
+		}
+	}
+}
diff --git a/testlint/testCerts/ecdsaP256ValidKUs.pem b/testlint/testCerts/ecdsaP256ValidKUs.pem
new file mode 100644
index 00000000..6f405856
--- /dev/null
+++ b/testlint/testCerts/ecdsaP256ValidKUs.pem
@@ -0,0 +1,28 @@
+-----BEGIN CERTIFICATE-----
+MIIE0DCCBHegAwIBAgIQCP0zdXA01ESx6eTjfitzfzAKBggqhkjOPQQDAjBvMQsw
+CQYDVQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28x
+GTAXBgNVBAoTEENsb3VkRmxhcmUsIEluYy4xIDAeBgNVBAMTF0Nsb3VkRmxhcmUg
+SW5jIEVDQyBDQS0yMB4XDTE5MDUwMzAwMDAwMFoXDTIwMDUwMzEyMDAwMFowaDEL
+MAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2Nv
+MRkwFwYDVQQKExBDbG91ZEZsYXJlLCBJbmMuMRkwFwYDVQQDExBzY290dGhlbG1l
+LmNvLnVrMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEYQYIOYOkA0SNAxlrsvTn
+r7BIOoNmUaBFG2oXXyKfphkc/5wXXMU1Ez1+pqjFHSwaAtCkgTzTNEEOxkuDicJL
+qKOCAvowggL2MB8GA1UdIwQYMBaAFD50LR/PRXUEfj/Aooc+TEODURPGMB0GA1Ud
+DgQWBBSlm2TM83mhbWzqLL2SZRmdHbh/WDAvBgNVHREEKDAmghBzY290dGhlbG1l
+LmNvLnVrghIqLnNjb3R0aGVsbWUuY28udWswDgYDVR0PAQH/BAQDAgeAMB0GA1Ud
+JQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjB5BgNVHR8EcjBwMDagNKAyhjBodHRw
+Oi8vY3JsMy5kaWdpY2VydC5jb20vQ2xvdWRGbGFyZUluY0VDQ0NBMi5jcmwwNqA0
+oDKGMGh0dHA6Ly9jcmw0LmRpZ2ljZXJ0LmNvbS9DbG91ZEZsYXJlSW5jRUNDQ0Ey
+LmNybDBMBgNVHSAERTBDMDcGCWCGSAGG/WwBATAqMCgGCCsGAQUFBwIBFhxodHRw
+czovL3d3dy5kaWdpY2VydC5jb20vQ1BTMAgGBmeBDAECAjB2BggrBgEFBQcBAQRq
+MGgwJAYIKwYBBQUHMAGGGGh0dHA6Ly9vY3NwLmRpZ2ljZXJ0LmNvbTBABggrBgEF
+BQcwAoY0aHR0cDovL2NhY2VydHMuZGlnaWNlcnQuY29tL0Nsb3VkRmxhcmVJbmNF
+Q0NDQS0yLmNydDAMBgNVHRMBAf8EAjAAMIIBAwYKKwYBBAHWeQIEAgSB9ASB8QDv
+AHUA7ku9t3XOYLrhQmkfq+GeZqMPfl+wctiDAMR7iXqo/csAAAFqfaV7eQAABAMA
+RjBEAiAcD2ch9qAysZxlIFqrYjElyv+6fPbwBZ+CFSgJW7d4cgIgUnpOdauBy9OX
+IeQerY0ElxqlPDFo0qTx34NBoPmfyOYAdgBep3P531bA57U2SH3QSeAyepGaDISh
+EhKEGHWWgXFFWAAAAWp9pXsQAAAEAwBHMEUCIQDlDcpBkwV225tooGiDmWX4bwvp
+ko6nmXnQ8RBeSltUGAIgXUCskAznLKPcqf7e55eEj9xwago28rJZny6stP9Nqakw
+CgYIKoZIzj0EAwIDRwAwRAIfVXERnB1UkKpq73xsv0H3o0SC/VGdyuIizTc1HHeP
+6gIhAKSIp4zfgOLP3l46y2yHzir9PdeCc+RGUVg4P4IgAANv
+-----END CERTIFICATE-----
diff --git a/testlint/testCerts/ecdsaP384InvalidKUs.pem b/testlint/testCerts/ecdsaP384InvalidKUs.pem
new file mode 100644
index 00000000..9be37e31
--- /dev/null
+++ b/testlint/testCerts/ecdsaP384InvalidKUs.pem
@@ -0,0 +1,23 @@
+-----BEGIN CERTIFICATE-----
+MIIDzjCCA1SgAwIBAgIIJSCuKluilGIwCgYIKoZIzj0EAwMwWzELMAkGA1UEBhMC
+SlAxKjAoBgNVBAoTIU5hdGlvbmFsIEluc3RpdHV0ZSBvZiBJbmZvcm1hdGljczEg
+MB4GA1UEAxMXTklJIE9wZW4gRG9tYWluIENBIC0gRzYwHhcNMTgwOTEzMDczNzA5
+WhcNMjAxMDE0MDczNzA5WjCBkjELMAkGA1UEBhMCSlAxDjAMBgNVBAgTBVRva3lv
+MSowKAYDVQQKEyFOYXRpb25hbCBJbnN0aXR1dGUgb2YgSW5mb3JtYXRpY3MxKjAo
+BgNVBAsTIU5hdGlvbmFsIFJlc2VhcmNoIEdyaWQgSW5pdGlhdGl2ZTEbMBkGA1UE
+AxMSY2EtcGVyZi5uYXJlZ2kub3JnMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE/SD/
+v76aTq/hyAOl2S2NJ5CI3nsbJUKXbOI3g7pvSBe6sLbB3DjSywtoUdQaOMzAafBp
+9LRbuw5yAgxK849ihwwFXcWj8tQhMx8pQQA4DsVXEeHxRW+oyC8RlxaRbDj1o4IB
+qzCCAacwHwYDVR0jBBgwFoAUOXvriXvlLtX3HpUUyu6u3ViURrAwPAYIKwYBBQUH
+AQEEMDAuMCwGCCsGAQUFBzABhiBodHRwOi8vbmlpZzYub2NzcC5zZWNvbXRydXN0
+Lm5ldDA1BgNVHREELjAsghJjYS1wZXJmLm5hcmVnaS5vcmeCFnhuLS11OGp0YTdl
+Lm5hcmVnaS5vcmcwYAYDVR0gBFkwVzBLBgwrBgEEAYH8CAMCAQEwOzA5BggrBgEF
+BQcCARYtaHR0cHM6Ly9yZXBvMS5zZWNvbXRydXN0Lm5ldC9zcHBjYS9uaWkvb2Rj
+YTMvMAgGBmeBDAECAjAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwSgYD
+VR0fBEMwQTA/oD2gO4Y5aHR0cDovL3JlcG8xLnNlY29tdHJ1c3QubmV0L3NwcGNh
+L25paS9vZGNhMy9mdWxsY3JsZzYuY3JsMB0GA1UdDgQWBBSxxMIAGlGI05wtKzNn
+mipBOHlakzAOBgNVHQ8BAf8EBAMCBaAwEwYKKwYBBAHWeQIEAwEB/wQCBQAwCgYI
+KoZIzj0EAwMDaAAwZQIxANiRIu/n0xKniXhqUAWLoKobyfL1qKTUrwP0lIASKn+G
+nwALkoA4VYPW/rYL9yODKgIwcbwNYE1G182mZDm14E1rYA05Qr3YhC9555ARjIwX
+q6nn1ujfCspce2yug4YPtyY5
+-----END CERTIFICATE-----