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

refactor of SMIME aia contains #777

Merged
merged 28 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6c23670
lint about the encoding of qcstatements for PSD2
Feb 4, 2020
4666bb7
Revert "lint about the encoding of qcstatements for PSD2"
Feb 4, 2020
01996c6
Merge https://github.com/zmap/zlint
Aug 26, 2020
28481cc
Merge https://github.com/zmap/zlint
Sep 1, 2021
749d896
Merge https://github.com/zmap/zlint
Oct 21, 2021
e56e2a0
util: gtld_map autopull updates for 2021-10-21T07:25:20 UTC
web-flow Oct 21, 2021
8600050
Merge pull request #1 from mtgag/zlint-gtld-update
mtgag Oct 21, 2021
30b096e
Merge https://github.com/zmap/zlint
mtgag Apr 19, 2023
92e659c
always check and perform the operation in the execution
mtgag Apr 27, 2023
351a379
Merge branch 'master' into master
christopher-henderson May 14, 2023
b52111b
Merge https://github.com/zmap/zlint
mtgag May 16, 2023
526f9be
Merge https://github.com/zmap/zlint
mtgag Jun 9, 2023
92902fc
Merge https://github.com/zmap/zlint
mtgag Jul 1, 2023
1652cfa
synchronised with project
mtgag Jul 5, 2023
d4f2f9f
synchronised with project
mtgag Aug 30, 2023
88c933e
Merge https://github.com/zmap/zlint
mtgag Aug 30, 2023
cee805f
Merge https://github.com/zmap/zlint
mtgag Dec 3, 2023
87ee071
changed date, added check for existent extension
mtgag Dec 6, 2023
f1dea7f
updates in config after tests
mtgag Dec 6, 2023
530737b
removed accidentally commited file
mtgag Dec 6, 2023
a1eee50
removed internal names part, kept only has http only
mtgag Dec 9, 2023
2a6b887
changes addressing discussion in PR. Internal names are checked, IP a…
mtgag Dec 10, 2023
313bed4
the check for HTTP scheme is not needed here. This is covered by the …
mtgag Dec 10, 2023
cb0e939
Merge branch 'zmap:master' into smime_aia_contains_refactor
mtgag Dec 10, 2023
8a5d97c
fixed test
mtgag Dec 10, 2023
447c0a0
Merge branch 'smime_aia_contains_refactor' of https://github.com/mtga…
mtgag Dec 10, 2023
29eaf04
renamed file
mtgag Dec 11, 2023
a5711df
one lint for internal names in AIA covers all S/MIME generations, leg…
mtgag Dec 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions v3/integration/small.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,7 @@
},
"e_subject_email_max_length": {},
"e_subject_empty_without_san": {},
"e_subject_given_name_max_length": {
"ErrCount": 1
},
"e_subject_given_name_max_length": {},
"e_subject_info_access_marked_critical": {},
"e_subject_locality_name_max_length": {},
"e_subject_not_dn": {},
Expand Down Expand Up @@ -322,6 +320,12 @@
"e_rsa_allowed_ku_ee": {
"ErrCount": 11
},
"e_no_underscores_before_1_6_2": {
"ErrCount": 13
},
"e_incorrect_ku_encoding": {
"ErrCount": 239
},
"n_ca_digital_signature_not_set": {
"NoticeCount": 29
},
Expand Down Expand Up @@ -423,6 +427,12 @@
"w_subject_dn_trailing_whitespace": {
"WarnCount": 4
},
"w_tls_server_cert_valid_time_longer_than_397_days": {}
"w_tls_server_cert_valid_time_longer_than_397_days": {},
"w_rfc_dnsname_underscore_in_trd": {
"WarnCount": 13
},
"w_sub_cert_aia_contains_internal_names": {
"WarnCount": 7
}
}
}
34 changes: 19 additions & 15 deletions v3/lints/cabf_smime_br/lint_strict_aia_contains_internal_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package cabf_smime_br
*/

import (
"net"
"net/url"
"time"

Expand All @@ -23,7 +24,7 @@ import (
"github.com/zmap/zlint/v3/util"
)

type smimeStrictAIAContainsInternalNames struct{}
type smimeAIAContainsInternalNames struct{}

/************************************************************************
BRs: 7.1.2.3c
Expand All @@ -34,39 +35,40 @@ values for each of the following types:
id-ad-ocsp specifies the URI of the Issuing CA's OCSP responder.
id-ad-caIssuers specifies the URI of the Issuing CA's Certificate.

For Strict and Multipurpose: When provided, every accessMethod SHALL have the URI scheme HTTP. Other schemes SHALL NOT be present.
*************************************************************************/

func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "w_smime_strict_aia_contains_internal_names",
Description: "SMIME Strict certificates authorityInformationAccess When provided, every accessMethod SHALL have the URI scheme HTTP. Other schemes SHALL NOT be present.",
Name: "w_smime_aia_contains_internal_names",
mtgag marked this conversation as resolved.
Show resolved Hide resolved
Description: "SMIME certificates authorityInformationAccess. Internal domain names should not be included.",
Citation: "BRs: 7.1.2.3c",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewSMIMEStrictAIAInternalName,
Lint: NewSMIMEAIAInternalName,
})
}

