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.

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

Add lint to cover TLS BR v2 EKU checks #833

Merged
merged 11 commits into from
Apr 28, 2024
24 changes: 12 additions & 12 deletions v3/integration/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@
"e_cert_unique_identifier_version_not_2_or_3": {},
"e_distribution_point_incomplete": {},
"e_dnsname_bad_character_in_label": {
"ErrCount": 55927
"ErrCount": 55930
},
"e_dnsname_contains_bare_iana_suffix": {
"ErrCount": 8
Expand All @@ -400,7 +400,7 @@
"ErrCount": 17
},
"e_dnsname_not_valid_tld": {
"ErrCount": 86371
"ErrCount": 86374
},
"e_dnsname_underscore_in_sld": {
"ErrCount": 5
Expand Down Expand Up @@ -491,7 +491,7 @@
"ErrCount": 2
},
"e_ext_san_missing": {
"ErrCount": 52385
"ErrCount": 52388
},
"e_ext_san_no_entries": {
"ErrCount": 3
Expand Down Expand Up @@ -576,7 +576,7 @@
"ErrCount": 370
},
"e_ocsp_id_pkix_ocsp_nocheck_ext_not_included_server_auth": {
"ErrCount": 93
"ErrCount": 95
},
"e_old_root_ca_rsa_mod_less_than_2048_bits": {
"ErrCount": 1
Expand Down Expand Up @@ -711,7 +711,7 @@
"ErrCount": 81098
},
"e_sub_cert_eku_server_auth_client_auth_missing": {
"ErrCount": 4934
"ErrCount": 4943
},
"e_sub_cert_given_name_surname_contains_correct_policy": {
"ErrCount": 1793
Expand Down Expand Up @@ -751,7 +751,7 @@
"ErrCount": 2
},
"e_subject_common_name_not_from_san": {
"ErrCount": 94976
"ErrCount": 94979
},
"e_subject_contains_noninformational_value": {
"ErrCount": 338
Expand Down Expand Up @@ -818,7 +818,7 @@
},
"e_cab_dv_subject_invalid_values": {},
"n_ca_digital_signature_not_set": {
"NoticeCount": 1409
"NoticeCount": 1411
},
"n_contains_redacted_dnsname": {
"NoticeCount": 464
Expand All @@ -845,10 +845,10 @@
"NoticeCount": 1415
},
"n_sub_ca_eku_not_technically_constrained": {
"NoticeCount": 10
"NoticeCount": 12
},
"n_subject_common_name_included": {
"NoticeCount": 712639
"NoticeCount": 712866
},
"w_ct_sct_policy_count_unsatisfied": {
"NoticeCount": 5003
Expand Down Expand Up @@ -935,17 +935,17 @@
"WarnCount": 9
},
"w_sub_ca_name_constraints_not_critical": {
"WarnCount": 115
"WarnCount": 116
},
"w_sub_cert_aia_contains_internal_names": {
"WarnCount": 210
},
"w_sub_cert_aia_does_not_contain_issuing_ca_url": {
"WarnCount": 48465
"WarnCount": 48469
},
"w_sub_cert_certificate_policies_marked_critical": {},
"w_sub_cert_eku_extra_values": {
"WarnCount": 25405
"WarnCount": 25412
},
"w_sub_cert_sha1_expiration_too_long": {
"WarnCount": 11058
Expand Down
2 changes: 1 addition & 1 deletion v3/integration/small.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@
},
"n_sub_ca_eku_not_technically_constrained": {},
"n_subject_common_name_included": {
"NoticeCount": 19776
"NoticeCount": 19785
},
"w_ct_sct_policy_count_unsatisfied": {
"NoticeCount": 176
Expand Down
2 changes: 1 addition & 1 deletion v3/lints/cabf_br/lint_sub_ca_eku_valid_fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestSubCAEKUValidFields(t *testing.T) {

func TestSubCAEKUNotValidFields(t *testing.T) {
inputPath := "subCAEKUNotValidFields.pem"
expected := lint.NA
expected := lint.Notice
out := test.TestLint("n_sub_ca_eku_not_technically_constrained", inputPath)
if out.Status != expected {
t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
Expand Down
81 changes: 81 additions & 0 deletions v3/lints/cabf_br/lint_sub_cert_eku_check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package cabf_br

/*
* ZLint Copyright 2024 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.
*/

import (
"fmt"

"github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/util"
)

type subExtKeyUsageCheck struct{}

func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "e_sub_cert_eku_check",
Description: "Subscriber certificates MUST have id-kp-serverAuth and MAY have id-kp-clientAuth present in extKeyUsage",
Citation: "BRs: 7.1.2.7.10 Subscriber Certificate Extended Key Usage",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABFBRs_2_0_0_Date,
},
Lint: NewSubExtKeyUsageCheck,
})
}

