Bug 1492414 - Modify CertBlocklist interface to a form that should work with Rust XPCom bindings r=keeler
authorMark Goodwin <mgoodwin@mozilla.com>
Fri, 21 Sep 2018 13:10:40 +0000
changeset 496747 6623a2039f219d2b164b0930d37df0b91700f296
parent 496746 f3ad3c9327b92fa9da92f20ffab864da79328676
child 496748 001c36e011c4873c6f2362091ea8af16d4d6e9f7
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1492414
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1492414 - Modify CertBlocklist interface to a form that should work with Rust XPCom bindings r=keeler Differential Revision: https://phabricator.services.mozilla.com/D6260
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/CSTrustDomain.cpp
security/manager/ssl/CertBlocklist.cpp
security/manager/ssl/nsICertBlocklist.idl
security/manager/ssl/tests/unit/test_cert_blocklist.js
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -201,26 +201,30 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
   if (!mCertBlocklist) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
   // The certificate blocklist currently only applies to TLS server
   // certificates.
   if (mCertDBTrustType == trustSSL) {
     bool isCertRevoked;
-    nsresult nsrv = mCertBlocklist->IsCertRevoked(
-                      candidateCert->derIssuer.data,
-                      candidateCert->derIssuer.len,
-                      candidateCert->serialNumber.data,
-                      candidateCert->serialNumber.len,
-                      candidateCert->derSubject.data,
-                      candidateCert->derSubject.len,
-                      candidateCert->derPublicKey.data,
-                      candidateCert->derPublicKey.len,
-                      &isCertRevoked);
+
+    nsAutoCString encIssuer;
+    nsAutoCString encSerial;
+    nsAutoCString encSubject;
+    nsAutoCString encPubKey;
+
+    nsresult nsrv = BuildRevocationCheckStrings(candidateCert.get(), encIssuer, encSerial, encSubject, encPubKey);
+
+    if (NS_FAILED(nsrv))  {
+      return Result::FATAL_ERROR_LIBRARY_FAILURE;
+    }
+
+    nsrv = mCertBlocklist->IsCertRevoked(
+      encIssuer, encSerial, encSubject, encPubKey, &isCertRevoked);
     if (NS_FAILED(nsrv)) {
       return Result::FATAL_ERROR_LIBRARY_FAILURE;
     }
 
     if (isCertRevoked) {
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
              ("NSSCertDBTrustDomain: certificate is in blocklist"));
       return Result::ERROR_REVOKED_CERTIFICATE;
@@ -1280,16 +1284,57 @@ DefaultServerNicknameForCert(const CERTC
     if (!conflict) {
       return NS_OK;
     }
   }
 
   return NS_ERROR_FAILURE;
 }
 
+nsresult
+BuildRevocationCheckStrings(const CERTCertificate* cert,
+                    /*out*/ nsCString& encIssuer,
+                    /*out*/ nsCString& encSerial,
+                    /*out*/ nsCString& encSubject,
+                    /*out*/ nsCString& encPubKey)
+{
+  // Convert issuer, serial, subject and pubKey data to Base64 encoded DER
+  nsDependentCSubstring issuerString(
+    BitwiseCast<char*, uint8_t*>(cert->derIssuer.data),
+    cert->derIssuer.len);
+  nsDependentCSubstring serialString(
+    BitwiseCast<char*, uint8_t*>(cert->serialNumber.data),
+    cert->serialNumber.len);
+  nsDependentCSubstring subjectString(
+    BitwiseCast<char*, uint8_t*>(cert->derSubject.data),
+    cert->derSubject.len);
+  nsDependentCSubstring pubKeyString(
+    BitwiseCast<char*, uint8_t*>(cert->derPublicKey.data),
+    cert->derPublicKey.len);
+
+  nsresult rv = Base64Encode(issuerString, encIssuer);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Encode(serialString, encSerial);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Encode(subjectString, encSubject);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Encode(pubKeyString, encPubKey);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  return NS_OK;
+}
+
 /**
  * Given a list of certificates representing a verified certificate path from an
  * end-entity certificate to a trust anchor, imports the intermediate
  * certificates into the permanent certificate database. This is an attempt to
  * cope with misconfigured servers that don't include the appropriate
  * intermediate certificates in the TLS handshake.
  *
  * @param certList the verified certificate list
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -53,16 +53,40 @@ void DisableMD5();
  */
 bool LoadLoadableRoots(const nsCString& dir);
 
 void UnloadLoadableRoots();
 
 nsresult DefaultServerNicknameForCert(const CERTCertificate* cert,
                               /*out*/ nsCString& nickname);
 
