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 lint for S/MIME BR 7.1.2.3 (k) #799

Merged
merged 3 commits into from
Mar 3, 2024

Conversation

bitlux
Copy link
Contributor

@bitlux bitlux commented Feb 21, 2024

Add a lint for S/MIME BR 7.1.2.3 (k).

I was unfortunately unable to create any certificates for testing using either genTestCerts or openssl. While the zcrypto x509.Certificate struct has an QCStatements field, it is ignored when creating a certificate. As far as I can tell, OpenSSL doesn't support writing the qcStatements extension. As such, I can understand if this PR isn't accepted.

Also, as far as I can tell, none of the S/MIME certificates in the integration testing repo have the qcStatements extension set (I set my lint to fail in all cases, and there were no failures, so I assume CheckApplies didn't return true for any certs).

@toddgaunt-gs
Copy link
Contributor

I've run into a similar issue before with genTestCerts, in which case I think I ended up manually marshaling some asn1 structures if you want to give that a shot. If I have some extra time later to play around, I may try to get a working example for you to do such a thing (no promises though :)

@toddgaunt-gs
Copy link
Contributor

I ended up just getting right to it actually!

It looks like including it inside of ExtraExtensions does the trick:

zlint/v3/cmd/genTestCerts/genTestCerts.go:105

		ExtraExtensions: []pkix.Extension{
			{
				Id:       asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 3},
				Critical: false,
				Value:    []byte{0x30, 0x00},
			},
		},

You'll just need to manually marshal a value there if you want, though I don't think it is necessary to test this lint since it doesn't care about the value.

@christopher-henderson
Copy link
Member

Thank you for the tip @toddgaunt-gs!

@bitlux I went ahead and generated some sample certs test updates. The following script should apply them.

cat > v3/testdata/smime/e_smime_qc_statements_must_not_be_critical_pass.pem <<EOF
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: 
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:11:d1:b5:e6:9a:aa:67:7c:06:39:35:96:c1:1a:
                    8e:8e:15:7b:af:a2:ab:72:e0:0c:8e:b4:56:1e:3c:
                    61:16:1d:a0:19:b9:66:03:77:0b:0d:10:18:19:6e:
                    43:57:3d:8d:2b:5a:2f:73:64:21:40:3c:b8:b5:bc:
                    7f:3e:1a:d2:98
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Certificate Policies: 
                Policy: 2.23.140.1.5.4.3
            qcStatements: 
                0.
    Signature Algorithm: ecdsa-with-SHA256
    Signature Value:
        30:44:02:20:04:cf:50:3e:11:20:5c:01:14:70:5d:a3:f9:16:
        2a:1a:49:ad:c0:8c:2f:27:97:17:c2:c2:d3:db:88:2a:ac:1e:
        02:20:02:d7:1c:fc:e2:6f:93:c5:39:92:e1:3a:75:86:06:d8:
        46:a8:af:6b:44:77:98:a4:ad:a8:29:5b:45:c9:11:0d
-----BEGIN CERTIFICATE-----
MIIBFzCBv6ADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMAAwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQR0bXm
mqpnfAY5NZbBGo6OFXuvoqty4AyOtFYePGEWHaAZuWYDdwsNEBgZbkNXPY0rWi9z
ZCFAPLi1vH8+GtKYoygwJjAUBgNVHSAEDTALMAkGB2eBDAEFBAMwDgYIKwYBBQUH
AQMEAjAAMAoGCCqGSM49BAMCA0cAMEQCIATPUD4RIFwBFHBdo/kWKhpJrcCMLyeX
F8LC09uIKqweAiAC1xz84m+TxTmS4Tp1hgbYRqiva0R3mKStqClbRckRDQ==
-----END CERTIFICATE-----
EOF

cat > v3/testdata/smime/e_smime_qc_statements_must_not_be_critical_fail.pem <<EOF
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: 
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:51:a4:d5:79:b9:32:be:a9:71:c1:d3:6c:9a:19:
                    94:d0:70:1f:64:bc:61:4e:a6:fc:5e:9f:ba:fb:4d:
                    b6:8a:a4:a0:2f:e6:13:16:f1:39:65:9c:02:ae:36:
                    22:a4:b6:59:49:02:ad:ec:3f:18:9c:93:32:42:6c:
                    9d:f8:9d:3a:bf
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Certificate Policies: 
                Policy: 2.23.140.1.5.4.3
            qcStatements: critical
                0.
    Signature Algorithm: ecdsa-with-SHA256
    Signature Value:
        30:44:02:20:17:97:15:1f:4f:72:1a:48:91:9d:b8:14:b7:61:
        96:cc:12:e1:5d:7a:47:4a:b2:98:31:81:70:fe:f4:1a:c9:a2:
        02:20:3d:1d:9c:00:cc:76:94:22:bb:37:cc:5c:5e:91:53:f9:
        1d:7b:0b:e9:01:54:38:16:85:3f:c0:10:3e:5d:df:bc
-----BEGIN CERTIFICATE-----
MIIBGjCBwqADAgECAgEDMAoGCCqGSM49BAMCMAAwIBcNMjMwOTAxMDAwMDAwWhgP
OTk5ODExMzAwMDAwMDBaMAAwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARRpNV5
uTK+qXHB02yaGZTQcB9kvGFOpvxen7r7TbaKpKAv5hMW8TllnAKuNiKktllJAq3s
PxickzJCbJ34nTq/oyswKTAUBgNVHSAEDTALMAkGB2eBDAEFBAMwEQYIKwYBBQUH
AQMBAf8EAjAAMAoGCCqGSM49BAMCA0cAMEQCIBeXFR9PchpIkZ24FLdhlswS4V16
R0qymDGBcP70GsmiAiA9HZwAzHaUIrs3zFxekVP5HXsL6QFUOBaFP8AQPl3fvA==
-----END CERTIFICATE-----
EOF

cat > v3/lints/cabf_smime_br/lint_qc_statements_not_critical_test.go <<EOF
package cabf_smime_br

import (
	"testing"

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

func TestSMIMEQCStatementsNotCritical(t *testing.T) {
	testCases := []struct {
		Name           string
		InputFilename  string
		ExpectedResult lint.LintStatus
	}{
		{
			Name:           "N/A - no qcStatements extension",
			InputFilename:  "smime/legacyAiaOneHTTPOneLdap.pem",
			ExpectedResult: lint.NA,
		},
		{
			Name:           "Pass - qcStatements not critical",
			InputFilename:  "smime/e_smime_qc_statements_must_not_be_critical_pass.pem",
			ExpectedResult: lint.Pass,
		},
		{
			Name:           "Fail - qcStatements critical",
			InputFilename:  "smime/e_smime_qc_statements_must_not_be_critical_fail.pem",
			ExpectedResult: lint.Error,
		},
	}
	for _, tc := range testCases {
		t.Run(tc.Name, func(t *testing.T) {
			result := test.TestLint("e_smime_qc_statements_must_not_be_critical", tc.InputFilename)
			if result.Status != tc.ExpectedResult {
				t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
			}
		})
	}
}
EOF

@bitlux
Copy link
Contributor Author

bitlux commented Feb 26, 2024

Thanks @toddgaunt-gs and @christopher-henderson! I've added the test cases.

@christopher-henderson christopher-henderson merged commit 83b5f8d into zmap:master Mar 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants