Bug 1250256 - Partially clean up nsSDR.cpp. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Tue, 01 Mar 2016 20:07:53 -0800
changeset 324686 a19efafe26aceb4feef4ec6e57757d77f868b690
parent 324685 4c2bb3fc363d90404e89724f8426d8f9eb0854f0
child 324687 f859932803e86f449793e3f99d905cf43bac0771
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1250256
milestone47.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 1250256 - Partially clean up nsSDR.cpp. r=keeler MozReview-Commit-ID: FoS4oTjnd7F
security/manager/ssl/ScopedNSSTypes.h
security/manager/ssl/nsSDR.cpp
security/manager/ssl/tests/unit/head_psm.js
security/manager/ssl/tests/unit/test_cert_blocklist.js
security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
security/manager/ssl/tests/unit/test_sdr.js
security/manager/ssl/tests/unit/xpcshell.ini
--- a/security/manager/ssl/ScopedNSSTypes.h
+++ b/security/manager/ssl/ScopedNSSTypes.h
@@ -336,16 +336,19 @@ MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(Un
 
 MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueNSSCMSMessage,
                                       NSSCMSMessage,
                                       NSS_CMSMessage_Destroy)
 MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueNSSCMSSignedData,
                                       NSSCMSSignedData,
                                       NSS_CMSSignedData_Destroy)
 
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniquePK11SlotInfo,
+                                      PK11SlotInfo,
+                                      PK11_FreeSlot)
 MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniquePK11SlotList,
                                       PK11SlotList,
                                       PK11_FreeSlotList)
 
 MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniquePLArenaPool,
                                       PLArenaPool,
                                       internal::PORT_FreeArena_false)
 
--- a/security/manager/ssl/nsSDR.cpp
+++ b/security/manager/ssl/nsSDR.cpp
@@ -57,96 +57,98 @@ NS_IMETHODIMP
 nsSecretDecoderRing::Encrypt(unsigned char* data, int32_t dataLen,
                              unsigned char** result, int32_t* _retval)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  nsresult rv = NS_OK;
-  ScopedPK11SlotInfo slot;
-  SECItem keyid;
-  SECItem request;
-  SECItem reply;
-  SECStatus s;
   nsCOMPtr<nsIInterfaceRequestor> ctx = new PipUIContext();
 
-  slot = PK11_GetInternalKeySlot();
-  if (!slot) { rv = NS_ERROR_NOT_AVAILABLE; goto loser; }
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  if (!slot) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
   /* Make sure token is initialized. */
-  rv = setPassword(slot, ctx, locker);
-  if (NS_FAILED(rv))
-    goto loser;
+  nsresult rv = setPassword(slot.get(), ctx, locker);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   /* Force authentication */
-  s = PK11_Authenticate(slot, true, ctx);
-  if (s != SECSuccess) { rv = NS_ERROR_FAILURE; goto loser; }
+  if (PK11_Authenticate(slot.get(), true, ctx) != SECSuccess) {
+    return NS_ERROR_FAILURE;
+  }
 
   /* Use default key id */
-  keyid.data = 0;
+  SECItem keyid;
+  keyid.data = nullptr;
   keyid.len = 0;
+  SECItem request;
   request.data = data;
   request.len = dataLen;
-  reply.data = 0;
+  SECItem reply;
+  reply.data = nullptr;
   reply.len = 0;
-  s= PK11SDR_Encrypt(&keyid, &request, &reply, ctx);
-  if (s != SECSuccess) { rv = NS_ERROR_FAILURE; goto loser; }
+  if (PK11SDR_Encrypt(&keyid, &request, &reply, ctx) != SECSuccess) {
+    return NS_ERROR_FAILURE;
+  }
 
   *result = reply.data;
   *_retval = reply.len;
 
