Repository: zmap/zlint
Pull request #309 (2019-08-12 22:12:08)

lints: add e_subject_printable_string_badalpha lint.
When the raw Subject RDNSequence of a Certificate includes a PrintableString type DirectoryString attribute the value of the attribute must adhere to the PrintableString character set defined in RFC 5280 Appendix B:

>The character string type PrintableString supports a very basic Latin character set: the lowercase letters 'a' through 'z', uppercase letters 'A' through 'Z', the digits '0' through '9', eleven special characters ' = ( ) + , - . / : ? and space.

If any of the PrintableString attributes in the linted Certificate's raw subject do not match a regexp for this character set an Error level lint result is returned by the `e_subject_printable_string_badalpha` lint.

_Adding a lint for this case has been on the back of my mind since an [unfortunate incident affecting an upstart CA back in 2018....](https://groups.google.com/d/msg/mozilla.dev.security.policy/wqySoetqUFM/34W5zvENAwAJ)_


Patch:
diff --git a/lints/lint_subject_printable_string_badalpha.go b/lints/lint_subject_printable_string_badalpha.go
new file mode 100644
index 00000000..b60e76f6
--- /dev/null
+++ b/lints/lint_subject_printable_string_badalpha.go
@@ -0,0 +1,108 @@
+/*
+ * 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 (
+	"encoding/asn1"
+	"errors"
+	"fmt"
+	"regexp"
+
+	"github.com/zmap/zcrypto/x509"
+	"github.com/zmap/zlint/util"
+)
+
+var (
+	// Per RFC 5280, Appendix B. ASN.1 Notes:
+	//   The character string type PrintableString supports a very basic Latin
+	//   character set: the lowercase letters 'a' through 'z', uppercase
+	//   letters 'A' through 'Z', the digits '0' through '9', eleven special
+	//   characters ' = ( ) + , - . / : ? and space.
+	printableStringRegex = regexp.MustCompile(`^[a-zA-Z0-9\=\(\)\+,\-.\/:\? ]+$`)
+)
+
+// validatePrintableString returns an error if the provided encoded printable
+// string doesn't adhere to the character set defined in RFC 5280.
+func validatePrintableString(rawPS []byte) error {
+	if !printableStringRegex.Match(rawPS) {
+		return errors.New("encoded PrintableString contained illegal characters")
+	}
+	return nil
+}
+
+type subjectPrintableStringBadAlpha struct {
+}
+
+func (l *subjectPrintableStringBadAlpha) Initialize() error {
+	return nil
+}
+
+// CheckApplies returns true for any certificate with a non-empty RawSubject.
+func (l *subjectPrintableStringBadAlpha) CheckApplies(c *x509.Certificate) bool {
+	return len(c.RawSubject) > 0
+}
+
+// Execute checks the certificate's RawSubject to ensure that any
+// PrintableString attribute/value pairs in the Subject match the character set
+// defined for this type in RFC 5280. An Error level LintResult is returned if any
+// of the PrintableString attributes do not match a regular expression for the
+// allowed character set.
+func (l *subjectPrintableStringBadAlpha) Execute(c *x509.Certificate) *LintResult {
+	rdnSequence := util.RawRDNSequence{}
+	rest, err := asn1.Unmarshal(c.RawSubject, &rdnSequence)
+	if err != nil {
+		return &LintResult{
+			Status:  Fatal,
+			Details: "Failed to Unmarshal RawSubject into RawRDNSequence",
+		}
+	}
+	if len(rest) > 0 {
+		return &LintResult{
+			Status:  Fatal,
+			Details: "Trailing data after RawSubject RawRDNSequence",
+		}
+	}
+
+	for _, attrTypeAndValueSet := range rdnSequence {
+		for _, attrTypeAndValue := range attrTypeAndValueSet {
+			// If the attribute type is a PrintableString the bytes of the attribute
+			// value must match the printable string alphabet.
+			if attrTypeAndValue.Value.Tag == asn1.TagPrintableString {
+				if err := validatePrintableString(attrTypeAndValue.Value.Bytes); err != nil {
+					return &LintResult{
+						Status: Error,
+						Details: fmt.Sprintf("RawSubject attr oid %s %s",
+							attrTypeAndValue.Type, err.Error()),
+					}
+				}
+			}
+		}
+	}
+
+	return &LintResult{
+		Status: Pass,
+	}
+}
+
+func init() {
+	RegisterLint(&Lint{
+		Name:          "e_subject_printable_string_badalpha",
+		Description:   "PrintableString type's alphabet only includes a-z, A-Z, 0-9, and 11 special characters",
+		Citation:      "RFC 5280: Appendix B. ASN.1 Notes",
+		Source:        RFC5280,
+		EffectiveDate: util.RFC2459Date,
+		Lint:          &subjectPrintableStringBadAlpha{},
+	})
+}


Test patch:
diff --git a/lints/lint_subject_printable_string_badalpha_test.go b/lints/lint_subject_printable_string_badalpha_test.go
new file mode 100644
index 00000000..e993eaf5
--- /dev/null
+++ b/lints/lint_subject_printable_string_badalpha_test.go
@@ -0,0 +1,47 @@
+package lints
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestSubjectPrintableStringBadAlpha(t *testing.T) {
+	testCases := []struct {
+		name     string
+		filename string
+		expected LintResult
+	}{
+		{
+			name: "valid subj. PrintableStrings",
+			// A RawSubject containing 8 PrintableString attributes all adhering to
+			// the expected character set.
+			filename: "subjectCommonNameLengthGood.pem",
+			expected: LintResult{
+				Status: Pass,
+			},
+		},
+		{
+			name: "invalid subj. CN PrintableString",
+			// A RawSubject containing a single PrintableString attribute (OID
+			// 2.5.4.3, subject common name) with an illegal character (`*`).
+			filename: "subjectCommonNamePrintableStringBadAlpha.pem",
+			expected: LintResult{
+				Status:  Error,
+				Details: "RawSubject attr oid 2.5.4.3 encoded PrintableString contained illegal characters",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			inputPath := fmt.Sprintf("%s%s", testCaseDir, tc.filename)
+			result := Lints["e_subject_printable_string_badalpha"].Execute(ReadCertificate(inputPath))
+			if result.Status != tc.expected.Status {
+				t.Errorf("expected result status %v was %v", tc.expected.Status, result.Status)
+			}
+			if result.Details != tc.expected.Details {
+				t.Errorf("expected result details %q was %q", tc.expected.Details, result.Details)
+			}
+		})
+	}
+}
diff --git a/testlint/testCerts/subjectCommonNamePrintableStringBadAlpha.pem b/testlint/testCerts/subjectCommonNamePrintableStringBadAlpha.pem
new file mode 100644
index 00000000..8ee93608
--- /dev/null
+++ b/testlint/testCerts/subjectCommonNamePrintableStringBadAlpha.pem
@@ -0,0 +1,30 @@
+-----BEGIN CERTIFICATE-----
+MIIFNzCCBB+gAwIBAgISA3tMpMmBnmWlBSUX7fqT42sRMA0GCSqGSIb3DQEBCwUA
+MEoxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MSMwIQYDVQQD
+ExpMZXQncyBFbmNyeXB0IEF1dGhvcml0eSBYMzAeFw0xODAzMTIyMjI4MjZaFw0x
+ODA2MTAyMjI4MjZaMCcxJTAjBgNVBAMTHCouYjRhODdjLmF3cy5yYWRpYW50bG9j
+ay5vcmcwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQD8EXvSc2wv/BFs
+Lc6FrkmAeBP+1BxP7dKUQ8k151sf7fXlGHL4HemT0geebA/ca09zjfAutcjoEal5
+8v3ECFWMo62kaTJfiLi4n+V0o+Ww24tY+7MHKmcUeYRexLcynh2tdQ4UUflNqh4C
+gNhBmpBG/N4uQCvfuM4HXWQsnco2aiJd1JarnOAUbscvm0OtVM1EvlfkQKwK6hkS
+TbjX6M7qg1u/kS7SBBk8YHz7+l6lF+hh4D2j0z36w9Zft2zEir7jkC/GKAghvTP8
+jAn8JtsWq6jKWbxm6NOYKNkthnjQy2EsO1rcpVrpIJ1FCBJoUWFY6jU3m4ESZ3hz
+N81fQWU7AgMBAAGjggI4MIICNDAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYI
+KwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFPyzheTX
+SwE4ZgtC288ESi2V9okRMB8GA1UdIwQYMBaAFKhKamMEfd265tE5t6ZFZe/zqOyh
+MG8GCCsGAQUFBwEBBGMwYTAuBggrBgEFBQcwAYYiaHR0cDovL29jc3AuaW50LXgz
+LmxldHNlbmNyeXB0Lm9yZzAvBggrBgEFBQcwAoYjaHR0cDovL2NlcnQuaW50LXgz
+LmxldHNlbmNyeXB0Lm9yZy8wQwYDVR0RBDwwOoIcKi5iNGE4N2MuYXdzLnJhZGlh
+bnRsb2NrLm9yZ4IaYjRhODdjLmF3cy5yYWRpYW50bG9jay5vcmcwgf4GA1UdIASB
+9jCB8zAIBgZngQwBAgEwgeYGCysGAQQBgt8TAQEBMIHWMCYGCCsGAQUFBwIBFhpo
+dHRwOi8vY3BzLmxldHNlbmNyeXB0Lm9yZzCBqwYIKwYBBQUHAgIwgZ4MgZtUaGlz
+IENlcnRpZmljYXRlIG1heSBvbmx5IGJlIHJlbGllZCB1cG9uIGJ5IFJlbHlpbmcg
+UGFydGllcyBhbmQgb25seSBpbiBhY2NvcmRhbmNlIHdpdGggdGhlIENlcnRpZmlj
+YXRlIFBvbGljeSBmb3VuZCBhdCBodHRwczovL2xldHNlbmNyeXB0Lm9yZy9yZXBv
+c2l0b3J5LzANBgkqhkiG9w0BAQsFAAOCAQEAR2ckU/ILsRLXuPuyk39SCoBHL+RF
+4yUq/+DLnR7FwGXi0/AC1ZKTSCe+ya+Z9tH8V9uDtV4l+qQURnFoCMVom7bwC2nb
+gAMaLvbnB/qOdWEHG2qbBcC+Ec++0GkuMtysGdKcGgIHBeQIP4AwNHzv0jIfJwvq
+7yJ55VFMDWceEf3vgwfs/j/QzV8Jse5sAg/QkcGQmrNTdqb8qOf2mOXYvend03sA
+e88+fCZUqQSLhwDB0jEhUvtZD/jtI9tAa2lQ5b0lQjAI4ueiKza9TedEb46ZX36w
+0nEa1uLJ1TpBIoGU+NRB5UWPJN1l8d18CFWe2hDdRDxahSgCtvSdfpLopg==
+-----END CERTIFICATE-----