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

Ensure AIA URLs point to public paths #760

Merged
3 changes: 3 additions & 0 deletions v3/integration/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,9 @@
"w_subject_surname_recommended_max_length": {},
"w_tls_server_cert_valid_time_longer_than_397_days": {
"WarnCount": 223
},
"w_sub_cert_aia_contains_internal_names": {
"WarnCount": 210
}
}
}
77 changes: 77 additions & 0 deletions v3/lints/cabf_br/lint_sub_cert_aia_contains_internal_names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package cabf_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"
"time"

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

type subCertAIAInternalName struct{}

/************************************************************************
BRs: 7.1.2.10.3
CA Certificate Authority Information Access
This extension MAY be present. If present, it MUST NOT be marked critical, and it MUST contain the
HTTP URL of the CA’s CRL service.

id-ad-ocsp A HTTP URL of the Issuing CA's OCSP responder.
id-ad-caIssuers A HTTP URL of the Issuing CA's Certificate.
*************************************************************************/

func init() {
lint.RegisterLint(&lint.Lint{
Name: "w_sub_cert_aia_contains_internal_names",
Description: "Subscriber certificates authorityInformationAccess extension should contain the HTTP URL of the issuing CA’s certificate, for public certificates this should not be an internal name",
Citation: "BRs: 7.1.2.10.3",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
Lint: NewSubCertAIAInternalName,
})
}

func NewSubCertAIAInternalName() lint.LintInterface {
return &subCertAIAInternalName{}
}

func (l *subCertAIAInternalName) CheckApplies(c *x509.Certificate) bool {
return util.IsSubscriberCert(c)
}

func (l *subCertAIAInternalName) 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 !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 !util.HasValidTLD(purl.Hostname(), time.Now()) {
return &lint.LintResult{Status: lint.Warn}
}
}
return &lint.LintResult{Status: lint.Pass}
}
35 changes: 35 additions & 0 deletions v3/lints/cabf_br/lint_sub_cert_aia_contains_internal_names_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package cabf_br

import (
"testing"

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

func TestAIAInternalName(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - aia with valid names",
InputFilename: "aiaWithValidNames.pem",
ExpectedResult: lint.Pass,
},
{
Name: "warn - aia with internal names",
InputFilename: "aiaWithInternalNames.pem",
ExpectedResult: lint.Warn,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_sub_cert_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)
}
})
}
}
95 changes: 95 additions & 0 deletions v3/lints/cabf_smime_br/lint_legacy_aia_contains_internal_names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
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"
"time"

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

type smimeLegacyAIAContainsInternalNames 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 Legacy: When provided, at least one accessMethod SHALL have the URI scheme HTTP. Other schemes (LDAP, FTP, ...) MAY be present.
*************************************************************************/

func init() {
lint.RegisterLint(&lint.Lint{
Name: "w_smime_legacy_aia_contains_internal_names",
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,
Lint: NewSMIMELegacyAIAInternalName,
})
}

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

func (l *smimeLegacyAIAContainsInternalNames) CheckApplies(c *x509.Certificate) bool {
return util.IsLegacySMIMECertificate(c)
}

func (l *smimeLegacyAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {

Choose a reason for hiding this comment

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

...at least one accessMethod SHALL have the URI scheme HTTP

I reckon that we can also enforce this clause a bit more closely by checking up on the parse scheme

atLeastOneHttp := false
...
...
if purl.Scheme == "http" {
   atLeastOneHttp = true
}
...
...
if !atLeastOneHttp {
    return &lint.LintResult{Status: lint.Error}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented my take on this one, but I became unsure about it so if you can take a look that would be helpful. Gist is, in the requirement is mentions that at least one accessMethod MUST be http, however I believe it applies independently to the OCSPServers and IssuingCAs. If you think I'm being overly aggressive here, let me know.

Choose a reason for hiding this comment

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

hrmmm I think that my only thought is that this would also require that there be a minimum of one at least OCSP and at least one CRL (that is, if the loop never enters then atLeastOnHttp will be false).

Regardless, if there is such a requirement (at least one OCSP and at least one CRL) then it should be a separate lint (trying to encode all surrounding facts into each lint is how they commonly end up in deadlock).

So I guess we could just check the empty condition before failing out.

	if !atLeastOneHttp && len(c.OCSPServer) != 0 {
		return &lint.LintResult{Status: lint.Error, Details: "at least one accessMethod MUST have the URI scheme HTTP"}
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I agree and made that change. Do you assume that the same change is not needed for the IssuingCertificateURL list?

Choose a reason for hiding this comment

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

If you extend this small change to CRLs as well then I think we'd be gtg on these!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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"}
}

atLeastOneHttp = false
for _, u := range c.IssuingCertificateURL {
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.IssuingCertificateURL) != 0 {
return &lint.LintResult{Status: lint.Error, Details: "at least one accessMethod MUST have the URI scheme HTTP"}
}

return &lint.LintResult{Status: lint.Pass}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package cabf_smime_br

import (
"testing"

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

func TestSMIMELegacyAIAInternalName(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - cert with SAN",
InputFilename: "smime/aiaWithValidNamesLegacy.pem",
ExpectedResult: lint.Pass,
},
{
Name: "error - cert without SAN",
InputFilename: "smime/aiaWithInternalNamesLegacy.pem",
ExpectedResult: lint.Warn,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_smime_legacy_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)
}
})
}
}
85 changes: 85 additions & 0 deletions v3/lints/cabf_smime_br/lint_strict_aia_contains_internal_names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
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"
"time"

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

type smimeStrictAIAContainsInternalNames 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.RegisterLint(&lint.Lint{
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.",
Citation: "BRs: 7.1.2.3c",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABEffectiveDate,
Lint: NewSMIMEStrictAIAInternalName,
})
}

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

func (l *smimeStrictAIAContainsInternalNames) CheckApplies(c *x509.Certificate) bool {
return util.IsStrictSMIMECertificate(c) || util.IsMultipurposeSMIMECertificate(c)
}

func (l *smimeStrictAIAContainsInternalNames) Execute(c *x509.Certificate) *lint.LintResult {
cardonator marked this conversation as resolved.
Show resolved Hide resolved
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 !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 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
@@ -0,0 +1,35 @@
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",
InputFilename: "smime/aiaWithInternalNamesStrict.pem",
ExpectedResult: lint.Warn,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_smime_strict_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)
}
})
}
}
Loading
Loading