Bug 1258647 - Content-Signature telemetry, r=keeler,francois

Differential Revision: https://phabricator.services.mozilla.com/D427

--HG--
extra : rebase_source : fb7f05afdc9b718a804190b1f4718ec2ae2d93cb

security/manager/ssl/ContentSignatureVerifier.cpp

@@ -65,6 +65,9 @@ ContentSignatureVerifier::VerifyContentSignature(
     if (rv == NS_ERROR_INVALID_SIGNATURE) {
       return NS_OK;
     }
+    // This failure can have many different reasons but we don't treat it as
+    // invalid signature.
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 3);
     return rv;
   }
 
@@ -191,7 +194,17 @@ ContentSignatureVerifier::CreateContextInternal(const nsACString& aData,
       return NS_ERROR_FAILURE;
     }
     // otherwise, assume the signature was invalid
-    CSVerifier_LOG(("CSVerifier: The supplied chain is bad\n"));
+    if (result == mozilla::pkix::Result::ERROR_EXPIRED_CERTIFICATE) {
+      Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 4);
+    } else if (result ==
+               mozilla::pkix::Result::ERROR_NOT_YET_VALID_CERTIFICATE) {
+      Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 5);
+    } else {
+      // Building cert chain failed for some other reason.
+      Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 6);
+    }
+    CSVerifier_LOG(("CSVerifier: The supplied chain is bad (%s)\n",
+                    MapResultToName(result)));
     return NS_ERROR_INVALID_SIGNATURE;
   }
 
@@ -208,6 +221,8 @@ ContentSignatureVerifier::CreateContextInternal(const nsACString& aData,
   BRNameMatchingPolicy nameMatchingPolicy(BRNameMatchingPolicy::Mode::Enforce);
   result = CheckCertHostname(certDER, hostnameInput, nameMatchingPolicy);
   if (result != Success) {
+    // EE cert isnot valid for the given host name.
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 7);
     return NS_ERROR_INVALID_SIGNATURE;
   }
 
@@ -215,6 +230,7 @@ ContentSignatureVerifier::CreateContextInternal(const nsACString& aData,
 
   // in case we were not able to extract a key
   if (!mKey) {
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 8);
     CSVerifier_LOG(("CSVerifier: unable to extract a key\n"));
     return NS_ERROR_INVALID_SIGNATURE;
   }
@@ -253,10 +269,14 @@ ContentSignatureVerifier::CreateContextInternal(const nsACString& aData,
   mCx = UniqueVFYContext(
     VFY_CreateContext(mKey.get(), &signatureItem, oid, nullptr));
   if (!mCx) {
+    // Creating context failed.
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 9);
     return NS_ERROR_INVALID_SIGNATURE;
   }
 
   if (VFY_Begin(mCx.get()) != SECSuccess) {
+    // Creating context failed.
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 9);
     return NS_ERROR_INVALID_SIGNATURE;
   }
 
@@ -423,13 +443,20 @@ ContentSignatureVerifier::End(bool* _retval)
 
   // If we didn't create the context yet, bail!
   if (!mHasCertChain) {
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 2);
     MOZ_ASSERT_UNREACHABLE(
       "Someone called ContentSignatureVerifier::End before "
       "downloading the cert chain.");
     return NS_ERROR_FAILURE;
   }
 
-  *_retval = (VFY_End(mCx.get()) == SECSuccess);
+  bool result = (VFY_End(mCx.get()) == SECSuccess);
+  if (result) {
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 0);
+  } else {
+    Accumulate(Telemetry::CONTENT_SIGNATURE_VERIFICATION_STATUS, 1);
+  }
+  *_retval = result;
 
   return NS_OK;
 }

security/manager/ssl/tests/unit/test_content_signing.js

@@ -13,6 +13,8 @@ const TEST_DATA_DIR = "test_content_signing/";
 
 const ONECRL_NAME = "oneCRL-signer.mozilla.org";
 const ABOUT_NEWTAB_NAME = "remotenewtab.content-signature.mozilla.org";
