Bug 1251801 - Improve handling of PK11_* function error codes. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Tue, 29 Mar 2016 18:14:29 -0700
changeset 290857 667fa766321fe60527c8888b350268ae117612cd
parent 290856 5ccacd83b165a02fe0813393c7e8cac1cbae791a
child 290858 8dea713c9fcde4a2b19492cb096043240935028f
push id74396
push usercbook@mozilla.com
push dateWed, 30 Mar 2016 06:57:08 +0000
treeherdermozilla-inbound@202e3131cadb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1251801
milestone48.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 1251801 - Improve handling of PK11_* function error codes. r=keeler MozReview-Commit-ID: 18acVVAuapm
security/manager/ssl/nsPK11TokenDB.cpp
security/manager/ssl/nsPK11TokenDB.h
--- a/security/manager/ssl/nsPK11TokenDB.cpp
+++ b/security/manager/ssl/nsPK11TokenDB.cpp
@@ -1,15 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  *
  * 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/. */
 #include "nsPK11TokenDB.h"
 
+#include "mozilla/unused.h"
 #include "nsIMutableArray.h"
 #include "nsISupports.h"
 #include "nsNSSComponent.h"
 #include "nsReadableUtils.h"
 #include "nsServiceManagerUtils.h"
 #include "prerror.h"
 #include "ScopedNSSTypes.h"
 #include "secerr.h"
@@ -23,27 +24,28 @@ nsPK11Token::nsPK11Token(PK11SlotInfo *s
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
 
   mSlot.reset(PK11_ReferenceSlot(slot));
   mSeries = PK11_GetSlotSeries(slot);
 
-  refreshTokenInfo(locker);
+  Unused << refreshTokenInfo(locker);
 }
 
