pki: Check for unused bits when verifying MTC proofs X.509 made its signatureValue a BIT STRING, so every algorithm is stuck having to check this. (The spec says to do this in a somewhat roundabout way right now. It says to decode signatureValue as an MTCProof and then the encoding is written so that only a whole number of bytes is valid.) Change-Id: I289c71f63d6e9d2b1acd25a70f2dd3792b85cfe5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/93887 Reviewed-by: Matt Mueller <mattm@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/gen/sources.bzl b/gen/sources.bzl index 1093ef7..34d79e2 100644 --- a/gen/sources.bzl +++ b/gen/sources.bzl
@@ -2845,6 +2845,7 @@ "pki/testdata/verify_unittest/mtc-leaf-b.pem", "pki/testdata/verify_unittest/mtc-leaf-bitflip.pem", "pki/testdata/verify_unittest/mtc-leaf-c.pem", + "pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem", "pki/testdata/verify_unittest/mtc-leaf.pem", "pki/testdata/verify_unittest/self-issued.pem", ]
diff --git a/gen/sources.cmake b/gen/sources.cmake index 44e9081..bc2cc27 100644 --- a/gen/sources.cmake +++ b/gen/sources.cmake
@@ -2901,6 +2901,7 @@ pki/testdata/verify_unittest/mtc-leaf-b.pem pki/testdata/verify_unittest/mtc-leaf-bitflip.pem pki/testdata/verify_unittest/mtc-leaf-c.pem + pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem pki/testdata/verify_unittest/mtc-leaf.pem pki/testdata/verify_unittest/self-issued.pem )
diff --git a/gen/sources.gni b/gen/sources.gni index 0c242ff..2f5232a 100644 --- a/gen/sources.gni +++ b/gen/sources.gni
@@ -2845,6 +2845,7 @@ "pki/testdata/verify_unittest/mtc-leaf-b.pem", "pki/testdata/verify_unittest/mtc-leaf-bitflip.pem", "pki/testdata/verify_unittest/mtc-leaf-c.pem", + "pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem", "pki/testdata/verify_unittest/mtc-leaf.pem", "pki/testdata/verify_unittest/self-issued.pem", ]
diff --git a/gen/sources.json b/gen/sources.json index 7981559..c6f5244 100644 --- a/gen/sources.json +++ b/gen/sources.json
@@ -2826,6 +2826,7 @@ "pki/testdata/verify_unittest/mtc-leaf-b.pem", "pki/testdata/verify_unittest/mtc-leaf-bitflip.pem", "pki/testdata/verify_unittest/mtc-leaf-c.pem", + "pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem", "pki/testdata/verify_unittest/mtc-leaf.pem", "pki/testdata/verify_unittest/self-issued.pem" ]
diff --git a/gen/sources.mk b/gen/sources.mk index 947b599..ac81b8f 100644 --- a/gen/sources.mk +++ b/gen/sources.mk
@@ -2818,6 +2818,7 @@ pki/testdata/verify_unittest/mtc-leaf-b.pem \ pki/testdata/verify_unittest/mtc-leaf-bitflip.pem \ pki/testdata/verify_unittest/mtc-leaf-c.pem \ + pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem \ pki/testdata/verify_unittest/mtc-leaf.pem \ pki/testdata/verify_unittest/self-issued.pem
diff --git a/pki/testdata/path_builder_unittest/mtc/mtc-config.json b/pki/testdata/path_builder_unittest/mtc/mtc-config.json index 72358f5..a63b102 100644 --- a/pki/testdata/path_builder_unittest/mtc/mtc-config.json +++ b/pki/testdata/path_builder_unittest/mtc/mtc-config.json
@@ -1,4 +1,5 @@ { + "Version": "davidben-09", "LogID": "32473.1", "Entries": [ {
diff --git a/pki/testdata/verify_unittest/mtc-config.json b/pki/testdata/verify_unittest/mtc-config.json index 066f55a..2676874 100644 --- a/pki/testdata/verify_unittest/mtc-config.json +++ b/pki/testdata/verify_unittest/mtc-config.json
@@ -1,4 +1,5 @@ { + "Version": "davidben-09", "LogID": "32473.1", "Entries": [ { @@ -33,6 +34,11 @@ "SubtreeStart": 8, "SubtreeEnd": 13, "BitFlipProof": true + }, + { + "SubtreeStart": 8, + "SubtreeEnd": 13, + "UnusedBit": true } ] },
diff --git a/pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem b/pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem new file mode 100644 index 0000000..91612bb --- /dev/null +++ b/pki/testdata/verify_unittest/mtc-leaf-unused-bit.pem
@@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBjjCCAQWgAwIBAgIBCTAMBgorBgEEAYLaSy8AMBkxFzAVBgorBgEEAYLaSy8B +DAczMjQ3My4xMB4XDTIwMDEwMTAwMDAwMFoXDTMwMTIzMTIzNTk1OVowADBZMBMG +ByqGSM49AgEGCCqGSM49AwEHA0IABMsYgtepbbPkji/OzsB4Rg8yvf3clHe7DPNI +wH+d1S/gFd/B6ByqZgOyyaGC3+MlYXFZXjXMbn7AH3D7nky19FajVTBTMA4GA1Ud +DwEB/wQEAwIHgDAWBgNVHSUBAf8EDDAKBggrBgEFBQcDATApBgNVHREBAf8EHzAd +gglhLmV4YW1wbGWCECouYmxhaC5hLmV4YW1wbGUwDAYKKwYBBAGC2ksvAAN1AQAA +AAAAAAAIAAAAAAAAAA0AYGfmiKnfNXzM/834NuaRq4i0xGTzYV6nx8s8iUliZgki +gQcBu21ak6gq40RFYQpnPL0juf88bIJsdUymRTaKOe9FOUG5ul4iE7jyCARvxLz2 +CyJdSTMySmd2rSx5KBjgGwAA +-----END CERTIFICATE-----
diff --git a/pki/testdata/verify_unittest/readme-mtc-certs.md b/pki/testdata/verify_unittest/readme-mtc-certs.md index a53ad52..a8ebb6e 100644 --- a/pki/testdata/verify_unittest/readme-mtc-certs.md +++ b/pki/testdata/verify_unittest/readme-mtc-certs.md
@@ -13,10 +13,11 @@ ## Instructions - Run - `go run github.com/davidben/merkle-tree-certs/demo@92282dba2bf361c486dda5fe7606ef77abd2a1a0 -config=mtc-config.json` + `go run github.com/ietf-plants-wg/merkle-tree-certs/demo@b0c83104918f10e8c813783f77434143eab4ef97 -config=mtc-config.json` - copy/move the following output files: - `out/cert_9_0.pem` to `mtc-leaf.pem` - `out/cert_9_1.pem` to `mtc-leaf-bitflip.pem` + - `out/cert_9_2.pem` to `mtc-leaf-unused-bit.pem` - `out/cert_10_0.pem` to `mtc-leaf-b.pem` - `out/cert_19_0.pem` to `mtc-leaf-c.pem` - edit `VerifyMTCTest::SetUp` to set the trusted subtrees to the ones output by
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc index 6e64dea..82792d6 100644 --- a/pki/verify_certificate_chain.cc +++ b/pki/verify_certificate_chain.cc
@@ -1140,7 +1140,8 @@ CBS mtc_proof(cert.signature_value().bytes()); uint64_t start, end; CBS inclusion_proof, signatures; - if (!CBS_get_u64(&mtc_proof, &start) || !CBS_get_u64(&mtc_proof, &end) || + if (cert.signature_value().unused_bits() != 0 || + !CBS_get_u64(&mtc_proof, &start) || !CBS_get_u64(&mtc_proof, &end) || !CBS_get_u16_length_prefixed(&mtc_proof, &inclusion_proof) || !CBS_get_u16_length_prefixed(&mtc_proof, &signatures) || CBS_len(&mtc_proof) != 0) {
diff --git a/pki/verify_unittest.cc b/pki/verify_unittest.cc index 088de3c..b5b30a4 100644 --- a/pki/verify_unittest.cc +++ b/pki/verify_unittest.cc
@@ -186,6 +186,7 @@ void SetUp() override { ASSERT_TRUE(ReadTestCertPem("mtc-leaf.pem", &generic_cert_)); ASSERT_TRUE(ReadTestCertPem("mtc-leaf-bitflip.pem", &bitflip_cert_)); + ASSERT_TRUE(ReadTestCertPem("mtc-leaf-unused-bit.pem", &unused_bit_cert_)); ASSERT_TRUE(ReadTestCertPem("mtc-leaf-b.pem", &leaf_b_)); ASSERT_TRUE(ReadTestCertPem("mtc-leaf-c.pem", &leaf_c_)); @@ -249,6 +250,7 @@ protected: std::string generic_cert_; std::string bitflip_cert_; + std::string unused_bit_cert_; std::string leaf_b_; std::string leaf_c_; @@ -326,6 +328,21 @@ VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE); } +TEST_F(VerifyMTCTest, UnusedBit) { + std::unique_ptr<VerifyTrustStore> trust_store = EmptyTrustStore(); + std::vector<TrustedSubtree> trusted_subtrees = {generic_cert_subtree_}; + auto mtc_anchor = std::make_shared<MTCAnchor>(MakeSpan(kAnchorLogId), + MakeSpan(trusted_subtrees)); + ASSERT_TRUE(trust_store->trust_store->AddMTCTrustAnchor(mtc_anchor)); + + CertificateVerifyOptions opts; + VerifyError error; + ASSERT_TRUE(PrepareOptsForVerify(unused_bit_cert_, trust_store.get(), &opts)); + EXPECT_FALSE(CertificateVerify(opts, &error)) << error.DiagnosticString(); + EXPECT_EQ(error.Code(), + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE); +} + TEST_F(VerifyMTCTest, WrongLogID) { // Trust the correct subtree for |generic_cert_| but with the wrong log ID. // Verifying the cert should fail because even though the proof evaluates to a