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 291034 667fa766321fe60527c8888b350268ae117612cd
parent 291033 5ccacd83b165a02fe0813393c7e8cac1cbae791a
child 291035 8dea713c9fcde4a2b19492cb096043240935028f
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1251801
milestone48.0a1
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;