bug 882865 - cryptojs key gen cleanup: use EqualsLiteral() r=bsmith r=Ms2ger r=khuey a=lsblakk
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 12 Jul 2013 10:00:22 -0700
changeset 143119 fdd8ebfb46de
parent 143118 b2c6abf26c62
child 143120 8f0702eea920
push id2661
push userdkeeler@mozilla.com
push date2013-07-22 18:56 +0000
treeherdermozilla-beta@fdd8ebfb46de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmith, Ms2ger, khuey, lsblakk
bugs882865
milestone23.0
bug 882865 - cryptojs key gen cleanup: use EqualsLiteral() r=bsmith r=Ms2ger r=khuey a=lsblakk
security/manager/ssl/src/nsCrypto.cpp
security/manager/ssl/tests/mochitest/bugs/Makefile.in
security/manager/ssl/tests/mochitest/bugs/test_bug882865.html
--- a/security/manager/ssl/src/nsCrypto.cpp
+++ b/security/manager/ssl/src/nsCrypto.cpp
@@ -31,16 +31,17 @@
 #include "nsIDOMClassInfo.h"
 #include "nsIDOMDocument.h"
 #include "nsIDocument.h"
 #include "nsIScriptObjectPrincipal.h"
 #include "nsIScriptContext.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsContentUtils.h"
 #include "nsDOMJSUtils.h"
+#include "nsJSUtils.h"
 #include "nsIXPConnect.h"
 #include "nsIRunnable.h"
 #include "nsIWindowWatcher.h"
 #include "nsIPrompt.h"
 #include "nsIFilePicker.h"
 #include "nsJSPrincipals.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptSecurityManager.h"
@@ -333,63 +334,63 @@ cryptojs_convert_to_mechanism(nsKeyGenTy
     break;
   default:
     retMech = CKM_INVALID_MECHANISM;
   }
   return retMech;
 }
 
 /*
- * This function converts a string read through JavaScript parameters
+ * This function takes a string read through JavaScript parameters
  * and translates it to the internal enumeration representing the
- * key gen type.
+ * key gen type. Leading and trailing whitespace must be already removed.
  */
 static nsKeyGenType