func NewSubExtKeyUsageCheck() lint.LintInterface {
return &subExtKeyUsageCheck{}
}

func (l *subExtKeyUsageCheck) CheckApplies(c *x509.Certificate) bool {
return util.IsSubscriberCert(c) && util.IsExtInCert(c, util.EkuSynOid)
}

func (l *subExtKeyUsageCheck) Execute(c *x509.Certificate) *lint.LintResult {
var hasClientAuthEKU, hasServerAuthEKU bool

for _, eku := range c.ExtKeyUsage {
switch eku {
case x509.ExtKeyUsageServerAuth:
hasServerAuthEKU = true

case x509.ExtKeyUsageClientAuth:
hasClientAuthEKU = true

case x509.ExtKeyUsageAny, x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping,
x509.ExtKeyUsageOcspSigning, x509.ExtKeyUsageEmailProtection:

return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("%s MUST NOT be present", util.GetEKUString(eku))}
}
}

if !hasServerAuthEKU {
return &lint.LintResult{Status: lint.Error, Details: "id-kp-serverAuth MUST be present"}
}

for _, eku := range c.UnknownExtKeyUsage {
if eku.Equal(util.PreCertificateSigningCertificateEKU) {
return &lint.LintResult{Status: lint.Error, Details: "Precertificate Signing Certificate extKeyUsage MUST NOT be present"}
}
}

if (len(c.ExtKeyUsage) > 2 && !hasClientAuthEKU) || len(c.UnknownExtKeyUsage) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could set a variable in the default case in the range c.ExtKeyUsage loop and check that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may advocate for keeping a list of of OIDs found during the execution of the lint and cross referencing them at them end against the list of known OIDs.

var known = map[string]struct{}{
	"1.3.6.1.5.5.7.3.1": {},
	"1.3.6.1.5.5.7.3.2": {},
	"1.3.6.1.5.5.7.3.3": {},
	"1.3.6.1.5.5.7.3.4": {},
	"1.3.6.1.5.5.7.3.8": {},
	"1.3.6.1.5.5.7.3.9": {},
	"2.5.29.37.0": {},
	"1.3.6.1.4.1.11129.2.4.4": {},
}

