Bug 1048133 - Check key algorithms before using them for encryption/signatures r=rbarnes a=abillings
authorTim Taubert <ttaubert@mozilla.com>
Thu, 07 Aug 2014 09:02:55 +0200
changeset 208284 6a7e6708ded1
parent 208283 6b9281a057b7
child 208285 1b581df81c9b
push id3801
push userttaubert@mozilla.com
push date2014-08-10 18:55 +0000
treeherdermozilla-beta@6a7e6708ded1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrbarnes, abillings
bugs1048133
milestone32.0
Bug 1048133 - Check key algorithms before using them for encryption/signatures r=rbarnes a=abillings
dom/crypto/WebCryptoTask.cpp
dom/crypto/test/tests.js
--- a/dom/crypto/WebCryptoTask.cpp
+++ b/dom/crypto/WebCryptoTask.cpp
@@ -85,16 +85,27 @@ enum TelemetryAlgorithm {
   }
 
 // OOM-safe CryptoBuffer copy, suitable for DoCrypto
 #define ATTEMPT_BUFFER_ASSIGN(dst, src) \
   if (!dst.Assign(src)) { \
     return NS_ERROR_DOM_UNKNOWN_ERR; \
   }
 
+// Safety check for algorithms that use keys, suitable for constructors
+#define CHECK_KEY_ALGORITHM(keyAlg, algName) \
+  { \
+    nsString keyAlgName; \
+    keyAlg->GetName(keyAlgName); \
+    if (!keyAlgName.EqualsLiteral(algName)) { \
+      mEarlyRv = NS_ERROR_DOM_INVALID_ACCESS_ERR; \
+      return; \
+    } \
+  }
+
 class ClearException
 {
 public:
   ClearException(JSContext* aCx)
     : mCx(aCx)
   {}
 
   ~ClearException()
@@ -251,27 +262,31 @@ public:
     {
       mEarlyRv = NS_ERROR_DOM_DATA_ERR;
       return;
     }
 
     // Cache parameters depending on the specific algorithm
     TelemetryAlgorithm telemetryAlg;
     if (algName.EqualsLiteral(WEBCRYPTO_ALG_AES_CBC)) {
+      CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_AES_CBC);
+
       mMechanism = CKM_AES_CBC_PAD;
       telemetryAlg = TA_AES_CBC;
       AesCbcParams params;
       nsresult rv = Coerce(aCx, params, aAlgorithm);
       if (NS_FAILED(rv) || !params.mIv.WasPassed()) {
         mEarlyRv = NS_ERROR_DOM_INVALID_ACCESS_ERR;
         return;
       }
 
       ATTEMPT_BUFFER_INIT(mIv, params.mIv.Value())
     } else if (algName.EqualsLiteral(WEBCRYPTO_ALG_AES_CTR)) {
+      CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_AES_CTR);
+
       mMechanism = CKM_AES_CTR;
       telemetryAlg = TA_AES_CTR;
       AesCtrParams params;
       nsresult rv = Coerce(aCx, params, aAlgorithm);
       if (NS_FAILED(rv) || !params.mCounter.WasPassed() ||
           !params.mLength.WasPassed()) {
         mEarlyRv = NS_ERROR_DOM_SYNTAX_ERR;
         return;
@@ -280,16 +295,18 @@ public:
       ATTEMPT_BUFFER_INIT(mIv, params.mCounter.Value())
       if (mIv.Length() != 16) {
         mEarlyRv = NS_ERROR_DOM_DATA_ERR;
         return;
       }
 
       mCounterLength = params.mLength.Value();
     } else if (algName.EqualsLiteral(WEBCRYPTO_ALG_AES_GCM)) {
+      CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_AES_GCM);
+
       mMechanism = CKM_AES_GCM;
       telemetryAlg = TA_AES_GCM;
       AesGcmParams params;
       nsresult rv = Coerce(aCx, params, aAlgorithm);
       if (NS_FAILED(rv) || !params.mIv.WasPassed()) {
         mEarlyRv = NS_ERROR_DOM_SYNTAX_ERR;
         return;
       }