func NewSMIMEStrictAIAInternalName() lint.LintInterface {
return &smimeStrictAIAContainsInternalNames{}
func NewSMIMEAIAInternalName() lint.LintInterface {
return &smimeAIAContainsInternalNames{}
}

func (l *smimeStrictAIAContainsInternalNames) CheckApplies(c *x509.Certificate) bool {
return util.IsStrictSMIMECertificate(c) || util.IsMultipurposeSMIMECertificate(c)
func (l *smimeAIAContainsInternalNames) CheckApplies(c *x509.Certificate) bool {
return util.IsExtInCert(c, util.AiaOID) && util.IsSMIMEBRCertificate(c)
}

func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {
func (l *smimeAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {
for _, u := range c.OCSPServer {
purl, err := url.Parse(u)
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}

if net.ParseIP(purl.Host) != nil {
continue
}

if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
Expand All @@ -76,9 +78,11 @@ func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}

if net.ParseIP(purl.Host) != nil {
continue
}

if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,29 @@ func TestSMIMEStrictAIAInternalName(t *testing.T) {
ExpectedResult: lint.Pass,
},
{
Name: "warn - aia with internal names",
Name: "warn - aia with internal names in AIA OCSP ",
InputFilename: "smime/aiaWithInternalNamesStrict.pem",
ExpectedResult: lint.Warn,
},
{
Name: "warn - aia with internal names in AIA CA issuers ",
InputFilename: "smime/aiaWithInternalNamesCaIssuersStrict.pem",
ExpectedResult: lint.Warn,
},
{
Name: "warn - aia with valid names, one is ldap",
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this version of the lint will now cover Legacy profile, which currently allows LDAP scheme to appear in the AIA. That may be unintentional or just missed. Along with that, I think if we're going to do this refactor for the Strict/MP profiles, that we should also do it for the legacy profiles which are additional lints that should be reviewed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the contains_internal_names lint, it is checked whether it is a BR-SMIME certificate, the scheme part is not checked at all and only the host part of the URL is checked. The certificate containing the LDAP URL is a Pass Test. I believe the current implementation covers these apsects. Has something not been taken into account? If there is something missing or is not correct I could extend the implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks. So, with this lint now covering Legacy, I think we should refactor the Legacy profile lint to only ensure the proper schemes, right? I would think we could just check that at least one OCSP URL and issuer url have the http scheme and remove the internal name checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commit a5711df covers this.

InputFilename: "smime/aiaWithLDAPOCSPStrict.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - aia with IP address in host part of the URL",
InputFilename: "smime/aiaWithIPAddress.pem",
ExpectedResult: lint.Pass,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_smime_strict_aia_contains_internal_names", tc.InputFilename)
result := test.TestLint("w_smime_aia_contains_internal_names", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
Expand Down
80 changes: 80 additions & 0 deletions v3/lints/cabf_smime_br/lint_strict_aia_has_http_only.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package cabf_smime_br

/*
* ZLint Copyright 2023 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 (
"net/url"

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

type smimeStrictAIAHasHTTPOnly struct{}

/************************************************************************
BRs: 7.1.2.3c
CA Certificate Authority Information Access
The authorityInformationAccess extension MAY contain one or more accessMethod
values for each of the following types:

id-ad-ocsp specifies the URI of the Issuing CA's OCSP responder.
id-ad-caIssuers specifies the URI of the Issuing CA's Certificate.

For Strict and Multipurpose: When provided, every accessMethod SHALL have the URI scheme HTTP. Other schemes SHALL NOT be present.
*************************************************************************/

func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "e_smime_strict_aia_shall_have_http_only",
Description: "SMIME Strict certificates authorityInformationAccess. When provided, every accessMethod SHALL have the URI scheme HTTP. Other schemes SHALL NOT be present.",
Citation: "BRs: 7.1.2.3c",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewSMIMEStrictAIAHasHTTPOnly,
})
}

func NewSMIMEStrictAIAHasHTTPOnly() lint.LintInterface {
return &smimeStrictAIAHasHTTPOnly{}
}

func (l *smimeStrictAIAHasHTTPOnly) CheckApplies(c *x509.Certificate) bool {
return util.IsExtInCert(c, util.AiaOID) && (util.IsStrictSMIMECertificate(c) || util.IsMultipurposeSMIMECertificate(c))
}

func (l *smimeStrictAIAHasHTTPOnly) Execute(c *x509.Certificate) *lint.LintResult {
for _, u := range c.OCSPServer {
purl, err := url.Parse(u)
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}
}
}
for _, u := range c.IssuingCertificateURL {
purl, err := url.Parse(u)
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}
}
}
return &lint.LintResult{Status: lint.Pass}
}
40 changes: 40 additions & 0 deletions v3/lints/cabf_smime_br/lint_strict_aia_has_http_only_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package cabf_smime_br

