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 lints checking for underscores in labels pre-and-post CABF 1.6.2 #656

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions v3/lints/cabf_br/lint_fdqn_must_not_contain_underscores.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* ZLint Copyright 2021 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 cabf_br

import (
"fmt"
"strings"

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

func init() {
lint.RegisterLint(&lint.Lint{
Name: "e_fqdn_must_not_contain_underscores",
Description: "Prior to April 1, 2019, certificates containing underscore characters (“_”) in domain labels in dNSName entries MAY be issued as follows: " +
"• dNSName entries MAY include underscore characters such that replacing all underscore characters with hyphen characters (“-“) would result in a valid domain label, and; " +
"• Underscore characters MUST NOT be placed in the left most domain label, and; " +
"* Such certificates MUST NOT be valid for longer than 30 days.",
Citation: "BR 7.1.4.2.1",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.CABFBRs_1_6_2_Date,
Lint: NewDNSNameMustNotIncludeUnderscore,
})
}

type DNSNameMustdNotIncludeUnderscore struct{}

func NewDNSNameMustNotIncludeUnderscore() lint.LintInterface {
return &DNSNameMustdNotIncludeUnderscore{}
}

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

func (l *DNSNameMustdNotIncludeUnderscore) Execute(c *x509.Certificate) *lint.LintResult {
for _, dns := range c.DNSNames {
fqdnPortion := util.RemovePrependedWildcard(dns)
labels := strings.Split(fqdnPortion, ".")
for _, label := range labels {
if strings.Contains(label, "_") {
return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("dNSName ('%s') MUST NOT contain an underscore character ('_')", dns)}
}
}
Comment on lines +52 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to loop over labels here, or even remove the wildcard?

Since your error message reflects the full FQDN (dns), couldn't this just be

if strings.Contains(dns, "_") {
  return ...
}

}
return &lint.LintResult{Status: lint.Pass}
}
54 changes: 54 additions & 0 deletions v3/lints/cabf_br/lint_fdqn_must_not_contain_underscores_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* ZLint Copyright 2021 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 cabf_br

import (
"testing"

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

func TestFQDNMustNotContainUnderscores(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "No underscores",
InputFilename: "dNSNameNoUnderscores.pem",
ExpectedResult: lint.Pass,
},
{
Name: "One underscore in label",
InputFilename: "dNSNameWithUnderscore.pem",
ExpectedResult: lint.Error,
},
{
Name: "Not effective",
InputFilename: "dNSNameUnderscoreAllowedBefore2019.pem",
ExpectedResult: lint.NE,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_fqdn_must_not_contain_underscores", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v", tc.ExpectedResult, result.Status)
}
})
}
}
93 changes: 93 additions & 0 deletions v3/lints/cabf_br/lint_fqdn_may_contain_underscores.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* ZLint Copyright 2021 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 cabf_br