-void
+nsresult
 nsPK11Token::refreshTokenInfo(const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   mTokenName = NS_ConvertUTF8toUTF16(PK11_GetTokenName(mSlot.get()));
 
   CK_TOKEN_INFO tokInfo;
-  if (PK11_GetTokenInfo(mSlot.get(), &tokInfo) != SECSuccess) {
-    return;
+  nsresult rv = MapSECStatus(PK11_GetTokenInfo(mSlot.get(), &tokInfo));
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
   // Set the Label field
   const char* ccLabel = reinterpret_cast<const char*>(tokInfo.label);
   const nsACString& cLabel = Substring(
     ccLabel,
     ccLabel + PL_strnlen(ccLabel, sizeof(tokInfo.label)));
   mTokenLabel = NS_ConvertUTF8toUTF16(cLabel);
@@ -69,16 +71,18 @@ nsPK11Token::refreshTokenInfo(const nsNS
 
   // Set the Serial Number field
   const char* ccSerial = reinterpret_cast<const char*>(tokInfo.serialNumber);
   const nsACString& cSerial = Substring(
     ccSerial,
     ccSerial + PL_strnlen(ccSerial, sizeof(tokInfo.serialNumber)));
   mTokenSerialNum = NS_ConvertUTF8toUTF16(cSerial);
   mTokenSerialNum.Trim(" ", false, true);
+
+  return NS_OK;
 }
 
 nsPK11Token::~nsPK11Token()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return;
   }
@@ -100,98 +104,116 @@ NS_IMETHODIMP nsPK11Token::GetTokenName(
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokenName = ToNewUnicode(mTokenName);
   if (!*aTokenName) return NS_ERROR_OUT_OF_MEMORY;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::GetTokenLabel(char16_t **aTokLabel)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokLabel = ToNewUnicode(mTokenLabel);
   if (!*aTokLabel) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::GetTokenManID(char16_t **aTokManID)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokManID = ToNewUnicode(mTokenManID);
   if (!*aTokManID) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::GetTokenHWVersion(char16_t **aTokHWVersion)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokHWVersion = ToNewUnicode(mTokenHWVersion);
   if (!*aTokHWVersion) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::GetTokenFWVersion(char16_t **aTokFWVersion)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokFWVersion = ToNewUnicode(mTokenFWVersion);
   if (!*aTokFWVersion) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::GetTokenSerialNumber(char16_t **aTokSerialNum)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // handle removals/insertions
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
-    refreshTokenInfo(locker);
+    nsresult rv = refreshTokenInfo(locker);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
   *aTokSerialNum = ToNewUnicode(mTokenSerialNum);
   if (!*aTokSerialNum) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::IsLoggedIn(bool *_retval)
 {
@@ -207,39 +229,38 @@ NS_IMETHODIMP nsPK11Token::IsLoggedIn(bo
 NS_IMETHODIMP
 nsPK11Token::Login(bool force)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   nsresult rv;
-  SECStatus srv;
   bool test;
   rv = this->NeedsLogin(&test);
   if (NS_FAILED(rv)) return rv;
   if (test && force) {
     rv = this->LogoutSimple();
     if (NS_FAILED(rv)) return rv;
   }
   rv = setPassword(mSlot.get(), mUIContext, locker);
   if (NS_FAILED(rv)) return rv;
-  srv = PK11_Authenticate(mSlot.get(), true, mUIContext);
-  return (srv == SECSuccess) ? NS_OK : NS_ERROR_FAILURE;
+
+  return MapSECStatus(PK11_Authenticate(mSlot.get(), true, mUIContext));
 }
 
 NS_IMETHODIMP nsPK11Token::LogoutSimple()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
-  // PK11_MapError sets CKR_USER_NOT_LOGGED_IN to SEC_ERROR_LIBRARY_FAILURE,
-  // so not going to learn anything here by a failure.  Treat it like void.
-  PK11_Logout(mSlot.get());
+  // PK11_Logout() can fail if the user wasn't logged in beforehand. We want
+  // this method to succeed even in this case, so we ignore the return value.
+  Unused << PK11_Logout(mSlot.get());
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::LogoutAndDropAuthenticatedResources()
 {
   static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
 
   nsresult rv = LogoutSimple();
@@ -255,18 +276,17 @@ NS_IMETHODIMP nsPK11Token::LogoutAndDrop
 }
 
 NS_IMETHODIMP nsPK11Token::Reset()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
-  PK11_ResetToken(mSlot.get(), 0);
-  return NS_OK;
+  return MapSECStatus(PK11_ResetToken(mSlot.get(), nullptr));
 }
 
 NS_IMETHODIMP nsPK11Token::GetMinimumPasswordLength(int32_t *aMinimumPasswordLength)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
@@ -291,39 +311,36 @@ NS_IMETHODIMP nsPK11Token::CheckPassword
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   NS_ConvertUTF16toUTF8 utf8Password(password);
   SECStatus srv =
     PK11_CheckUserPassword(mSlot.get(), const_cast<char*>(utf8Password.get()));
   if (srv != SECSuccess) {
     *_retval =  false;
-    if (PR_GetError() != SEC_ERROR_BAD_PASSWORD) {
+    PRErrorCode error = PR_GetError();
+    if (error != SEC_ERROR_BAD_PASSWORD) {
       /* something really bad happened - throw an exception */
-      return NS_ERROR_FAILURE;
+      return mozilla::psm::GetXPCOMFromNSSError(error);
     }
   } else {
     *_retval =  true;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPK11Token::InitPassword(const char16_t *initialPassword)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   NS_ConvertUTF16toUTF8 utf8Password(initialPassword);
-  if (PK11_InitPin(mSlot.get(), "", const_cast<char*>(utf8Password.get()))
-        != SECSuccess) {
-    return NS_ERROR_FAILURE;
-  }
-
-  return NS_OK;
+  return MapSECStatus(
+    PK11_InitPin(mSlot.get(), "", const_cast<char*>(utf8Password.get())));
 }
 
 NS_IMETHODIMP
 nsPK11Token::GetAskPasswordTimes(int32_t* askTimes)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
@@ -365,20 +382,20 @@ NS_IMETHODIMP nsPK11Token::ChangePasswor
 
   NS_ConvertUTF16toUTF8 utf8OldPassword(oldPassword);
   NS_ConvertUTF16toUTF8 utf8NewPassword(newPassword);
 
   // nsCString.get() will return an empty string instead of nullptr even if it
   // was initialized with nullptr. PK11_ChangePW() has different semantics for
   // the empty string and for nullptr, so we can't just use get().
   // See Bug 447589.
-  SECStatus srv = PK11_ChangePW(mSlot.get(),
+  return MapSECStatus(PK11_ChangePW(
+    mSlot.get(),
     (oldPassword ? const_cast<char*>(utf8OldPassword.get()) : nullptr),
-    (newPassword ? const_cast<char*>(utf8NewPassword.get()) : nullptr));
-  return (srv == SECSuccess) ? NS_OK : NS_ERROR_FAILURE;
+    (newPassword ? const_cast<char*>(utf8NewPassword.get()) : nullptr)));
 }
 
 NS_IMETHODIMP nsPK11Token::IsHardwareToken(bool *_retval)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
--- a/security/manager/ssl/nsPK11TokenDB.h
+++ b/security/manager/ssl/nsPK11TokenDB.h
@@ -26,17 +26,17 @@ public:
 
   explicit nsPK11Token(PK11SlotInfo *slot);
 
 protected:
   virtual ~nsPK11Token();
 
 private:
   friend class nsPK11TokenDB;
-  void refreshTokenInfo(const nsNSSShutDownPreventionLock& proofOfLock);
+  nsresult refreshTokenInfo(const nsNSSShutDownPreventionLock& proofOfLock);
 
   nsString mTokenName;
   nsString mTokenLabel, mTokenManID, mTokenHWVersion, mTokenFWVersion;
   nsString mTokenSerialNum;
   mozilla::UniquePK11SlotInfo mSlot;
   int mSeries;
   nsCOMPtr<nsIInterfaceRequestor> mUIContext;
   virtual void virtualDestroyNSSReference() override;