+/**
+ * Build strings of base64 encoded issuer, serial, subject and public key data
+ * from the supplied certificate for use in revocation checks.
+ *
+ * @param cert
+ *        The CERTCertificate* from which to extract the data.
+ * @param out encIssuer
+ *        The string to populate with base64 encoded issuer data.
+ * @param out encSerial
+ *        The string to populate with base64 encoded serial number data.
+ * @param out encSubject
+ *        The string to populate with base64 encoded subject data.
+ * @param out encPubKey
+ *        The string to populate with base64 encoded public key data.
+ * @return
+ *        NS_OK, unless there's a Base64 encoding problem, in which case
+ *        NS_ERROR_FAILURE.
+ */
+nsresult BuildRevocationCheckStrings(const CERTCertificate* cert,
+                              /*out*/ nsCString& encIssuer,
+                              /*out*/ nsCString& encSerial,
+                              /*out*/ nsCString& encSubject,
+                              /*out*/ nsCString& encPubKey);
+
 void SaveIntermediateCerts(const UniqueCERTCertList& certList);
 
 class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain
 {
 
 public:
   typedef mozilla::pkix::Result Result;
 
--- a/security/manager/ssl/CSTrustDomain.cpp
+++ b/security/manager/ssl/CSTrustDomain.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CSTrustDomain.h"
 #include "mozilla/Base64.h"
 #include "mozilla/Preferences.h"
 #include "nsNSSCertificate.h"
 #include "nsNSSComponent.h"
+#include "NSSCertDBTrustDomain.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "pkix/pkixnss.h"
 
 using namespace mozilla::pkix;
 
 namespace mozilla { namespace psm {
 
@@ -39,27 +40,29 @@ CSTrustDomain::GetCertTrust(EndEntityOrC
   SECItem candidateCertDERSECItem = UnsafeMapInputToSECItem(candidateCertDER);
   UniqueCERTCertificate candidateCert(
     CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &candidateCertDERSECItem,
                             nullptr, false, true));
   if (!candidateCert) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
 
+  nsAutoCString encIssuer;
+  nsAutoCString encSerial;
+  nsAutoCString encSubject;
+  nsAutoCString encPubKey;
+
+  nsresult nsrv = BuildRevocationCheckStrings(candidateCert.get(), encIssuer, encSerial, encSubject, encPubKey);
+  if (NS_FAILED(nsrv)) {
+    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+  }
+
   bool isCertRevoked;
-  nsresult nsrv = mCertBlocklist->IsCertRevoked(
-                    candidateCert->derIssuer.data,
-                    candidateCert->derIssuer.len,
-                    candidateCert->serialNumber.data,
-                    candidateCert->serialNumber.len,
-                    candidateCert->derSubject.data,
-                    candidateCert->derSubject.len,
-                    candidateCert->derPublicKey.data,
-                    candidateCert->derPublicKey.len,
-                    &isCertRevoked);
+  nsrv = mCertBlocklist->IsCertRevoked(
+    encIssuer, encSerial, encSubject, encPubKey, &isCertRevoked);
   if (NS_FAILED(nsrv)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
   if (isCertRevoked) {
     CSTrust_LOG(("CSTrustDomain: certificate is revoked\n"));
     return Result::ERROR_REVOKED_CERTIFICATE;
   }
--- a/security/manager/ssl/CertBlocklist.cpp
+++ b/security/manager/ssl/CertBlocklist.cpp
@@ -503,58 +503,64 @@ CertBlocklist::SaveEntries()
            ("CertBlocklist::SaveEntries saving revocation data failed"));
     return rv;
   }
   mModified = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-CertBlocklist::IsCertRevoked(const uint8_t* aIssuer,
-                             uint32_t aIssuerLength,
-                             const uint8_t* aSerial,
-                             uint32_t aSerialLength,
-                             const uint8_t* aSubject,
-                             uint32_t aSubjectLength,
-                             const uint8_t* aPubKey,
-                             uint32_t aPubKeyLength,
+CertBlocklist::IsCertRevoked(const nsACString& aIssuerString,
+                             const nsACString& aSerialNumberString,
+                             const nsACString& aSubjectString,
+                             const nsACString& aPubKeyString,
                              bool* _retval)
 {
   MutexAutoLock lock(mMutex);
+  MOZ_LOG(gCertBlockPRLog, LogLevel::Warning, ("CertBlocklist::IsCertRevoked"));
 
-  MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
-          ("CertBlocklist::IsCertRevoked?"));
-  nsresult rv = EnsureBackingFileInitialized(lock);
+  nsCString decodedIssuer;
+  nsCString decodedSerial;
+  nsCString decodedSubject;
+  nsCString decodedPubKey;
+
+  nsresult rv = Base64Decode(aIssuerString, decodedIssuer);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Decode(aSerialNumberString, decodedSerial);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Decode(aSubjectString, decodedSubject);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = Base64Decode(aPubKeyString, decodedPubKey);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  Input issuer;
-  Input serial;
-  if (issuer.Init(aIssuer, aIssuerLength) != Success) {
-    return NS_ERROR_FAILURE;
-  }
-  if (serial.Init(aSerial, aSerialLength) != Success) {
-    return NS_ERROR_FAILURE;
-  }
-
-  CertBlocklistItem issuerSerial(aIssuer, aIssuerLength, aSerial, aSerialLength,
-                                 BlockByIssuerAndSerial);
-
-  nsAutoCString encDN;
-  nsAutoCString encOther;
-
-  issuerSerial.ToBase64(encDN, encOther);
+  rv = EnsureBackingFileInitialized(lock);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
+  CertBlocklistItem issuerSerial(
+    BitwiseCast<const uint8_t*, const char*>(decodedIssuer.get()),
+    decodedIssuer.Length(),
+    BitwiseCast<const uint8_t*, const char*>(decodedSerial.get()),
+    decodedSerial.Length(),
+    BlockByIssuerAndSerial);
+
+  MOZ_LOG(gCertBlockPRLog,
+          LogLevel::Warning,
           ("CertBlocklist::IsCertRevoked issuer %s - serial %s",
-           encDN.get(), encOther.get()));
+           PromiseFlatCString(aIssuerString).get(),
+           PromiseFlatCString(aSerialNumberString).get()));
 
   *_retval = mBlocklist.Contains(issuerSerial);
 
   if (*_retval) {
     MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
             ("certblocklist::IsCertRevoked found by issuer / serial"));
     return NS_OK;
   }