import (
"testing"

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

func TestSMIMEStrictAIAHasHTTPOnly(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - aia with valid names",
InputFilename: "smime/aiaWithValidNamesStrict.pem",
ExpectedResult: lint.Pass,
},
{
Name: "warn - aia with internal names",
InputFilename: "smime/aiaWithInternalNamesStrict.pem",
ExpectedResult: lint.Pass,
},
{
Name: "warn - aia with internal names",
InputFilename: "smime/aiaWithLDAPOCSPStrict.pem",
ExpectedResult: lint.Error,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_smime_strict_aia_shall_have_http_only", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
})
}
}
88 changes: 88 additions & 0 deletions v3/testdata/smime/aiaWithIPAddress.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number:
5a:35:62:65:45:00:06:4d:53:f4:df:9f
Signature Algorithm: sha256WithRSAEncryption
Issuer: CN = Lint CA, O = Lint, C = DE
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Sep 1 00:00:00 2024 GMT
Subject: CN = Certificate, O = Lint, C = DE
Subject Public Key Info:
Public Key Algorithm: rsaEncryption
RSA Public-Key: (2048 bit)
Modulus:
00:ce:e7:4c:52:f7:2b:a6:55:dd:21:2d:9e:85:e7:
a9:83:66:48:15:89:b1:4c:c2:63:08:fc:ec:86:ef:
9a:b3:96:42:f4:fd:1c:d8:65:a1:aa:c2:7c:92:69:
b6:59:44:40:c6:9b:73:11:16:7e:fb:73:4f:9c:31:
12:f9:0f:23:ef:8b:f2:7a:1b:96:7c:7d:1f:c2:75:
98:79:fc:13:74:9e:5f:60:6d:c4:4a:a8:07:08:2a:
ef:0f:82:43:ac:48:31:19:2d:e1:0f:f4:a1:f1:a6:
0b:fd:c1:45:e4:7c:d5:83:25:0e:17:3e:64:cc:0c:
62:1a:61:92:5f:c3:d7:92:b4:5e:7e:a6:db:d2:4f:
c3:e6:65:33:6d:1d:42:3e:4d:d2:18:15:dc:84:3c:
b0:02:da:aa:87:72:6a:d1:03:3d:ce:bc:67:ec:3c:
1e:11:bc:d6:df:ea:4b:91:ec:70:3b:9e:05:fe:6c:
97:8a:34:a5:45:67:83:37:51:f5:40:1a:6d:07:b6:
f8:e8:3d:9e:25:51:4c:59:20:96:71:b3:d2:81:b0:
44:34:7b:d8:8e:07:36:08:de:d9:15:42:2c:ca:1a:
0c:e5:5d:f1:9c:da:af:20:98:e9:76:af:3f:78:22:
a2:32:b4:c2:a2:3a:9c:3c:30:ec:e7:b9:e9:b9:4e:
28:1f
Exponent: 65537 (0x10001)
X509v3 extensions:
X509v3 Authority Key Identifier:
keyid:D7:41:86:D3:AD:A1:D9:97:CE:38:D3:D5:F1:16:3B:A3:8C:67:95:44

X509v3 Subject Key Identifier:
07:6B:7A:FA:43:EF:B0:ED:01:BA:30:E2:5E:D0:3F:C5:C8:0F:A9:4E
X509v3 Extended Key Usage:
E-mail Protection
X509v3 Subject Alternative Name:
email:test@example.com
Authority Information Access:
OCSP - URI:http://192.0.2.42/ocsp

X509v3 Certificate Policies:
Policy: 2.23.140.1.5.1.3