-loser:
-  return rv;
+  return NS_OK;
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-Decrypt(unsigned char * data, int32_t dataLen, unsigned char * *result, int32_t *_retval)
+NS_IMETHODIMP
+nsSecretDecoderRing::Decrypt(unsigned char* data, int32_t dataLen,
+                             unsigned char** result, int32_t* _retval)
 {
   nsNSSShutDownPreventionLock locker;
-  nsresult rv = NS_OK;
-  ScopedPK11SlotInfo slot;
-  SECStatus s;
-  SECItem request;
-  SECItem reply;
+  if (isAlreadyShutDown()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   nsCOMPtr<nsIInterfaceRequestor> ctx = new PipUIContext();
 
-  *result = 0;
+  *result = nullptr;
   *_retval = 0;
 
   /* Find token with SDR key */
-  slot = PK11_GetInternalKeySlot();
-  if (!slot) { rv = NS_ERROR_NOT_AVAILABLE; goto loser; }
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  if (!slot) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
   /* Force authentication */
-  if (PK11_Authenticate(slot, true, ctx) != SECSuccess)
-  {
-    rv = NS_ERROR_NOT_AVAILABLE;
-    goto loser;
+  if (PK11_Authenticate(slot.get(), true, ctx) != SECSuccess) {
+    return NS_ERROR_NOT_AVAILABLE;
   }
 
+  SECItem request;
   request.data = data;
   request.len = dataLen;
-  reply.data = 0;
+  SECItem reply;
+  reply.data = nullptr;
   reply.len = 0;
-  s = PK11SDR_Decrypt(&request, &reply, ctx);
-  if (s != SECSuccess) { rv = NS_ERROR_FAILURE; goto loser; }
+  if (PK11SDR_Decrypt(&request, &reply, ctx) != SECSuccess) {
+    return NS_ERROR_FAILURE;
+  }
 
   *result = reply.data;
   *_retval = reply.len;
 
-loser:
-  return rv;
+  return NS_OK;
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-EncryptString(const char *text, char **_retval)
+NS_IMETHODIMP
+nsSecretDecoderRing::EncryptString(const char* text, char** _retval)
 {
-  nsNSSShutDownPreventionLock locker;
   nsresult rv = NS_OK;
   unsigned char *encrypted = 0;
   int32_t eLen;
 
   if (!text || !_retval) {
     rv = NS_ERROR_INVALID_POINTER;
     goto loser;
   }
@@ -157,20 +159,19 @@ EncryptString(const char *text, char **_
   rv = encode(encrypted, eLen, _retval);
 
 loser:
   if (encrypted) PORT_Free(encrypted);
 
   return rv;
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-DecryptString(const char *crypt, char **_retval)
+NS_IMETHODIMP
+nsSecretDecoderRing::DecryptString(const char* crypt, char** _retval)
 {
-  nsNSSShutDownPreventionLock locker;
   nsresult rv = NS_OK;
   char *r = 0;
   unsigned char *decoded = 0;
   int32_t decodedLen;
   unsigned char *decrypted = 0;
   int32_t decryptedLen;
 
   if (!crypt || !_retval) {
@@ -204,123 +205,133 @@ loser:
 NS_IMETHODIMP
 nsSecretDecoderRing::ChangePassword()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  nsresult rv;
-  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
-  if (!slot) return NS_ERROR_NOT_AVAILABLE;
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  if (!slot) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
-  /* Convert UTF8 token name to UCS2 */
-  NS_ConvertUTF8toUTF16 tokenName(PK11_GetTokenName(slot));
+  NS_ConvertUTF8toUTF16 tokenName(PK11_GetTokenName(slot.get()));
 
-  /* Get the set password dialog handler imlementation */
   nsCOMPtr<nsITokenPasswordDialogs> dialogs;
-
-  rv = getNSSDialogs(getter_AddRefs(dialogs),
-                     NS_GET_IID(nsITokenPasswordDialogs),
-                     NS_TOKENPASSWORDSDIALOG_CONTRACTID);
-  if (NS_FAILED(rv)) return rv;
+  nsresult rv = getNSSDialogs(getter_AddRefs(dialogs),
+                              NS_GET_IID(nsITokenPasswordDialogs),
+                              NS_TOKENPASSWORDSDIALOG_CONTRACTID);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   nsCOMPtr<nsIInterfaceRequestor> ctx = new PipUIContext();
-  bool canceled;
-  rv = dialogs->SetPassword(ctx, tokenName.get(), &canceled);
-  /* canceled is ignored */
-
-  return rv;
+  bool canceled; // Ignored
+  return dialogs->SetPassword(ctx, tokenName.get(), &canceled);
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-Logout()
+NS_IMETHODIMP
+nsSecretDecoderRing::Logout()
 {
   static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
 
   nsresult rv;
   nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
   if (NS_FAILED(rv))
     return rv;
 
   {
     nsNSSShutDownPreventionLock locker;
+    if (isAlreadyShutDown()) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+
     PK11_LogoutAll();
     SSL_ClearSessionCache();
   }
 
   return NS_OK;
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-LogoutAndTeardown()
+NS_IMETHODIMP
+nsSecretDecoderRing::LogoutAndTeardown()
 {
   static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
 
   nsresult rv;
   nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
   if (NS_FAILED(rv))
     return rv;
 
   {
     nsNSSShutDownPreventionLock locker;
+    if (isAlreadyShutDown()) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+
     PK11_LogoutAll();
     SSL_ClearSessionCache();
   }
 
   rv = nssComponent->LogoutAuthenticatedPK11();
 
   // After we just logged out, we need to prune dead connections to make
   // sure that all connections that should be stopped, are stopped. See
   // bug 517584.
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os)
     os->NotifyObservers(nullptr, "net:prune-dead-connections", nullptr);
 
   return rv;
 }
 
-NS_IMETHODIMP nsSecretDecoderRing::
-SetWindow(nsISupports *w)
+NS_IMETHODIMP
+nsSecretDecoderRing::SetWindow(nsISupports*)
 {
-  return NS_OK;
+  return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 // Support routines
 
-nsresult nsSecretDecoderRing::
-encode(const unsigned char *data, int32_t dataLen, char **_retval)
+nsresult
+nsSecretDecoderRing::encode(const unsigned char* data, int32_t dataLen,
+                            char** _retval)
 {
-  nsresult rv = NS_OK;
-
-  char *result = PL_Base64Encode((const char *)data, dataLen, nullptr);
-  if (!result) { rv = NS_ERROR_OUT_OF_MEMORY; goto loser; }
+  char* result = PL_Base64Encode((const char*)data, dataLen, nullptr);
+  if (!result) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
   *_retval = NS_strdup(result);
   PR_DELETE(result);
-  if (!*_retval) { rv = NS_ERROR_OUT_OF_MEMORY; goto loser; }
+  if (!*_retval) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-loser:
-  return rv;
+  return NS_OK;
 }
 
-nsresult nsSecretDecoderRing::
-decode(const char *data, unsigned char **result, int32_t * _retval)
+nsresult
+nsSecretDecoderRing::decode(const char* data, unsigned char** result,
+                            int32_t* _retval)
 {
-  nsresult rv = NS_OK;
   uint32_t len = strlen(data);
   int adjust = 0;
 
   /* Compute length adjustment */
   if (data[len-1] == '=') {
     adjust++;
-    if (data[len-2] == '=') adjust++;
+    if (data[len - 2] == '=') {
+      adjust++;
+    }
   }
 
   *result = (unsigned char *)PL_Base64Decode(data, len, nullptr);
-  if (!*result) { rv = NS_ERROR_ILLEGAL_VALUE; goto loser; }
+  if (!*result) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
 
   *_retval = (len*3)/4 - adjust;
 
-loser:
-  return rv;
+  return NS_OK;
 }
--- a/security/manager/ssl/tests/unit/head_psm.js
+++ b/security/manager/ssl/tests/unit/head_psm.js
@@ -1,22 +1,25 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/.
  */
 "use strict";
 
-var { 'classes': Cc, 'interfaces': Ci, 'utils': Cu, 'results': Cr } = Components;
+const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 
-var { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
-var { FileUtils } = Cu.import("resource://gre/modules/FileUtils.jsm", {});
-var { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
-var { Promise } = Cu.import("resource://gre/modules/Promise.jsm", {});
-var { HttpServer } = Cu.import("resource://testing-common/httpd.js", {});
-var { ctypes } = Cu.import("resource://gre/modules/ctypes.jsm");
+const { ctypes } = Cu.import("resource://gre/modules/ctypes.jsm", {});
+const { FileUtils } = Cu.import("resource://gre/modules/FileUtils.jsm", {});
+const { HttpServer } = Cu.import("resource://testing-common/httpd.js", {});
+const { MockRegistrar } =
+  Cu.import("resource://testing-common/MockRegistrar.jsm", {});
+const { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
+const { Promise } = Cu.import("resource://gre/modules/Promise.jsm", {});
+const { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
+const { XPCOMUtils } = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
 
 const isDebugBuild = Cc["@mozilla.org/xpcom/debug;1"]
                        .getService(Ci.nsIDebug2).isDebugBuild;
 
 // The test EV roots are only enabled in debug builds as a security measure.
 //
 // Bug 1008316: B2G doesn't have EV enabled, so EV is not expected even in debug
 // builds.
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -6,18 +6,16 @@
 // This test checks a number of things:
 // * it ensures that data loaded from revocations.txt on startup is present
 // * it ensures that certItems in blocklist.xml are persisted correctly
 // * it ensures that items in the CertBlocklist are seen as revoked by the
 //   cert verifier
 // * it does a sanity check to ensure other cert verifier behavior is
 //   unmodified
 
-var { XPCOMUtils } = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
-
 // First, we need to setup appInfo for the blocklist service to work
 var id = "xpcshell@tests.mozilla.org";
 var appName = "XPCShell";
 var version = "1";
 var platformVersion = "1.9.2";
 var appInfo = {
   // nsIXULAppInfo
   vendor: "Mozilla",
--- a/security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
+++ b/security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
@@ -2,18 +2,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 "use strict";
 
 // In safe mode, PKCS#11 modules should not be loaded. This test tests this by
 // simulating starting in safe mode and then attempting to load a module.
 
-var { XPCOMUtils } = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
-
 function run_test() {
   do_get_profile();
 
   // Simulate starting in safe mode.
   let xulRuntime = {
     inSafeMode: true,
     logConsoleErrors: true,
     OS: "XPCShell",
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_sdr.js
@@ -0,0 +1,84 @@
+// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/publicdomain/zero/1.0/
+"use strict";
+
+// Tests various aspects of the nsISecretDecoderRing implementation.
+
+const { AppConstants } =
+  Cu.import("resource://gre/modules/AppConstants.jsm", {});
+
+do_get_profile();
+
+let gSetPasswordShownCount = 0;
+
+// Mock implementation of nsITokenPasswordDialogs.
+const gTokenPasswordDialogs = {
+  setPassword: (ctx, tokenName, canceled) => {
+    gSetPasswordShownCount++;
+    do_print(`setPassword() called; shown ${gSetPasswordShownCount} times`);
+    do_print(`tokenName: ${tokenName}`);
+    canceled.value = false;
+  },
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsITokenPasswordDialogs])
+};
+
+function run_test() {
+  // We have to set a password and login before we attempt to encrypt anything.
+  // In particular, failing to do so will cause the Encrypt() implementation to
+  // pop up a dialog asking for a password to be set. This won't work in the
+  // xpcshell environment and will lead to an assertion.
+  let tokenDB = Cc["@mozilla.org/security/pk11tokendb;1"]
+                  .getService(Ci.nsIPK11TokenDB);
+  let token = tokenDB.getInternalKeyToken();
+  token.initPassword("");
+  token.login(/*force*/ false);
+
+  let sdr = Cc["@mozilla.org/security/sdr;1"]
+              .getService(Ci.nsISecretDecoderRing);
+
+  // Test valid inputs for encryptString() and decryptString().
+  let inputs = [
+    "",
+    "foo",
+    "1234567890`~!#$%^&*()-_=+{[}]|\\:;'\",<.>/?",
+  ];
+  for (let input of inputs) {
+    let encrypted = sdr.encryptString(input);
+
+    notEqual(input, encrypted,
+             "Encypted input should not just be the input itself");
+
+    try {
+      atob(encrypted);
+    } catch (e) {
+      ok(false, `encryptString() should have returned Base64: ${e}`);
+    }
+
+    equal(input, sdr.decryptString(encrypted),
+          "decryptString(encryptString(input)) should return input");
+  }
+
+  // Test invalid inputs for decryptString().
+  throws(() => sdr.decryptString("*"), /NS_ERROR_ILLEGAL_VALUE/,
+         "decryptString() should throw if given non-Base64 input");
+
+  // Test calling changePassword() pops up the appropriate dialog.
+  // Note: On Android, nsITokenPasswordDialogs is apparently not implemented,
+  //       which also seems to prevent us from mocking out the interface.
+  if (AppConstants.platform != "android") {
+    let tokenPasswordDialogsCID =
+      MockRegistrar.register("@mozilla.org/nsTokenPasswordDialogs;1",
+                             gTokenPasswordDialogs);
+    do_register_cleanup(() => {
+      MockRegistrar.unregister(tokenPasswordDialogsCID);
+    });
+
+    equal(gSetPasswordShownCount, 0,
+          "changePassword() dialog should have been shown zero times");
+    sdr.changePassword();
+    equal(gSetPasswordShownCount, 1,
+          "changePassword() dialog should have been shown exactly once");
+  }
+}
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -102,16 +102,17 @@ skip-if = toolkit == 'gonk'
 run-sequentially = hardcoded ports
 [test_pinning.js]
 run-sequentially = hardcoded ports
 # This test can take longer than 300 seconds on B2G emulator debug builds, so
 # give it enough time to finish. See bug 1081128.
 requesttimeoutfactor = 2
 [test_pinning_dynamic.js]
 [test_pinning_header_parsing.js]
+[test_sdr.js]
 [test_signed_apps.js]
 [test_signed_apps-marketplace.js]
 [test_signed_dir.js]
 tags = addons psm
 [test_sss_eviction.js]
 [test_sss_readstate.js]
 [test_sss_readstate_child.js]
 support-files = sss_readstate_child_worker.js