@@ -562,42 +568,48 @@ CertBlocklist::IsCertRevoked(const uint8
   nsCOMPtr<nsICryptoHash> crypto;
   crypto = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
 
   rv = crypto->Init(nsICryptoHash::SHA256);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  rv = crypto->Update(aPubKey, aPubKeyLength);
+  rv = crypto->Update(
+    BitwiseCast<const uint8_t*, const char*>(decodedPubKey.get()),
+    decodedPubKey.Length());
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   nsCString hashString;
   rv = crypto->Finish(false, hashString);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   CertBlocklistItem subjectPubKey(
-    aSubject,
-    static_cast<size_t>(aSubjectLength),
+    BitwiseCast<const uint8_t*, const char*>(decodedSubject.get()),
+    decodedSubject.Length(),
     BitwiseCast<const uint8_t*, const char*>(hashString.get()),
     hashString.Length(),
     BlockBySubjectAndPubKey);
 
-  rv = subjectPubKey.ToBase64(encDN, encOther);
+  nsCString encodedHash;
+  rv = Base64Encode(hashString, encodedHash);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
-          ("CertBlocklist::IsCertRevoked subject %s - pubKey hash %s",
-           encDN.get(), encOther.get()));
+  MOZ_LOG(gCertBlockPRLog,
+          LogLevel::Warning,
+          ("CertBlocklist::IsCertRevoked subject %s - pubKeyHash %s (pubKey %s)",
+           PromiseFlatCString(aSubjectString).get(),
+           PromiseFlatCString(encodedHash).get(),
+           PromiseFlatCString(aPubKeyString).get()));
   *_retval = mBlocklist.Contains(subjectPubKey);
 
   MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
           ("CertBlocklist::IsCertRevoked by subject / pubkey? %s",
            *_retval ? "true" : "false"));
 
   return NS_OK;
 }