-cryptojs_interpret_key_gen_type(char *keyAlg)
+cryptojs_interpret_key_gen_type(const nsAString& keyAlg)
 {
-  char *end;
-  if (!keyAlg) {
-    return invalidKeyGen;
+  if (keyAlg.EqualsLiteral("rsa-ex")) {
+    return rsaEnc;
   }
-  /* First let's remove all leading and trailing white space */
-  while (isspace(keyAlg[0])) keyAlg++;
-  end = strchr(keyAlg, '\0');
-  if (!end) {
-    return invalidKeyGen;
+  if (keyAlg.EqualsLiteral("rsa-dual-use")) {
+    return rsaDualUse;
   }
-  end--;
-  while (isspace(*end)) end--;
-  end[1] = '\0';
-  if (strcmp(keyAlg, "rsa-ex") == 0) {
-    return rsaEnc;
-  } else if (strcmp(keyAlg, "rsa-dual-use") == 0) {
-    return rsaDualUse;
-  } else if (strcmp(keyAlg, "rsa-sign") == 0) {
+  if (keyAlg.EqualsLiteral("rsa-sign")) {
     return rsaSign;
-  } else if (strcmp(keyAlg, "rsa-sign-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("rsa-sign-nonrepudiation")) {
     return rsaSignNonrepudiation;
-  } else if (strcmp(keyAlg, "rsa-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("rsa-nonrepudiation")) {
     return rsaNonrepudiation;
-  } else if (strcmp(keyAlg, "ec-ex") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("ec-ex")) {
     return ecEnc;
-  } else if (strcmp(keyAlg, "ec-dual-use") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("ec-dual-use")) {
     return ecDualUse;
-  } else if (strcmp(keyAlg, "ec-sign") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("ec-sign")) {
     return ecSign;
-  } else if (strcmp(keyAlg, "ec-sign-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("ec-sign-nonrepudiation")) {
     return ecSignNonrepudiation;
-  } else if (strcmp(keyAlg, "ec-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("ec-nonrepudiation")) {
     return ecNonrepudiation;
-  } else if (strcmp(keyAlg, "dsa-sign-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("dsa-sign-nonrepudiation")) {
     return dsaSignNonrepudiation;
-  } else if (strcmp(keyAlg, "dsa-sign") ==0 ){
+  }
+  if (keyAlg.EqualsLiteral("dsa-sign")) {
     return dsaSign;
-  } else if (strcmp(keyAlg, "dsa-nonrepudiation") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("dsa-nonrepudiation")) {
     return dsaNonrepudiation;
-  } else if (strcmp(keyAlg, "dh-ex") == 0) {
+  }
+  if (keyAlg.EqualsLiteral("dh-ex")) {
     return dhEx;
   }
   return invalidKeyGen;
 }
 
 /* 
  * input: null terminated char* pointing to (the remainder of) an
  * EC key param string.
@@ -910,17 +911,17 @@ cryptojs_generateOneKeyPair(JSContext *c
 static nsresult
 cryptojs_ReadArgsAndGenerateKey(JSContext *cx,
                                 JS::Value *argv,
                                 nsKeyPairInfo *keyGenType,
                                 nsIInterfaceRequestor *uiCxt,
                                 PK11SlotInfo **slot, bool willEscrow)
 {
   JSString  *jsString;
-  JSAutoByteString params, keyGenAlg;
+  JSAutoByteString params;
   int    keySize;
   nsresult  rv;
 
   if (!JSVAL_IS_INT(argv[0])) {
     JS_ReportError(cx, "%s%s\n", JS_ERROR,
                    "passed in non-integer for key size");
     return NS_ERROR_FAILURE;
   }
@@ -936,38 +937,42 @@ cryptojs_ReadArgsAndGenerateKey(JSContex
   if (JSVAL_IS_NULL(argv[2])) {
     JS_ReportError(cx,"%s%s\n", JS_ERROR,
              "key generation type not specified");
     return NS_ERROR_FAILURE;
   }
   jsString = JS_ValueToString(cx, argv[2]);
   NS_ENSURE_TRUE(jsString, NS_ERROR_OUT_OF_MEMORY);
   argv[2] = STRING_TO_JSVAL(jsString);
-  keyGenAlg.encodeLatin1(cx, jsString);
-  NS_ENSURE_TRUE(!!keyGenAlg, NS_ERROR_OUT_OF_MEMORY);
-  keyGenType->keyGenType = cryptojs_interpret_key_gen_type(keyGenAlg.ptr());
+  nsDependentJSString dependentKeyGenAlg;
+  NS_ENSURE_TRUE(dependentKeyGenAlg.init(cx, jsString), NS_ERROR_UNEXPECTED);
+  nsAutoString keyGenAlg(dependentKeyGenAlg);
+  keyGenAlg.Trim("\r\n\t ");
+  keyGenType->keyGenType = cryptojs_interpret_key_gen_type(keyGenAlg);
   if (keyGenType->keyGenType == invalidKeyGen) {
+    NS_LossyConvertUTF16toASCII keyGenAlgNarrow(dependentKeyGenAlg);
     JS_ReportError(cx, "%s%s%s", JS_ERROR,
                    "invalid key generation argument:",
-                   keyGenAlg.ptr());
+                   keyGenAlgNarrow.get());
     goto loser;
   }
   if (!*slot) {
     *slot = nsGetSlotForKeyGen(keyGenType->keyGenType, uiCxt);
     if (!*slot)
       goto loser;
   }
 
   rv = cryptojs_generateOneKeyPair(cx,keyGenType,keySize,params.ptr(),uiCxt,
                                    *slot,willEscrow);
 
   if (rv != NS_OK) {
+    NS_LossyConvertUTF16toASCII keyGenAlgNarrow(dependentKeyGenAlg);
     JS_ReportError(cx,"%s%s%s", JS_ERROR,
                    "could not generate the key for algorithm ",
-                   keyGenAlg.ptr());
+                   keyGenAlgNarrow.get());
     goto loser;
   }
   return NS_OK;
 loser:
   return NS_ERROR_FAILURE;
 }
 
 //Utility funciton to free up the memory used by nsKeyPairInfo
--- a/security/manager/ssl/tests/mochitest/bugs/Makefile.in
+++ b/security/manager/ssl/tests/mochitest/bugs/Makefile.in
@@ -14,16 +14,24 @@ include $(DEPTH)/config/autoconf.mk
 MOCHITEST_FILES = \
         test_bug480509.html \
         test_bug483440.html \
         test_bug484111.html \
 	test_ev_validation.html \
 	test_ev_validation_child.html \
         $(NULL)
 
+# test_bug882865.html tests crypto.generateCRMFRequest, which isn't
+# available if legacy crypto has been disabled.
+ifndef MOZ_DISABLE_CRYPTOLEGACY
+MOCHITEST_FILES += \
+        test_bug882865.html \
+        $(NULL)
+endif
+
 MOCHITEST_CHROME_FILES = \
 	test_certificate_overrides.html \
         test_bug413909.html \
         test_bug480619.html \
         test_bug644006.html \
         $(NULL)
 
 include $(topsrcdir)/config/rules.mk
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/mochitest/bugs/test_bug882865.html
@@ -0,0 +1,39 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test bug 882865</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body onload="onWindowLoad()">
+<script class="testbody" type="text/javascript">
+
+  SimpleTest.waitForExplicitFinish();
+
+  function onWindowLoad()
+  {
+    try {
+      var crmfObject = crypto.generateCRMFRequest("CN=undefined", "regToken",
+                                                  "authenticator", null, "",
+                                                  512, null, "  rsa-ex   ",
+                                                  1024, null, "\r\n\t rsa-sign\t");
+      ok(true, "no exception thrown in generateCRMFRequest");
+    } catch (e) {
+      ok(false, "unexpected exception: " + e);
+    }
+
+    var o200 = document.documentElement;
+    var o1 = crypto;
+    try {
+      o1.generateCRMFRequest(o200.writeln, o200, 'X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X X', null, o1, 1404343237, Math.PI, []);
+      ok(false, "execution should not reach this line");
+    } catch (e) {
+      // The 'key generation argument' in this case was an empty array,
+      // which gets interpreted as an empty string.
+      is(e.toString(), "Error: error:invalid key generation argument:", "expected exception");
+    }
+    SimpleTest.finish();
+  }
+</script>
+</body>
+</html>