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 all 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
}
}
}
91 changes: 91 additions & 0 deletions v3/lints/cabf_smime_br/lint_aia_contains_internal_names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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"
"net/url"
"time"

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

type smimeAIAContainsInternalNames 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.

*************************************************************************/

func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "w_smime_aia_contains_internal_names",
Description: "SMIME certificates authorityInformationAccess. Internal domain names should not be included.",
Citation: "BRs: 7.1.2.3c",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewSMIMEAIAInternalName,
})
}

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

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

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 net.ParseIP(purl.Host) != nil {
continue
}

if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
}
for _, u := range c.IssuingCertificateURL {
purl, err := url.Parse(u)
if err != nil {
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}
}
}
return &lint.LintResult{Status: lint.Pass}
}
50 changes: 50 additions & 0 deletions v3/lints/cabf_smime_br/lint_aia_contains_internal_names_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package cabf_smime_br

import (
"testing"

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

func TestSMIMEStrictAIAInternalName(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 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",
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_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)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ package cabf_smime_br

import (
"net/url"
"time"

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

type smimeLegacyAIAContainsInternalNames struct{}
type smimeLegacyAIAHasOneHTTP struct{}

/************************************************************************
BRs: 7.1.2.3c
Expand All @@ -40,40 +39,37 @@ For Legacy: When provided, at least one accessMethod SHALL have the URI scheme H
func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "w_smime_legacy_aia_contains_internal_names",
Name: "e_smime_legacy_aia_shall_have_one_http",

Choose a reason for hiding this comment

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

Good catch on the change from w to e. It really should have been marked as an error class in the first place.

Description: "SMIME Legacy certificates authorityInformationAccess When provided, at least one accessMethod SHALL have the URI scheme HTTP. Other schemes (LDAP, FTP, ...) MAY be present.",
Citation: "BRs: 7.1.2.3c",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewSMIMELegacyAIAInternalName,
Lint: NewSMIMELegacyAIAHasOneHTTP,
})
}

func NewSMIMELegacyAIAInternalName() lint.LintInterface {
return &smimeLegacyAIAContainsInternalNames{}
func NewSMIMELegacyAIAHasOneHTTP() lint.LintInterface {
return &smimeLegacyAIAHasOneHTTP{}
}

func (l *smimeLegacyAIAContainsInternalNames) CheckApplies(c *x509.Certificate) bool {
return util.IsLegacySMIMECertificate(c)
func (l *smimeLegacyAIAHasOneHTTP) CheckApplies(c *x509.Certificate) bool {
return util.IsSubscriberCert(c) && util.IsExtInCert(c, util.AiaOID) && util.IsLegacySMIMECertificate(c)
}

func (l *smimeLegacyAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {
func (l *smimeLegacyAIAHasOneHTTP) Execute(c *x509.Certificate) *lint.LintResult {
atLeastOneHttp := false
for _, u := range c.OCSPServer {
purl, err := url.Parse(u)
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
if purl.Scheme == "http" {
atLeastOneHttp = true
}
}
if !atLeastOneHttp && len(c.OCSPServer) != 0 {
return &lint.LintResult{Status: lint.Error, Details: "at least one accessMethod MUST have the URI scheme HTTP"}
return &lint.LintResult{Status: lint.Error, Details: "at least one id-ad-ocsp accessMethod MUST have the URI scheme HTTP"}
}

atLeastOneHttp = false
Expand All @@ -82,15 +78,12 @@ func (l *smimeLegacyAIAContainsInternalNames) Execute(c *x509.Certificate) *lint
if err != nil {
return &lint.LintResult{Status: lint.Error}
}
if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
if purl.Scheme == "http" {
atLeastOneHttp = true
}
}
if !atLeastOneHttp && len(c.IssuingCertificateURL) != 0 {
return &lint.LintResult{Status: lint.Error, Details: "at least one accessMethod MUST have the URI scheme HTTP"}
return &lint.LintResult{Status: lint.Error, Details: "at least one id-ad-caIssuers accessMethod MUST have the URI scheme HTTP"}
}

return &lint.LintResult{Status: lint.Pass}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,26 @@ import (
"github.com/zmap/zlint/v3/test"
)

func TestSMIMELegacyAIAInternalName(t *testing.T) {
func TestSMIMELegacyAIAHasOneHTTP(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - cert with SAN",
InputFilename: "smime/aiaWithValidNamesLegacy.pem",
Name: "pass - aia with one ldap URI and one HTTP in each method",
InputFilename: "smime/legacyAiaOneHTTPOneLdap.pem",
ExpectedResult: lint.Pass,
},
{
Name: "error - cert without SAN",
InputFilename: "smime/aiaWithInternalNamesLegacy.pem",
ExpectedResult: lint.Warn,
Name: "error - aia with only ldap URIs HTTP in each method",
InputFilename: "smime/legacyAiaLdapOnly.pem",
ExpectedResult: lint.Error,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_smime_legacy_aia_contains_internal_names", tc.InputFilename)
result := test.TestLint("e_smime_legacy_aia_shall_have_one_http", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ package cabf_smime_br

import (
"net/url"
"time"

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

type smimeStrictAIAContainsInternalNames struct{}
type smimeStrictAIAHasHTTPOnly struct{}

/************************************************************************
BRs: 7.1.2.3c
Expand All @@ -40,25 +39,25 @@ For Strict and Multipurpose: When provided, every accessMethod SHALL have the UR
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: "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.CABEffectiveDate,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewSMIMEStrictAIAInternalName,
Lint: NewSMIMEStrictAIAHasHTTPOnly,
})
}

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

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

func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {
func (l *smimeStrictAIAHasHTTPOnly) Execute(c *x509.Certificate) *lint.LintResult {
for _, u := range c.OCSPServer {
purl, err := url.Parse(u)
if err != nil {
Expand All @@ -67,9 +66,6 @@ func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}
}
if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
}
for _, u := range c.IssuingCertificateURL {
purl, err := url.Parse(u)
Expand All @@ -79,9 +75,6 @@ func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint
if purl.Scheme != "http" {
return &lint.LintResult{Status: lint.Error}
}
if !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
}
return &lint.LintResult{Status: lint.Pass}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/zmap/zlint/v3/test"
)

func TestSMIMEStrictAIAInternalName(t *testing.T) {
func TestSMIMEStrictAIAHasHTTPOnly(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
Expand All @@ -21,12 +21,17 @@ func TestSMIMEStrictAIAInternalName(t *testing.T) {
{
Name: "warn - aia with internal names",
InputFilename: "smime/aiaWithInternalNamesStrict.pem",
ExpectedResult: lint.Warn,
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("w_smime_strict_aia_contains_internal_names", tc.InputFilename)
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)
}
Expand Down
Loading
Loading