func lint() {
    found := []string
    for _, eku := range c.ExtKeyUsage {
        found = append(found, eku.String()
        ....
        ....
        ....
    }
    for _, eku := range.UnknownExtKeyUsage {
        found = append(found, eku.String()
        ....
        ....
        ....
    }
    for _, oid := range found {
        _, ok := known[oid]
        if !ok {
            return &lint.LintResult{Status: lint.Warn, Details: "any other value than id-kp-serverAuth and id-kp-clientAuth is NOT RECOMMENDED"}
        }
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ExtKeyUsage and UnknownExtKeyUsage are not of the same type:

ExtKeyUsage        []ExtKeyUsage           // Sequence of extended key usages.
UnknownExtKeyUsage []asn1.ObjectIdentifier // Encountered extended key usages unknown to this package.

The lint currently checks for MUST, MUST NOT, MAY, and NOT RECOMMENDED, and returns the corresponding lint status and error.

We can create a map with each EKU and the requirement (i.e., MUST) and lookup the value while ranging over the two EKU lists, but I guess this would be less efficient

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon that this would be an efficiency modifier in the range of nanoseconds (possibly into ms) for each run of the lint. In general, legibility trumps such optimizations since correctness of the lint is paramount.

return &lint.LintResult{Status: lint.Warn, Details: "any other value than id-kp-serverAuth and id-kp-clientAuth is NOT RECOMMENDED"}
}

return &lint.LintResult{Status: lint.Pass}
}
75 changes: 75 additions & 0 deletions v3/lints/cabf_br/lint_sub_cert_eku_check_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package cabf_br

/*
* ZLint Copyright 2024 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.
*/

import (
"testing"

"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/test"
)

func TestSubExtKeyUsageCheck(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - serverAuth EKU",
InputFilename: "subExtKeyUsageServerAuth.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - serverAuth and clientAuth EKU",
InputFilename: "subExtKeyUSageServerAndClientAuth.pem",
ExpectedResult: lint.Pass,
},
{
Name: "error - only clientAuth EKU with CA/B TLS BR policy OID",
InputFilename: "subExtKeyUsageClientAuth.pem",
ExpectedResult: lint.Error,
},
{
Name: "ne - only clientAuth EKU, with CA/B TLS BR policy OID, NotBefore before effective date",
InputFilename: "subExtKeyUsageClientAuthPreBRv2.pem",
ExpectedResult: lint.NE,
},
{
Name: "error - serverAuth and timeStamping EKU",
InputFilename: "subExtKeyUsageServerAuthAndTimeStamping.pem",
ExpectedResult: lint.Error,
},
{
Name: "error - serverAuth and unknown PreCertSigCert EKU",
InputFilename: "subExtKeyUsageServerAuthAndPreCertSigCert.pem",
ExpectedResult: lint.Error,
},
{
Name: "warn - serverAuth and MicrosoftDocumentSigning EKU",
InputFilename: "subExtKeyUsageServerAuthAndUnknown.pem",
ExpectedResult: lint.Warn,
},
}

for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_sub_cert_eku_check", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
})
}
}
11 changes: 6 additions & 5 deletions v3/lints/cabf_br/lint_sub_cert_eku_extra_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ present.
func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "w_sub_cert_eku_extra_values",
Description: "Subscriber Certificate: extKeyUsage values other than id-kp-serverAuth, id-kp-clientAuth, and id-kp-emailProtection SHOULD NOT be present.",
Citation: "BRs: 7.1.2.3",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
Name: "w_sub_cert_eku_extra_values",
Description: "Subscriber Certificate: extKeyUsage values other than id-kp-serverAuth, id-kp-clientAuth, and id-kp-emailProtection SHOULD NOT be present.",
Citation: "BRs: 7.1.2.3",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
IneffectiveDate: util.CABFBRs_2_0_0_Date,
},
Lint: NewSubExtKeyUsageLegalUsage,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ present.
func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "e_sub_cert_eku_server_auth_client_auth_missing",
Description: "Subscriber certificates MUST have either id-kp-serverAuth or id-kp-clientAuth or both present in extKeyUsage",
Citation: "BRs: 7.1.2.3",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
Name: "e_sub_cert_eku_server_auth_client_auth_missing",
Description: "Subscriber certificates MUST have either id-kp-serverAuth or id-kp-clientAuth or both present in extKeyUsage",
Citation: "BRs: 7.1.2.3",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
IneffectiveDate: util.CABFBRs_2_0_0_Date,
},
Lint: NewSubExtKeyUsageClientOrServer,
})
Expand Down
38 changes: 38 additions & 0 deletions v3/testdata/subExtKeyUSageServerAndClientAuth.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 15 12:00:00 2023 GMT
Not After : Sep 15 12:00:00 2024 GMT
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:f1:d5:c1:b1:f9:0f:63:d7:c2:ba:83:a3:c6:b7:
fb:cf:07:bb:c7:33:03:61:4d:13:0f:45:46:8d:47:
7f:41:ce:8f:9f:1f:85:37:ee:ff:30:00:37:3c:53:
8b:60:aa:38:48:a8:5c:ca:c7:0c:78:12:f4:2e:ca:
62:d8:f7:6c:b5
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Extended Key Usage:
TLS Web Server Authentication, TLS Web Client Authentication
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:45:02:20:79:05:6d:37:0a:db:85:a5:6f:54:68:1d:37:50:
33:ce:4a:97:d6:59:77:d3:98:1b:df:89:43:f9:02:d8:e2:91:
02:21:00:83:cd:55:a0:54:f1:88:be:71:23:96:7b:4c:38:6a:
d6:14:2c:dc:e0:75:e8:51:c3:bd:bc:09:74:63:5e:3d:7b
-----BEGIN CERTIFICATE-----
MIIBDzCBtqADAgECAgEDMAoGCCqGSM49BAMCMAAwHhcNMjMwOTE1MTIwMDAwWhcN
MjQwOTE1MTIwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8dXBsfkP
Y9fCuoOjxrf7zwe7xzMDYU0TD0VGjUd/Qc6Pnx+FN+7/MAA3PFOLYKo4SKhcyscM
eBL0Lspi2PdstaMhMB8wHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMAoG
CCqGSM49BAMCA0gAMEUCIHkFbTcK24Wlb1RoHTdQM85Kl9ZZd9OYG9+JQ/kC2OKR
AiEAg81VoFTxiL5xI5Z7TDhq1hQs3OB16FHDvbwJdGNePXs=
-----END CERTIFICATE-----
40 changes: 40 additions & 0 deletions v3/testdata/subExtKeyUsageClientAuth.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 15 12:00:00 2023 GMT
Not After : Sep 15 12:00:00 2024 GMT
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:04:3b:ca:38:58:01:84:3c:49:f1:45:a8:b6:02:
18:00:dc:f3:3d:78:2b:d0:ff:3d:16:62:07:4d:2e:
63:e0:92:0b:45:fc:f8:90:90:ce:bf:b6:db:ba:60:
e5:d2:fb:b0:f7:d1:39:18:7a:40:a9:7d:e3:22:ac:
ef:e9:f6:70:4c
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Extended Key Usage:
TLS Web Client Authentication
X509v3 Certificate Policies:
Policy: 2.23.140.1.2.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:44:02:20:25:79:7b:74:45:0d:29:f9:24:f0:81:4d:03:e4:
e0:b6:01:19:31:ee:dc:81:e6:bb:2a:88:91:ae:cd:d5:8e:5a:
02:20:03:2f:9a:da:62:e6:d4:8f:96:99:f4:ea:22:0d:23:e0:
a4:9b:eb:42:08:9f:47:ff:50:ba:e7:f1:e7:9f:15:39
-----BEGIN CERTIFICATE-----
MIIBGTCBwaADAgECAgEDMAoGCCqGSM49BAMCMAAwHhcNMjMwOTE1MTIwMDAwWhcN
MjQwOTE1MTIwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEBDvKOFgB
hDxJ8UWotgIYANzzPXgr0P89FmIHTS5j4JILRfz4kJDOv7bbumDl0vuw99E5GHpA
qX3jIqzv6fZwTKMsMCowEwYDVR0lBAwwCgYIKwYBBQUHAwIwEwYDVR0gBAwwCjAI
BgZngQwBAgEwCgYIKoZIzj0EAwIDRwAwRAIgJXl7dEUNKfkk8IFNA+TgtgEZMe7c
gea7KoiRrs3VjloCIAMvmtpi5tSPlpn06iINI+Ckm+tCCJ9H/1C65/HnnxU5
-----END CERTIFICATE-----
Loading
Loading