+var VERIFICATION_HISTOGRAM = Services.telemetry
+                                     .getHistogramById("CONTENT_SIGNATURE_VERIFICATION_STATUS");
 
 function getSignatureVerifier() {
   return Cc["@mozilla.org/security/contentsignatureverifier;1"]
@@ -33,6 +35,19 @@ function loadChain(prefix, names) {
   return chain;
 }
 
+function check_telemetry(expected_index, expected) {
+  for (let i = 0; i < 10; i++) {
+    let expected_value = 0;
+    if (i == expected_index) {
+      expected_value = expected;
+    }
+    equal(VERIFICATION_HISTOGRAM.snapshot().counts[i], expected_value,
+      "count " + i + ": " + VERIFICATION_HISTOGRAM.snapshot().counts[i] +
+      " expected " + expected_value);
+  }
+  VERIFICATION_HISTOGRAM.clear();
+}
+
 function run_test() {
   // set up some data
   const DATA = readFile(do_get_file(TEST_DATA_DIR + "test.txt"));
@@ -56,11 +71,21 @@ function run_test() {
   let noSANChain = loadChain(TEST_DATA_DIR + "content_signing",
                              ["onecrl_no_SAN_ee", "int", "root"]);
 
+  let expiredOneCRLChain = loadChain(TEST_DATA_DIR + "content_signing",
+                                     ["onecrl_ee_expired", "int", "root"]);
+
+  let notValidYetOneCRLChain = loadChain(TEST_DATA_DIR + "content_signing",
+                                         ["onecrl_ee_not_valid_yet", "int",
+                                          "root"]);
+
   // Check signature verification works without error before the root is set
+  VERIFICATION_HISTOGRAM.clear();
   let chain1 = oneCRLChain.join("\n");
   let verifier = getSignatureVerifier();
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain1, ONECRL_NAME),
      "Before the root is set, signatures should fail to verify but not throw.");
+  // Check for generic chain building error.
+  check_telemetry(6, 1);
 
   setRoot(TEST_DATA_DIR + "content_signing_root.pem");
 
@@ -73,12 +98,16 @@ function run_test() {
   ok(verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain2,
                                      ABOUT_NEWTAB_NAME),
      "A newtab signature should verify with the newtab chain");
+  // Check for valid signature
+  check_telemetry(0, 2);
 
   // Check a bad signature when a good chain is provided
   chain1 = oneCRLChain.join("\n");
   verifier = getSignatureVerifier();
   ok(!verifier.verifyContentSignature(DATA, BAD_SIGNATURE, chain1, ONECRL_NAME),
      "A bad signature should not verify");
+  // Check for invalid signature
+  check_telemetry(1, 1);
 
   // Check a good signature from cert with good SAN but a different key than the
   // one used to create the signature
@@ -87,6 +116,8 @@ function run_test() {
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, badKeyChain,
                                       ONECRL_NAME),
      "A signature should not verify if the signing key is wrong");
+  // Check for wrong key in cert.
+  check_telemetry(9, 1);
 
   // Check a good signature from cert with good SAN but a different key than the
   // one used to create the signature (this time, an RSA key)
@@ -95,6 +126,8 @@ function run_test() {
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, rsaKeyChain,
                                       ONECRL_NAME),
      "A signature should not verify if the signing key is wrong (RSA)");
+  // Check for wrong key in cert.
+  check_telemetry(9, 1);
 
   // Check a good signature from cert with good SAN but with chain missing root
   let missingRoot = [oneCRLChain[0], oneCRLChain[1]].join("\n");
@@ -102,6 +135,8 @@ function run_test() {
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, missingRoot,
                                       ONECRL_NAME),
      "A signature should not verify if the chain is incomplete (missing root)");
+  // Check for generic chain building error.
+  check_telemetry(6, 1);
 
   // Check a good signature from cert with good SAN but with no path to root
   let missingInt = [oneCRLChain[0], oneCRLChain[2]].join("\n");
@@ -109,6 +144,8 @@ function run_test() {
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, missingInt,
                                       ONECRL_NAME),
      "A signature should not verify if the chain is incomplete (missing int)");
+  // Check for generic chain building error.
+  check_telemetry(6, 1);
 
   // Check good signatures from good certificates with the wrong SANs
   chain1 = oneCRLChain.join("\n");