import (
"fmt"
"strings"
"time"

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

func init() {
lint.RegisterLint(&lint.Lint{
Name: "w_fqdn_may_contain_underscores",
Description: "Prior to April 1, 2019, certificates containing underscore characters (“_”) in domain labels in dNSName entries MAY be issued as follows: " +
"• dNSName entries MAY include underscore characters such that replacing all underscore characters with hyphen characters (“-“) would result in a valid domain label, and; " +
"• Underscore characters MUST NOT be placed in the left most domain label, and; " +
"* Such certificates MUST NOT be valid for longer than 30 days.",
Citation: "BR 7.1.4.2.1",
Source: lint.CABFBaselineRequirements,
EffectiveDate: util.ZeroDate,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is correct?

That's what the discussion on the other issue was trying to capture:

  • From Zero Date to present, the IETF specifications forbid underscores
  • From Zero Date to 2018-12-10 (aka SC12), the IETF/RFC 5280 rules applied
  • From 2018-12-10 until 2019-04-01 T00:00:00Z, underscores followed BRs only where the LDH rule allowed them. Specifically:
    • no leading underscores, because of the [A-Za-z] rule in 1034 and the [A-Za-Z0-9] modifications in 1123
    • no underscores in the third-and-fourth character because with hyphens they would be reserved
    • no trailing underscore because that'd be a trailing hyphen, which would violate the <let-dig> termination of <label> from 1034

So basically, the IETF rules always apply, but if we're applying BR rules, then we've got three separate lint periods.

IneffectiveDate: util.CABFBRs_1_6_2_Date,
Lint: NewDNSNameMayIncludeUnderscore,
})
}

type DNSNameShouldNotIncludeUnderscore struct{}

func NewDNSNameMayIncludeUnderscore() lint.LintInterface {
return &DNSNameShouldNotIncludeUnderscore{}
}

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

func (l *DNSNameShouldNotIncludeUnderscore) Execute(c *x509.Certificate) *lint.LintResult {
validLongerThanThirtyDays := c.NotAfter.Sub(c.NotBefore) > time.Hour*24*30
for _, dns := range c.DNSNames {
fqdnPortion := util.RemovePrependedWildcard(dns)
labels := strings.Split(fqdnPortion, ".")
if len(labels) > 0 {
leftMostLabel := labels[0]
if strings.Contains(leftMostLabel, "_") {
return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("The left most label of '%s' MUST NOT contain an underscore character ('_')", dns)}
}
}
for _, label := range labels {
containsUnderscore := strings.Contains(label, "_")
wouldBeValidIfHyphenInstead := util.IsLDHLabel(strings.ReplaceAll(label, "_", "-"))
if containsUnderscore && wouldBeValidIfHyphenInstead && !validLongerThanThirtyDays {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this linting separate things? I thought we encouraged these as separate lints.

Namely, you're testing both the 30 day rule and the LDH-valid rule, but I think we'd have these separate?

Broken out, I think this is four BR lints plus one IETF lint:

  • BR lint
    • No underscore: Effective date is Zero date, Ineffective date is 2018-12-10 (which is the real 1.6.2 date)
    • 30-days-or-less when underscore present: Effective date is 2018-12-10, Ineffective date is 2019-04-01 (underscore sunset date)
    • Valid LDH-when-replaced: Effective date is 2018-12-10. Ineffective date is 2019-04-01.
    • No underscore. Effective date is 2019-04-01. No ineffective date
  • IETF lint
    • No underscore. Effective date is Zero date.

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, especially since I appear to have gotten some core aspects incorrect attempting to bundle these together.

I have fired up the following in the same order as you proposed:

#659 (No underscores are allowed in DNSNames before BR 1.6.2's permissibility period)
#660 (Permit underscores in DNSNames if-and-only-if those certificates are valid for less than 30 days and during BR 1.6.2's permissibility period)
#661 (Permit underscores in DNSNames if-and-only-if replacing all underscores results in valid LDH labels during BR 1.6.2's permissibility period)
#662 (No underscores are allowed in DNSNames after BR 1.6.2's permissibility period)
#663 (RFC lint to disallow all underscores in DNSNames)

I'll keep this PR open for the moment for tracking sake, but I plan to close it as it is superseded by the above three.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good deal. I realize some of the feedback propagates through these changes, thanks for dealing with it :)

// Fulfills all clauses, so simply warn that this is deprecated.
return &lint.LintResult{Status: lint.Warn, Details: fmt.Sprintf("%s contains an underscore "+
"character ('_'). This character MUST NOT appear within the FQDN after April 1, 2019. For more "+
"information, please see Ballot SC12: Sunset of Underscores in dNSNames "+
"(https://cabforum.org/2018/11/12/ballot-sc-12-sunset-of-underscores-in-dnsnames/)", dns)}
} else if containsUnderscore && !wouldBeValidIfHyphenInstead {
// Fails the first clause about replacing the _ with - which must result in a valid LDH.
return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("%s contains an underscore "+
"character ('_') that, when replaces by a hyphen, does not result in a valid LDH label. This "+
"character MUST NOT appear within the FQDN after April 1, 2019. For more information, please see "+
"Ballot SC12: Sunset of Underscores in dNSNames "+
"(https://cabforum.org/2018/11/12/ballot-sc-12-sunset-of-underscores-in-dnsnames/)", dns)}
} else if containsUnderscore && wouldBeValidIfHyphenInstead && validLongerThanThirtyDays {
// Fails the third clause regarding validity dates being longer than 30 days.
return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("%s contains an underscore "+
"character ('_') and the certificate is valid for more than 30 days. This character MUST NOT appear "+
"within the FQDN after April 1, 2019. For more information, please see Ballot SC12: Sunset of "+
"Underscores in dNSNames "+
"(https://cabforum.org/2018/11/12/ballot-sc-12-sunset-of-underscores-in-dnsnames/)", dns)}
} else {
// Label passes lint.
continue
}
}
}
return &lint.LintResult{Status: lint.Pass}
}
69 changes: 69 additions & 0 deletions v3/lints/cabf_br/lint_fqdn_may_contain_underscores_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* ZLint Copyright 2021 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 cabf_br