@@ -407,16 +424,18 @@ public:
                  mozilla::dom::Key& aKey, const CryptoOperationData& aData,
                  bool aEncrypt)
     : mPrivKey(aKey.GetPrivateKey())
     , mPubKey(aKey.GetPublicKey())
     , mEncrypt(aEncrypt)
   {
     Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_RSAES_PKCS1);
 
+    CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_RSAES_PKCS1);
+
     ATTEMPT_BUFFER_INIT(mData, aData);
 
     if (mEncrypt) {
       if (!mPubKey) {
         mEarlyRv = NS_ERROR_DOM_INVALID_ACCESS_ERR;
         return;
       }
       mStrength = SECKEY_PublicKeyStrength(mPubKey);
@@ -480,16 +499,18 @@ public:
            mozilla::dom::Key& aKey,
            const CryptoOperationData& aSignature,
            const CryptoOperationData& aData,
            bool aSign)
     : mMechanism(aKey.Algorithm()->Mechanism())
     , mSymKey(aKey.GetSymKey())
     , mSign(aSign)
   {
+    CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_HMAC);
+
     ATTEMPT_BUFFER_INIT(mData, aData);
     if (!aSign) {
       ATTEMPT_BUFFER_INIT(mSignature, aSignature);
     }
 
     // Check that we got a symmetric key
     if (mSymKey.Length() == 0) {
       mEarlyRv = NS_ERROR_DOM_DATA_ERR;
@@ -587,16 +608,18 @@ public:
     : mOidTag(SEC_OID_UNKNOWN)
     , mPrivKey(aKey.GetPrivateKey())
     , mPubKey(aKey.GetPublicKey())
     , mSign(aSign)
     , mVerified(false)
   {
     Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_RSASSA_PKCS1);
 
+    CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_RSASSA_PKCS1);
+
     ATTEMPT_BUFFER_INIT(mData, aData);
     if (!aSign) {
       ATTEMPT_BUFFER_INIT(mSignature, aSignature);
     }
 
     // Look up the SECOidTag based on the KeyAlgorithm
     // static_cast is safe because we only get here if the algorithm name
     // is RSASSA-PKCS1-v1_5, and that only happens if we've constructed
--- a/dom/crypto/test/tests.js
+++ b/dom/crypto/test/tests.js
@@ -882,8 +882,44 @@ TestArray.addTest(
       .then(
         complete(that, function(x) { return !x; }),
         fail
       );
   }
 );
 
 
+// -----------------------------------------------------------------------------
+TestArray.addTest(
+  "Test that we check keys before using them for encryption/signatures",
+  function() {
+    var that = this;
+
+    function doCheckRSASSA() {
+      var alg = {name: "HMAC", hash: {name: "SHA-1"}};
+
+      function doSign(x) {
+        return crypto.subtle.sign("RSASSA-PKCS1-v1_5", x, new Uint8Array());
+      }
+
+      return crypto.subtle.generateKey(alg, false, ["sign"]).then(doSign);
+    }
+
+    function doCheckRSAES() {
+      var alg = {
+        name: "RSAES-PKCS1-v1_5",
+        modulusLength: 1024,
+        publicExponent: new Uint8Array([0x01, 0x00, 0x01])
+      };
+
+      function doEncrypt(x) {
+        var alg = {name: "RSA-OAEP", hash: "SHA-1"};
+        return crypto.subtle.encrypt(alg, x.publicKey, new Uint8Array());
+      }
+
+      return crypto.subtle.generateKey(alg, false, ["encrypt"]).then(doEncrypt);
+    }
+
+    doCheckRSASSA().then(error(that), function () {
+      doCheckRSAES().then(error(that), complete(that));
+    });
+  }
+);