@@ -116,17 +153,39 @@ function run_test() {
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain1,
                                       ABOUT_NEWTAB_NAME),
      "A OneCRL signature should not verify if we require the newtab SAN");
+  // Check for invalid EE cert.
+  check_telemetry(7, 1);
 
   chain2 = remoteNewTabChain.join("\n");
   verifier = getSignatureVerifier();
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain2,
                                       ONECRL_NAME),
      "A newtab signature should not verify if we require the OneCRL SAN");
+  // Check for invalid EE cert.
+  check_telemetry(7, 1);
 
   // Check good signatures with good chains with some other invalid names
   verifier = getSignatureVerifier();
   ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain1, ""),
      "A signature should not verify if the SANs do not match an empty name");
+  // Check for invalid EE cert.
+  check_telemetry(7, 1);
+
+  // Test expired certificate.
+  let chainExpired = expiredOneCRLChain.join("\n");
+  verifier = getSignatureVerifier();
+  ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chainExpired, ""),
+     "A signature should not verify if the signing certificate is expired");
+  // Check for expired cert.
+  check_telemetry(4, 1);
+
+  // Test not valid yet certificate.
+  let chainNotValidYet = notValidYetOneCRLChain.join("\n");
+  verifier = getSignatureVerifier();
+  ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chainNotValidYet, ""),
+     "A signature should not verify if the signing certificate is not valid yet");
+  // Check for not yet valid cert.
+  check_telemetry(5, 1);
 
   let relatedName = "subdomain." + ONECRL_NAME;
   verifier = getSignatureVerifier();

security/manager/ssl/tests/unit/test_content_signing/content_signing_onecrl_ee_expired.pem

@@ -0,0 +1,15 @@
+-----BEGIN CERTIFICATE-----
+MIICTjCCATagAwIBAgIUOQNrYQz01j0SirgoHMLKbtGL9RowDQYJKoZIhvcNAQEL
+BQAwETEPMA0GA1UEAwwGaW50LUNBMCIYDzIwMTMwMTAxMDAwMDAwWhgPMjAxNDAx
+MDEwMDAwMDBaMBwxGjAYBgNVBAMMEWVlLWludC1DQS1leHBpcmVkMHYwEAYHKoZI
+zj0CAQYFK4EEACIDYgAEoWhyQzYrXHsYifN5FUYVocc/tI3uhj4CKRXbYI4lLeS3
+Ey2ozpjoMVNOapwMCwnI1jmt6DIG5bqBNHOhH6Mw4F2oyW5Dg/4nhz2pcQO+KIjP
+8ALwWvcaH93Mg3SqbqnOoz0wOzATBgNVHSUEDDAKBggrBgEFBQcDAzAkBgNVHREE
+HTAbghlvbmVDUkwtc2lnbmVyLm1vemlsbGEub3JnMA0GCSqGSIb3DQEBCwUAA4IB
+AQBZJPo4llgMe5588+BnRLnFguspIiwMWmTeqCfi8VQBx/tUwRiTizbU7J2Yh9bo
+yZEPKfPSP2o8J0eSUgvXdVOxU1fNRuocsVfXUlveq5x10ddjXBT9X4AY1mtR7HJw
+hl/7269N8b4itfrfvZmCBToJayjv0I2N84bqjpOnXJ/iB5YVdk8oZIJDXWi4SR3B
+E9IejwA1fikpt++RjpJSZ1BSNU7FfiyGGUonxHDoP/29znaOJnpAqaH5LVJCRkfN
+H12vePBbunZd+ay5r+mMJPaXR+V2sY8OaOfcrPSHQLa8Eb/EEhBuITMKkOucohjx
+zqvM6S2iOI9GbwHClybEHRO7
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_content_signing/content_signing_onecrl_ee_expired.pem.certspec

@@ -0,0 +1,6 @@
+issuer:int-CA
+subject:ee-int-CA-expired
+subjectKey:secp384r1
+validity:20130101-20140101
+extension:extKeyUsage:codeSigning
+extension:subjectAlternativeName:oneCRL-signer.mozilla.org

security/manager/ssl/tests/unit/test_content_signing/content_signing_onecrl_ee_not_valid_yet.pem

