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

CABF SMIME BR Appendix A.1 - countryName matches registration scheme id #768

Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* 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.
*/

package cabf_smime_br

import (
"fmt"
"regexp"

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

// Regex to match the start of an organization identifier: 3 character registration scheme identifier and 2 character ISO 3166 country code
var countryRegex = regexp.MustCompile(`^([A-Z]{3})([A-Z]{2})`)

func init() {
lint.RegisterCertificateLint(&lint.CertificateLint{
LintMetadata: lint.LintMetadata{
Name: "e_registration_scheme_id_matches_subject_country",
Description: "The country code used in the Registration Scheme identifier SHALL match that of the subject:countryName in the Certificate as specified in Section 7.1.4.2.2",
Citation: "Appendix A A.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Citation: "Appendix A A.1",
Citation: "Appendix A.1",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
},
Lint: NewRegistrationSchemeIDMatchesSubjectCountry,
})
}

type registrationSchemeIDMatchesSubjectCountry struct{}

// NewRegistrationSchemeIDMatchesSubjectCountry creates a new linter to enforce SHALL requirements for registration scheme identifiers matching subject:countryName
func NewRegistrationSchemeIDMatchesSubjectCountry() lint.LintInterface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func NewRegistrationSchemeIDMatchesSubjectCountry() lint.LintInterface {
func NewRegistrationSchemeIDMatchesSubjectCountry() lint.CertificateLintInterface {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have seen LintInterface was deprecated. Updated as suggested.

return &registrationSchemeIDMatchesSubjectCountry{}
}

// CheckApplies returns true if the provided certificate contains subject:countryName and one-or-more of the following SMIME BR policy identifiers:
Copy link
Contributor

Choose a reason for hiding this comment

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

Firstly, this comment doesn't mention the update to need a partially valid subject.organizationID.

Secondly, I wonder if you could compress the list of policies by just saying "and the certificate contains an Organization or Sponsor Validated policy OID"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made comment more relevant to full content of CheckApplies

// - Organization Validated Strict
// - Organization Validated Multipurpose
// - Organization Validated Legacy
// - Sponsor Validated Strict
// - Sponsor Validated Multipurpose
// - Sponsor Validated Legacy
func (l *registrationSchemeIDMatchesSubjectCountry) CheckApplies(c *x509.Certificate) bool {
if c.Subject.Country == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit picky but I wonder if this would be better if you also checked that there was a non-empty-string value (or make sure that there's at least one value that's two characters long?) in c.Subject.Country?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added subsequent check that c.Subject.Country[0] is 2 characters in length because as you raised in a different comment, the Execute function of this lint is assuming that the first countryName value is present and (at least could be) valid - e.g. ISO 3166 or XX

return false
}

for _, id := range c.Subject.OrganizationIDs {
submatches := countryRegex.FindStringSubmatch(id)
if len(submatches) < 3 {

Choose a reason for hiding this comment

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

Thankfully len(nil) won't explode here 😄 (I had to double check this myself, it returns 0 which makes sense).

return false
}
}

return util.IsOrganizationValidatedCertificate(c) || util.IsSponsorValidatedCertificate(c)
}

// Execute applies the requirements on matching subject:countryName with registration scheme identifiers
func (l *registrationSchemeIDMatchesSubjectCountry) Execute(c *x509.Certificate) *lint.LintResult {
country := c.Subject.Country[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how well this combines with the CheckApplies. I have got a comment in this review suggesting you loop to find one countryName of length 2 but I wonder if you should be saying this lint applies if something like this is true:
c.Subject.Country[0] == <TWO CAPITAL LETTERS>
Given that the Execute function of this lint is assuming that the first countryName value is present and valid.


for _, id := range c.Subject.OrganizationIDs {
if err := verifySMIMEOrganizationIdentifierContainsSubjectNameCountry(id, country); err != nil {
return &lint.LintResult{Status: lint.Error, Details: err.Error()}
}
}
return &lint.LintResult{Status: lint.Pass}
}

// verifySMIMEOrganizationIdentifierContainSubjectNameCountry verifies that the country code used in the subject:organizationIdentifier matches subject:countryName
func verifySMIMEOrganizationIdentifierContainsSubjectNameCountry(id string, country string) error {
submatches := countryRegex.FindStringSubmatch(id)
// Captures the country code from the organization identifier

Choose a reason for hiding this comment

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

Suggested change
// Captures the country code from the organization identifier
// Captures the country code from the organization identifier
// Note that this raw indexing into the second position is only safe
// due to a length check done in CheckApplies

I often find it useful for the poor unfortunate souls in the future to leave a note as to why an unchecked index/dereference is safe, just in case those safety guarantees are broken by someone in the future and now the cause has to be hunted down.

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 have added this suggested comment.

identifierCountry := submatches[2]

if identifierCountry != country {
return fmt.Errorf("the country code used in the Registration Scheme identifier SHALL match that of the subject:countryName")
}

return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package cabf_smime_br

import (
"testing"

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

func TestRegistrationSchemeIDMatchesSubjectNameCountry(t *testing.T) {

testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
ExpectedDetails string
}{
{
Name: "pass - organization validated certificate with subject:Name:Country matching subject:organizationIdentifier",
InputFilename: "smime/organization_validated_with_matching_country.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - sponsor validated certificate with subject:Name:Country matching subject:organizationIdentifier",
InputFilename: "smime/sponsor_validated_with_matching_country.pem",
ExpectedResult: lint.Pass,
},
{
Name: "error - individual validated certificate",
InputFilename: "smime/individual_validated_with_matching_country.pem",
ExpectedResult: lint.NA,
},
{
Name: "error - no country specified in certificate",
InputFilename: "smime/organization_validatged_with_no_country_specified.pem",
ExpectedResult: lint.NA,
},
{
Name: "error - organization validated certificate with subject:organizationIdentifier in incorrect format",
InputFilename: "smime/organization_validated_with_incorrect_format_identifier.pem",
ExpectedResult: lint.NA,
},
{
Name: "error - organization validated certificate with subject:Name:Country not matching subject:organizationIdentifier",
InputFilename: "smime/organization_validated_with_non_matching_country.pem",
ExpectedResult: lint.Error,
ExpectedDetails: "the country code used in the Registration Scheme identifier SHALL match that of the subject:countryName",
},
}

for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_registration_scheme_id_matches_subject_country", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v", tc.ExpectedResult, result.Status)
}

if tc.ExpectedDetails != result.Details {
t.Errorf("expected details: %q, was %q", tc.ExpectedDetails, result.Details)
}
})
}

}
39 changes: 39 additions & 0 deletions v3/testdata/smime/individual_validated_with_matching_country.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject: C = GB, organizationIdentifier = NTRGB-12345678
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:f5:60:29:a4:49:91:2e:f8:98:b6:f2:77:ca:a1:
e8:ef:3f:61:38:dc:14:52:fe:fb:76:40:f3:48:11:
4f:62:5d:39:7b:1c:e4:64:e3:bc:8c:ef:67:c8:cd:
43:54:2e:6c:7d:78:94:20:80:a0:79:26:47:0b:93:
4e:c1:6b:06:35
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Certificate Policies:
Policy: 2.23.140.1.5.4.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:46:02:21:00:ac:c2:22:ab:e6:e8:2e:43:3e:44:49:3f:46:
26:d3:a5:2d:7f:dd:9e:7c:09:54:5d:10:12:b7:e8:42:36:05:
27:02:21:00:fc:37:86:54:2f:48:ac:84:4d:a5:32:ad:6b:f0:
a4:1d:39:c5:dc:eb:43:a7:f8:c1:4e:c5:68:f8:d0:0e:61:fe
-----BEGIN CERTIFICATE-----
MIIBLzCB1aADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMCYxCzAJBgNVBAYTAkdCMRcwFQYDVQRhEw5OVFJHQi0x
MjM0NTY3ODBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABPVgKaRJkS74mLbyd8qh
6O8/YTjcFFL++3ZA80gRT2JdOXsc5GTjvIzvZ8jNQ1QubH14lCCAoHkmRwuTTsFr
BjWjGDAWMBQGA1UdIAQNMAswCQYHZ4EMAQUEATAKBggqhkjOPQQDAgNJADBGAiEA
rMIiq+boLkM+REk/RibTpS1/3Z58CVRdEBK36EI2BScCIQD8N4ZUL0ishE2lMq1r
8KQdOcXc60On+MFOxWj40A5h/g==
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject: C = GB, organizationIdentifier = NGB-12345678
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:12:e7:c6:32:81:8f:3e:cb:5a:9b:45:25:53:a6:
04:0a:b9:87:2a:92:c6:1c:df:ee:bf:65:66:2a:84:
a0:1a:f3:42:7a:8f:13:35:e0:7c:51:73:8c:dc:b5:
51:62:7b:06:71:09:22:20:85:3f:28:02:70:5b:22:
53:9d:8a:69:10
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Certificate Policies:
Policy: 2.23.140.1.5.2.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:46:02:21:00:a4:b8:3c:57:91:01:1e:27:ed:77:e2:94:fb:
4f:75:bb:5a:74:72:28:32:7f:bc:62:08:1f:61:32:91:29:83:
4c:02:21:00:fe:ec:86:77:78:58:b5:fe:90:04:6a:9f:b8:ca:
01:d3:82:4e:7b:64:90:f5:c3:7d:16:3e:60:30:7b:ab:83:90
-----BEGIN CERTIFICATE-----
MIIBLTCB06ADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMCQxCzAJBgNVBAYTAkdCMRUwEwYDVQRhEwxOR0ItMTIz
NDU2NzgwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQS58YygY8+y1qbRSVTpgQK
uYcqksYc3+6/ZWYqhKAa80J6jxM14HxRc4zctVFiewZxCSIghT8oAnBbIlOdimkQ
oxgwFjAUBgNVHSAEDTALMAkGB2eBDAEFAgEwCgYIKoZIzj0EAwIDSQAwRgIhAKS4
PFeRAR4n7XfilPtPdbtadHIoMn+8YggfYTKRKYNMAiEA/uyGd3hYtf6QBGqfuMoB
04JOe2SQ9cN9Fj5gMHurg5A=
-----END CERTIFICATE-----
39 changes: 39 additions & 0 deletions v3/testdata/smime/organization_validated_with_matching_country.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject: C = GB, organizationIdentifier = NTRGB-12345678
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:37:a6:d9:19:6d:90:51:59:5c:db:c2:a8:27:b1:
8d:c4:01:3b:87:cb:8d:5e:e9:1b:53:d2:e7:aa:9c:
06:58:85:89:8b:e1:70:9c:23:12:d6:c1:7b:6b:22:
6e:27:6b:01:a2:88:b6:77:90:a2:6d:f1:bf:26:04:
f0:f2:3a:82:93
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Certificate Policies:
Policy: 2.23.140.1.5.2.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:46:02:21:00:9b:2b:51:2d:88:ab:f2:44:5f:9c:b6:14:81:
9c:c8:e3:21:e4:23:0d:9b:a0:71:77:b1:78:8c:da:a0:d3:a5:
eb:02:21:00:a3:6b:34:38:a4:9c:b2:a6:f3:4e:30:6a:41:2a:
aa:1f:9c:e3:84:98:4e:da:3a:bb:4e:06:46:2c:2c:ba:93:8b
-----BEGIN CERTIFICATE-----
MIIBLzCB1aADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMCYxCzAJBgNVBAYTAkdCMRcwFQYDVQRhEw5OVFJHQi0x
MjM0NTY3ODBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDem2RltkFFZXNvCqCex
jcQBO4fLjV7pG1PS56qcBliFiYvhcJwjEtbBe2sibidrAaKItneQom3xvyYE8PI6
gpOjGDAWMBQGA1UdIAQNMAswCQYHZ4EMAQUCATAKBggqhkjOPQQDAgNJADBGAiEA
mytRLYir8kRfnLYUgZzI4yHkIw2boHF3sXiM2qDTpesCIQCjazQ4pJyypvNOMGpB
KqofnOOEmE7aOrtOBkYsLLqTiw==
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject: C = GB, organizationIdentifier = NTRFR-12345678
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:d9:56:b4:43:cf:87:9f:20:71:a6:79:02:a2:57:
c5:0a:d0:52:23:ac:c1:aa:d3:7e:f8:b5:f5:5b:8c:
da:f1:17:24:5f:ee:55:a0:46:3d:2b:e0:a2:8a:0d:
4f:4b:cb:9b:df:3e:de:0a:96:4d:fe:8a:aa:dc:24:
38:3a:79:41:2b
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Certificate Policies:
Policy: 2.23.140.1.5.2.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:44:02:20:5f:3c:88:87:67:7c:f8:e7:d1:c4:b3:6f:7a:e1:
c4:f2:71:db:ae:3a:79:06:db:b0:2e:05:07:03:8d:b2:30:55:
02:20:24:3b:f2:be:35:0e:a9:70:21:33:1f:95:f9:44:42:62:
ea:35:77:63:fa:31:b4:04:e2:46:d9:55:a8:8d:24:cc
-----BEGIN CERTIFICATE-----
MIIBLTCB1aADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMCYxCzAJBgNVBAYTAkdCMRcwFQYDVQRhEw5OVFJGUi0x
MjM0NTY3ODBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABNlWtEPPh58gcaZ5AqJX
xQrQUiOswarTfvi19VuM2vEXJF/uVaBGPSvgoooNT0vLm98+3gqWTf6KqtwkODp5
QSujGDAWMBQGA1UdIAQNMAswCQYHZ4EMAQUCATAKBggqhkjOPQQDAgNHADBEAiBf
PIiHZ3z459HEs2964cTycduuOnkG27AuBQcDjbIwVQIgJDvyvjUOqXAhMx+V+URC
Yuo1d2P6MbQE4kbZVaiNJMw=
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Certificate:
Data:
Version: 3 (0x2)
Serial Number: 3 (0x3)
Signature Algorithm: ecdsa-with-SHA256
Issuer:
Validity
Not Before: Sep 1 00:00:00 2023 GMT
Not After : Nov 30 00:00:00 9998 GMT
Subject: organizationIdentifier = NTRGB-12345678
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:5a:3e:14:73:c8:68:32:d7:c0:d8:dc:c5:3e:bd:
ca:ca:a9:4e:48:9b:8a:ec:6a:5a:9a:5c:c0:8a:af:
e9:21:c4:d9:5f:b6:a7:d9:62:75:a5:5f:60:81:3a:
90:cf:98:98:ae:25:8f:39:b5:4c:f9:ef:32:54:a7:
a4:06:eb:ef:d8
ASN1 OID: prime256v1
NIST CURVE: P-256
X509v3 extensions:
X509v3 Certificate Policies:
Policy: 2.23.140.1.5.2.1
Signature Algorithm: ecdsa-with-SHA256
Signature Value:
30:45:02:20:59:47:29:1a:67:67:67:c5:6f:2e:af:50:f5:09:
41:56:1c:76:b1:e8:85:1e:5e:84:ea:61:b4:d8:a0:87:f2:5c:
02:21:00:bf:6f:ce:d8:0d:0c:cc:32:19:d9:cc:60:65:47:65:
59:dd:87:99:3b:96:1d:76:b6:66:c3:10:e6:33:c8:2d:e3
-----BEGIN CERTIFICATE-----
MIIBITCByKADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMBkxFzAVBgNVBGETDk5UUkdCLTEyMzQ1Njc4MFkwEwYH
KoZIzj0CAQYIKoZIzj0DAQcDQgAEWj4Uc8hoMtfA2NzFPr3KyqlOSJuK7GpamlzA
iq/pIcTZX7an2WJ1pV9ggTqQz5iYriWPObVM+e8yVKekBuvv2KMYMBYwFAYDVR0g
BA0wCzAJBgdngQwBBQIBMAoGCCqGSM49BAMCA0gAMEUCIFlHKRpnZ2fFby6vUPUJ
QVYcdrHohR5ehOphtNigh/JcAiEAv2/O2A0MzDIZ2cxgZUdlWd2HmTuWHXa2ZsMQ
5jPILeM=
-----END CERTIFICATE-----
Loading