import (
"testing"

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

func TestFQDNMayContainUnderscores(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "No underscores",
InputFilename: "dNSNameNoUnderscoresButAllowed.pem",
ExpectedResult: lint.Pass,
},
{
Name: "Underscores but allowable",
InputFilename: "dNSNameUnderscoreAllowedBefore2019.pem",
ExpectedResult: lint.Warn,
},
{
Name: "Underscore in left most label",
InputFilename: "dNSNameInLeftMostLabel.pem",
ExpectedResult: lint.Error,
},
{
Name: "Underscore but validity is too long",
InputFilename: "dNSNameWithUnderscoreValidityTooLong.pem",
ExpectedResult: lint.Error,
},
{
Name: "Not a valid label is _ replaced with -",
InputFilename: "dNSNameUnderscoreInvalidEvenIfReplaced.pem",
ExpectedResult: lint.Error,
},
{
Name: "Not effective",
InputFilename: "dNSNameWithUnderscore.pem",
ExpectedResult: lint.NE,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_fqdn_may_contain_underscores", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v", tc.ExpectedResult, result.Status)
}
})
}
}
37 changes: 37 additions & 0 deletions v3/testdata/dNSNameInLeftMostLabel.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Jan 1 00:00:00 2018 GMT
Not After : Jan 2 00:00:00 2018 GMT
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:08:c3:ca:32:66:23:2f:4b:a7:16:41:29:32:11:
f3:ac:19:fa:ed:31:b1:2c:a2:6a:0e:11:de:db:05:
34:cb:45:c7:33:9d:7e:e6:9b:35:9a:3b:32:50:79:
59:e3:8c:b0:51:f9:d9:e6:dc:38:8f:76:3c:3d:e1:
17:a7:4d:d1:3e
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Subject Alternative Name:
DNS:totally_.not_fine.with.me
Signature Algorithm: ecdsa-with-SHA256
30:45:02:21:00:aa:d0:d9:7b:7a:13:4b:a4:0d:64:9c:7a:b8:
c9:ef:9e:22:d8:a0:ae:c2:4a:f4:0a:6b:88:b1:00:b1:1e:e8:
7c:02:20:5d:83:23:e9:1e:39:1a:5b:7b:3f:fa:9d:3b:3d:bf:
ec:0d:20:11:ae:e9:27:74:66:e8:74:71:3d:ed:7e:98:10
-----BEGIN CERTIFICATE-----
MIIBFjCBvaADAgECAgEDMAoGCCqGSM49BAMCMAAwHhcNMTgwMTAxMDAwMDAwWhcN
MTgwMTAyMDAwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAECMPKMmYj
L0unFkEpMhHzrBn67TGxLKJqDhHe2wU0y0XHM51+5ps1mjsyUHlZ44ywUfnZ5tw4
j3Y8PeEXp03RPqMoMCYwJAYDVR0RBB0wG4IZdG90YWxseV8ubm90X2ZpbmUud2l0
aC5tZTAKBggqhkjOPQQDAgNIADBFAiEAqtDZe3oTS6QNZJx6uMnvniLYoK7CSvQK
a4ixALEe6HwCIF2DI+keORpbez/6nTs9v+wNIBGu6Sd0Zuh0cT3tfpgQ
-----END CERTIFICATE-----
37 changes: 37 additions & 0 deletions v3/testdata/dNSNameNoUnderscores.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Nov 30 00:00:00 9997 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:55:8f:1d:1a:73:7e:54:de:33:03:3a:77:b5:e9:
ad:17:e1:03:17:ea:d5:ed:86:2b:35:ba:ed:e3:ed:
64:77:47:a8:4a:5d:d4:68:43:42:9e:18:00:6e:e0:
a5:dd:ec:c4:d6:d3:e4:29:d0:d5:54:b4:11:e7:db:
d4:77:a1:23:a0
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Subject Alternative Name:
DNS:totally.fine.with.me
Signature Algorithm: ecdsa-with-SHA256
30:45:02:21:00:e8:06:1f:e1:27:63:30:5c:30:46:35:d4:f4:
1c:72:29:d7:3e:0d:9c:f2:12:ea:45:60:f5:2b:87:d6:77:93:
31:02:20:3f:6f:08:8c:8f:b2:e4:e8:9a:54:2f:dc:e1:4a:63:
9f:a6:6e:db:06:c8:f8:fc:9e:92:d4:ae:5f:71:d2:07:b6
-----BEGIN CERTIFICATE-----
MIIBFTCBvKADAgECAgEDMAoGCCqGSM49BAMCMAAwIhgPOTk5NzExMzAwMDAwMDBa
GA85OTk4MTEzMDAwMDAwMFowADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABFWP
HRpzflTeMwM6d7XprRfhAxfq1e2GKzW67ePtZHdHqEpd1GhDQp4YAG7gpd3sxNbT
5CnQ1VS0Eefb1HehI6CjIzAhMB8GA1UdEQQYMBaCFHRvdGFsbHkuZmluZS53aXRo
Lm1lMAoGCCqGSM49BAMCA0gAMEUCIQDoBh/hJ2MwXDBGNdT0HHIp1z4NnPIS6kVg
9SuH1neTMQIgP28IjI+y5OiaVC/c4Upjn6Zu2wbI+PyektSuX3HSB7Y=
-----END CERTIFICATE-----
37 changes: 37 additions & 0 deletions v3/testdata/dNSNameNoUnderscoresButAllowed.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Jan 1 00:00:00 2018 GMT
Not After : Jan 2 00:00:00 2018 GMT
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:bc:0a:29:a0:6d:3d:e7:dc:e9:06:a0:f4:99:b1:
f8:05:f5:62:c6:ac:ca:8b:0a:28:82:5a:25:d6:aa:
04:a4:07:5a:ac:c4:e6:07:d0:4c:93:ef:30:57:34:
c8:4d:19:c9:0b:6b:c0:b4:3c:d7:7e:cd:f4:d6:7e:
71:a3:d6:8a:94
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Subject Alternative Name:
DNS:totally.fine.with.me
Signature Algorithm: ecdsa-with-SHA256
30:44:02:20:4c:18:8a:0b:97:84:00:d8:f6:0a:ce:ae:60:c3:
a0:f9:9e:82:a8:31:eb:63:a6:49:81:41:49:cf:e8:5e:97:76:
02:20:40:0e:89:66:66:2d:d7:6c:fe:77:38:21:1a:45:83:d2:
df:a8:c1:2c:fa:7c:51:0e:0d:dc:f8:d3:3e:c2:04:84
-----BEGIN CERTIFICATE-----
MIIBEDCBuKADAgECAgEDMAoGCCqGSM49BAMCMAAwHhcNMTgwMTAxMDAwMDAwWhcN
MTgwMTAyMDAwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEvAopoG09
59zpBqD0mbH4BfVixqzKiwooglol1qoEpAdarMTmB9BMk+8wVzTITRnJC2vAtDzX
fs301n5xo9aKlKMjMCEwHwYDVR0RBBgwFoIUdG90YWxseS5maW5lLndpdGgubWUw
CgYIKoZIzj0EAwIDRwAwRAIgTBiKC5eEANj2Cs6uYMOg+Z6CqDHrY6ZJgUFJz+he
l3YCIEAOiWZmLdds/nc4IRpFg9LfqMEs+nxRDg3c+NM+wgSE
-----END CERTIFICATE-----
Loading