@@ -0,0 +1,15 @@
+-----BEGIN CERTIFICATE-----
+MIICVDCCATygAwIBAgIUbV+rBAfhGRv/bU22A92xneoAy3owDQYJKoZIhvcNAQEL
+BQAwETEPMA0GA1UEAwwGaW50LUNBMCIYDzIwNTAwMTAxMDAwMDAwWhgPMjA1MTAx
+MDEwMDAwMDBaMCIxIDAeBgNVBAMMF2VlLWludC1DQS1ub3QteWV0LXZhbGlkMHYw
+EAYHKoZIzj0CAQYFK4EEACIDYgAEoWhyQzYrXHsYifN5FUYVocc/tI3uhj4CKRXb
+YI4lLeS3Ey2ozpjoMVNOapwMCwnI1jmt6DIG5bqBNHOhH6Mw4F2oyW5Dg/4nhz2p
+cQO+KIjP8ALwWvcaH93Mg3SqbqnOoz0wOzATBgNVHSUEDDAKBggrBgEFBQcDAzAk
+BgNVHREEHTAbghlvbmVDUkwtc2lnbmVyLm1vemlsbGEub3JnMA0GCSqGSIb3DQEB
+CwUAA4IBAQAjXmLNn2kLa/FzNp7F3PqcSXuAO2jT31Y2g4pZnVqCDfMqplsl2ZFn
+oam3wyQnepm3q9DD4BOAW9JFYR3wqnl9cBRNHlSGyjGM4qBpuSD6WxAz7EdFcRO6
+fcA50245fAuB45UJeYJ58QvIBv7AwoBGnqAI7ZDN3eIGopZIL56jiH7vO9WyQPWj
+XZAWrXTG68rEf0RxXRtjUv9coFiuInT8+oyXB3NwK2EbaI5IeR+x3qIDEgNKk+t+
+PlE3NrtaAiK19p0s9RtQQilBKNmo+5irrUq/OD2H1aurDaAXpLTM5vLUpfyN3/qD
+HzuZujaUIeMsRiXsIRDNql1S+nq4oNRy
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_content_signing/content_signing_onecrl_ee_not_valid_yet.pem.certspec

@@ -0,0 +1,6 @@
+issuer:int-CA
+subject:ee-int-CA-not-yet-valid
+subjectKey:secp384r1
+validity:20500101-20510101
+extension:extKeyUsage:codeSigning
+extension:subjectAlternativeName:oneCRL-signer.mozilla.org

security/manager/ssl/tests/unit/test_content_signing/moz.build

@@ -9,6 +9,8 @@
 #    'content_signing_int.pem',
 #    'content_signing_onecrl_RSA_ee.pem',
 #    'content_signing_onecrl_ee.pem',
+#    'content_signing_onecrl_ee_expired.pem',
+#    'content_signing_onecrl_ee_not_valid_yet.pem',
 #    'content_signing_onecrl_no_SAN_ee.pem',
 #    'content_signing_onecrl_wrong_key_ee.pem',
 #    'content_signing_remote_newtab_ee.pem',

toolkit/components/telemetry/Histograms.json

@@ -10341,6 +10341,15 @@
     "n_values": 10,
     "description": "How often would blocked mixed content be allowed if HSTS upgrades were allowed? 0=display/no-HSTS, 1=display/HSTS, 2=active/no-HSTS, 3=active/HSTS"
   },
+  "CONTENT_SIGNATURE_VERIFICATION_STATUS": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["seceng-telemetry@mozilla.com", "fkiefer@mozilla.com"],
+    "expires_in_version": "never",
+    "kind": "enumerated",
+    "n_values": 20,
+    "bug_numbers": [1258647],
+    "description": "What was the result of the content signature verification? 0=valid, 1=invalid, 2=noCertChain, 3=createContextFailedWithOtherError, 4=expiredCert, 5=certNotValidYet, 6=buildCertChainFailed, 7=eeCertForWrongHost, 8=extractKeyError, 9=vfyContextError"
+  },
   "HSTS_UPGRADE_SOURCE": {
     "record_in_processes": [ "main" ],
     "alert_emails": ["seceng-telemetry@mozilla.com"],