Signature Algorithm: sha256WithRSAEncryption
26:53:97:a8:d3:b0:f6:d5:19:7d:3a:95:b0:ef:4c:75:e9:01:
d8:d2:41:1c:1f:82:22:14:c6:a3:01:eb:16:bb:c6:d4:ab:4a:
3e:8d:ee:9f:01:47:30:eb:d0:1f:e3:b6:cf:83:76:4e:95:e0:
3f:ca:1d:c9:2d:e8:58:c1:d0:45:1c:27:3d:07:78:0a:61:ec:
4f:9b:67:7c:fd:0e:ee:6d:0d:37:64:70:d7:c7:2f:84:60:66:
1e:1a:7a:0b:45:d9:91:e5:37:26:0b:1b:23:d7:da:10:37:f1:
66:90:3d:ed:89:27:34:2d:68:66:f4:ae:2e:0e:b0:0a:f7:dd:
e0:59:33:30:f2:19:7c:ac:28:4a:aa:fe:45:62:18:4b:2c:b5:
f8:b5:fd:02:c2:aa:cb:bc:0d:b3:77:f1:d7:fc:55:52:ea:23:
f7:d3:a0:09:52:78:be:7f:ef:3c:f3:2a:2d:87:15:54:84:db:
47:89:ec:ad:63:41:7c:41:67:58:bf:77:09:5b:b4:c8:63:a2:
3c:a3:df:8d:6d:2e:9c:28:34:3d:5d:79:57:33:05:d8:ec:27:
d6:d1:79:fd:d2:b2:5f:41:a1:e4:78:77:ce:31:ad:2e:e3:ea:
1e:26:72:e4:fc:09:84:3a:ee:92:37:fb:6d:c8:f1:13:40:e2:
45:a8:47:b4
-----BEGIN CERTIFICATE-----
MIIDpjCCAo6gAwIBAgIMWjViZUUABk1T9N+fMA0GCSqGSIb3DQEBCwUAMC4xEDAO
BgNVBAMMB0xpbnQgQ0ExDTALBgNVBAoMBExpbnQxCzAJBgNVBAYTAkRFMB4XDTIz
MDkwMTAwMDAwMFoXDTI0MDkwMTAwMDAwMFowMjEUMBIGA1UEAwwLQ2VydGlmaWNh
dGUxDTALBgNVBAoMBExpbnQxCzAJBgNVBAYTAkRFMIIBIjANBgkqhkiG9w0BAQEF
AAOCAQ8AMIIBCgKCAQEAzudMUvcrplXdIS2eheepg2ZIFYmxTMJjCPzshu+as5ZC
9P0c2GWhqsJ8kmm2WURAxptzERZ++3NPnDES+Q8j74vyehuWfH0fwnWYefwTdJ5f
YG3ESqgHCCrvD4JDrEgxGS3hD/Sh8aYL/cFF5HzVgyUOFz5kzAxiGmGSX8PXkrRe
fqbb0k/D5mUzbR1CPk3SGBXchDywAtqqh3Jq0QM9zrxn7DweEbzW3+pLkexwO54F
/myXijSlRWeDN1H1QBptB7b46D2eJVFMWSCWcbPSgbBENHvYjgc2CN7ZFUIsyhoM
5V3xnNqvIJjpdq8/eCKiMrTCojqcPDDs57npuU4oHwIDAQABo4G/MIG8MB8GA1Ud
IwQYMBaAFNdBhtOtodmXzjjT1fEWO6OMZ5VEMB0GA1UdDgQWBBQHa3r6Q++w7QG6
MOJe0D/FyA+pTjATBgNVHSUEDDAKBggrBgEFBQcDBDAbBgNVHREEFDASgRB0ZXN0
QGV4YW1wbGUuY29tMDIGCCsGAQUFBwEBBCYwJDAiBggrBgEFBQcwAYYWaHR0cDov
LzE5Mi4wLjIuNDIvb2NzcDAUBgNVHSAEDTALMAkGB2eBDAEFAQMwDQYJKoZIhvcN
AQELBQADggEBACZTl6jTsPbVGX06lbDvTHXpAdjSQRwfgiIUxqMB6xa7xtSrSj6N
7p8BRzDr0B/jts+Ddk6V4D/KHckt6FjB0EUcJz0HeAph7E+bZ3z9Du5tDTdkcNfH
L4RgZh4aegtF2ZHlNyYLGyPX2hA38WaQPe2JJzQtaGb0ri4OsAr33eBZMzDyGXys
KEqq/kViGEsstfi1/QLCqsu8DbN38df8VVLqI/fToAlSeL5/7zzzKi2HFVSE20eJ
7K1jQXxBZ1i/dwlbtMhjojyj341tLpwoND1deVczBdjsJ9bRef3Ssl9BoeR4d84x
rS7j6h4mcuT8CYQ67pI3+23I8RNA4kWoR7Q=
-----END CERTIFICATE-----
Loading
Loading