--- a/security/manager/ssl/nsICertBlocklist.idl
+++ b/security/manager/ssl/nsICertBlocklist.idl
@@ -37,30 +37,26 @@ interface nsICertBlocklist : nsISupports
    * Persist (fresh) blocklist entries to the profile (if a profile directory is
    * available). Note: calling this will result in synchronous I/O.
    */
   [must_use]
   void saveEntries();
 
   /**
    * Check if a certificate is blocked.
-   * issuer - issuer name, DER encoded
-   * serial - serial number, DER encoded
-   * subject - subject name, DER encoded
-   * pubkey - public key, DER encoded
+   * issuer - issuer name, DER, Base64 encoded
+   * serial - serial number, DER, BAse64 encoded
+   * subject - subject name, DER, Base64 encoded
+   * pubkey - public key, DER, Base64 encoded
    */
   [must_use]
-  boolean isCertRevoked([const, array, size_is(issuer_length)] in octet issuer,
-                        in unsigned long issuer_length,
-                        [const, array, size_is(serial_length)] in octet serial,
-                        in unsigned long serial_length,
-                        [const, array, size_is(subject_length)] in octet subject,
-                        in unsigned long subject_length,
-                        [const, array, size_is(pubkey_length)] in octet pubkey,
-                        in unsigned long pubkey_length);
+  boolean isCertRevoked(in ACString issuer,
+                        in ACString serial,
+                        in ACString subject,
+                        in ACString pubkey);
 
    /**
     * Check that the blocklist data is current. Specifically, that the current
     * time is no more than security.onecrl.maximum_staleness_in_seconds seconds
     * after the last blocklist update (as stored in the
     * app.update.lastUpdateTime.blocklist-background-update-timer pref)
     */
   [must_use]
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -157,20 +157,16 @@ testserver.start(-1);
 var port = testserver.identity.primaryPort;
 
 // Setup the addonManager
 var addonManager = Cc["@mozilla.org/addons/integration;1"]
                      .getService(Ci.nsIObserver)
                      .QueryInterface(Ci.nsITimerCallback);
 addonManager.observe(null, "addons-startup", null);
 
-var converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
-                  .createInstance(Ci.nsIScriptableUnicodeConverter);
-converter.charset = "UTF-8";
-
 function verify_cert(file, expectedError) {
   let ee = constructCertFromFile(file);
   return checkCertErrorGeneric(certDB, ee, expectedError,
                                certificateUsageSSLServer);
 }
 
 // The certificate blocklist currently only applies to TLS server certificates.
 async function verify_non_tls_usage_succeeds(file) {
@@ -185,29 +181,18 @@ async function verify_non_tls_usage_succ
 
 function load_cert(cert, trust) {
   let file = "bad_certs/" + cert + ".pem";
   addCertFromFile(certDB, file, trust);
 }
 
 function test_is_revoked(certList, issuerString, serialString, subjectString,
                          pubKeyString) {
-  let issuer = converter.convertToByteArray(issuerString || "", {});
-  let serial = converter.convertToByteArray(serialString || "", {});
-  let subject = converter.convertToByteArray(subjectString || "", {});
-  let pubKey = converter.convertToByteArray(pubKeyString || "", {});
-
-  return certList.isCertRevoked(issuer,
-                                issuerString ? issuerString.length : 0,
-                                serial,
-                                serialString ? serialString.length : 0,
-                                subject,
-                                subjectString ? subjectString.length : 0,
-                                pubKey,
-                                pubKeyString ? pubKeyString.length : 0);
+  return certList.isCertRevoked(btoa(issuerString), btoa(serialString),
+                                btoa(subjectString), btoa(pubKeyString));
 }
 
 function fetch_blocklist() {
   Services.prefs.setBoolPref("services.settings.load_dump", false);
   Services.prefs.setBoolPref("services.settings.verify_signature", false);
   Services.prefs.setCharPref("services.settings.server",
                              `http://localhost:${port}/v1`);