From 8ca090b1dc0125cce02f304f113d5dc8453b3be6 Mon Sep 17 00:00:00 2001 From: Cykesiopka Date: Sat, 1 Oct 2016 00:46:13 +0800 Subject: [PATCH] Bug 1304587 - Avoid using types that correspond to char/char16_t strings in PKCS #11 IDL files. r=keeler Typically, the interfaces involved don't need to use raw char/char16_t strings, and hence can benefit from the additional safety of using the Mozilla string classes. In some places, this patch also changes some UTF-16 APIs to UTF-8 where the implementations can never actually support UTF-16. This reduces the amount of code and runtime conversion. MozReview-Commit-ID: y8o5wLBohe --- security/manager/ssl/nsIPK11Token.idl | 35 +++- security/manager/ssl/nsIPK11TokenDB.idl | 2 +- security/manager/ssl/nsIPKCS11Module.idl | 6 +- security/manager/ssl/nsIPKCS11ModuleDB.idl | 4 +- security/manager/ssl/nsIPKCS11Slot.idl | 24 ++- security/manager/ssl/nsPK11TokenDB.cpp | 180 +++++------------ security/manager/ssl/nsPK11TokenDB.h | 11 +- security/manager/ssl/nsPKCS11Slot.cpp | 184 +++++++----------- security/manager/ssl/nsPKCS11Slot.h | 7 +- security/manager/ssl/nsPKCS12Blob.cpp | 10 +- .../ssl/tests/unit/test_pkcs11_slot.js | 3 + .../ssl/tests/unit/test_pkcs11_token.js | 4 +- 12 files changed, 182 insertions(+), 288 deletions(-) diff --git a/security/manager/ssl/nsIPK11Token.idl b/security/manager/ssl/nsIPK11Token.idl index ae4e287379197..1d53ce19a2da8 100644 --- a/security/manager/ssl/nsIPK11Token.idl +++ b/security/manager/ssl/nsIPK11Token.idl @@ -16,13 +16,21 @@ interface nsIPK11Token : nsISupports /* * The name of the token */ - readonly attribute wstring tokenName; - - readonly attribute wstring tokenLabel; - readonly attribute wstring tokenManID; - readonly attribute wstring tokenHWVersion; - readonly attribute wstring tokenFWVersion; - readonly attribute wstring tokenSerialNumber; + readonly attribute AUTF8String tokenName; + readonly attribute AUTF8String tokenLabel; + /** + * Manufacturer ID of the token. + */ + readonly attribute AUTF8String tokenManID; + /** + * Hardware version of the token. + */ + readonly attribute AUTF8String tokenHWVersion; + /** + * Firmware version of the token. + */ + readonly attribute AUTF8String tokenFWVersion; + readonly attribute AUTF8String tokenSerialNumber; /* * Login information @@ -42,9 +50,16 @@ interface nsIPK11Token : nsISupports */ readonly attribute long minimumPasswordLength; readonly attribute boolean needsUserInit; - boolean checkPassword(in wstring password); /* Logs out if check fails */ - void initPassword(in wstring initialPassword); - void changePassword(in wstring oldPassword, in wstring newPassword); + /** + * Checks whether the given password is correct. Logs the token out if an + * incorrect password is given. + * + * @param password The password to check. + * @return true if the password was correct, false otherwise. + */ + boolean checkPassword(in AUTF8String password); + void initPassword(in AUTF8String initialPassword); + void changePassword(in AUTF8String oldPassword, in AUTF8String newPassword); long getAskPasswordTimes(); long getAskPasswordTimeout(); void setAskPasswordDefaults([const] in long askTimes, [const] in long timeout); diff --git a/security/manager/ssl/nsIPK11TokenDB.idl b/security/manager/ssl/nsIPK11TokenDB.idl index b10b875663e42..297f3da11a251 100644 --- a/security/manager/ssl/nsIPK11TokenDB.idl +++ b/security/manager/ssl/nsIPK11TokenDB.idl @@ -33,7 +33,7 @@ interface nsIPK11TokenDB : nsISupports /* * Find a token by name */ - nsIPK11Token findTokenByName(in wstring tokenName); + nsIPK11Token findTokenByName(in AUTF8String tokenName); /* * List all tokens diff --git a/security/manager/ssl/nsIPKCS11Module.idl b/security/manager/ssl/nsIPKCS11Module.idl index 1b81037064a60..8a15918784dea 100644 --- a/security/manager/ssl/nsIPKCS11Module.idl +++ b/security/manager/ssl/nsIPKCS11Module.idl @@ -12,10 +12,10 @@ interface nsISimpleEnumerator; [scriptable, uuid(8a44bdf9-d1a5-4734-bd5a-34ed7fe564c2)] interface nsIPKCS11Module : nsISupports { - readonly attribute wstring name; - readonly attribute wstring libName; + readonly attribute AUTF8String name; + readonly attribute AUTF8String libName; - nsIPKCS11Slot findSlotByName(in wstring name); + nsIPKCS11Slot findSlotByName(in AUTF8String name); nsISimpleEnumerator listSlots(); }; diff --git a/security/manager/ssl/nsIPKCS11ModuleDB.idl b/security/manager/ssl/nsIPKCS11ModuleDB.idl index 8b9a6746ed76c..6c0749de57f9d 100644 --- a/security/manager/ssl/nsIPKCS11ModuleDB.idl +++ b/security/manager/ssl/nsIPKCS11ModuleDB.idl @@ -21,9 +21,9 @@ interface nsIPKCS11ModuleDB : nsISupports nsIPKCS11Module getInternalFIPS(); - nsIPKCS11Module findModuleByName(in wstring name); + nsIPKCS11Module findModuleByName(in AUTF8String name); - nsIPKCS11Slot findSlotByName(in wstring name); + nsIPKCS11Slot findSlotByName(in AUTF8String name); nsISimpleEnumerator listModules(); diff --git a/security/manager/ssl/nsIPKCS11Slot.idl b/security/manager/ssl/nsIPKCS11Slot.idl index a4af5775c7f5d..7c41e4fcc3a33 100644 --- a/security/manager/ssl/nsIPKCS11Slot.idl +++ b/security/manager/ssl/nsIPKCS11Slot.idl @@ -10,12 +10,20 @@ interface nsIPK11Token; [scriptable, uuid(c2d4f296-ee60-11d4-998b-00b0d02354a0)] interface nsIPKCS11Slot : nsISupports { - - readonly attribute wstring name; - readonly attribute wstring desc; - readonly attribute wstring manID; - readonly attribute wstring HWVersion; - readonly attribute wstring FWVersion; + readonly attribute AUTF8String name; + readonly attribute AUTF8String desc; + /** + * Manufacturer ID of the slot. + */ + readonly attribute AUTF8String manID; + /** + * Hardware version of the slot. + */ + readonly attribute AUTF8String HWVersion; + /** + * Firmware version of the slot. + */ + readonly attribute AUTF8String FWVersion; const unsigned long SLOT_DISABLED = 0; const unsigned long SLOT_NOT_PRESENT = 1; @@ -32,7 +40,5 @@ interface nsIPKCS11Slot : nsISupports { nsIPK11Token getToken(); /* more fun with workarounds - we're referring to everything by token name */ - readonly attribute wstring tokenName; - + readonly attribute AUTF8String tokenName; }; - diff --git a/security/manager/ssl/nsPK11TokenDB.cpp b/security/manager/ssl/nsPK11TokenDB.cpp index 4037a1fddbee3..d2f6918571895 100644 --- a/security/manager/ssl/nsPK11TokenDB.cpp +++ b/security/manager/ssl/nsPK11TokenDB.cpp @@ -5,15 +5,16 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nsPK11TokenDB.h" +#include "ScopedNSSTypes.h" #include "mozilla/Casting.h" #include "mozilla/Unused.h" #include "nsIMutableArray.h" #include "nsISupports.h" #include "nsNSSComponent.h" +#include "nsPromiseFlatString.h" #include "nsReadableUtils.h" #include "nsServiceManagerUtils.h" #include "prerror.h" -#include "ScopedNSSTypes.h" #include "secerr.h" extern mozilla::LazyLogModule gPIPNSSLog; @@ -38,7 +39,7 @@ nsPK11Token::nsPK11Token(PK11SlotInfo* slot) nsresult nsPK11Token::refreshTokenInfo(const nsNSSShutDownPreventionLock& /*proofOfLock*/) { - mTokenName = NS_ConvertUTF8toUTF16(PK11_GetTokenName(mSlot.get())); + mTokenName = PK11_GetTokenName(mSlot.get()); CK_TOKEN_INFO tokInfo; nsresult rv = MapSECStatus(PK11_GetTokenInfo(mSlot.get(), &tokInfo)); @@ -48,27 +49,27 @@ nsPK11Token::refreshTokenInfo(const nsNSSShutDownPreventionLock& /*proofOfLock*/ // Set the Label field const char* ccLabel = mozilla::BitwiseCast(tokInfo.label); - const nsACString& cLabel = Substring( - ccLabel, - ccLabel + PL_strnlen(ccLabel, sizeof(tokInfo.label))); - mTokenLabel = NS_ConvertUTF8toUTF16(cLabel); + // TODO(Bug 1305930): Stop using PL_strnlen() if/when all our supported + // platforms provide strnlen(). + mTokenLabel.Assign(ccLabel, PL_strnlen(ccLabel, sizeof(tokInfo.label))); mTokenLabel.Trim(" ", false, true); // Set the Manufacturer field const char* ccManID = mozilla::BitwiseCast(tokInfo.manufacturerID); - const nsACString& cManID = Substring( + mTokenManufacturerID.Assign( ccManID, - ccManID + PL_strnlen(ccManID, sizeof(tokInfo.manufacturerID))); - mTokenManID = NS_ConvertUTF8toUTF16(cManID); - mTokenManID.Trim(" ", false, true); + PL_strnlen(ccManID, sizeof(tokInfo.manufacturerID))); + mTokenManufacturerID.Trim(" ", false, true); // Set the Hardware Version field + mTokenHWVersion.Truncate(); mTokenHWVersion.AppendInt(tokInfo.hardwareVersion.major); mTokenHWVersion.Append('.'); mTokenHWVersion.AppendInt(tokInfo.hardwareVersion.minor); // Set the Firmware Version field + mTokenFWVersion.Truncate(); mTokenFWVersion.AppendInt(tokInfo.firmwareVersion.major); mTokenFWVersion.Append('.'); mTokenFWVersion.AppendInt(tokInfo.firmwareVersion.minor); @@ -76,10 +77,8 @@ nsPK11Token::refreshTokenInfo(const nsNSSShutDownPreventionLock& /*proofOfLock*/ // Set the Serial Number field const char* ccSerial = mozilla::BitwiseCast(tokInfo.serialNumber); - const nsACString& cSerial = Substring( - ccSerial, - ccSerial + PL_strnlen(ccSerial, sizeof(tokInfo.serialNumber))); - mTokenSerialNum = NS_ConvertUTF8toUTF16(cSerial); + mTokenSerialNum.Assign(ccSerial, + PL_strnlen(ccSerial, sizeof(tokInfo.serialNumber))); mTokenSerialNum.Trim(" ", false, true); return NS_OK; @@ -107,137 +106,61 @@ nsPK11Token::destructorSafeDestroyNSSReference() mSlot = nullptr; } -NS_IMETHODIMP -nsPK11Token::GetTokenName(char16_t** aTokenName) +nsresult +nsPK11Token::GetAttributeHelper(const nsACString& attribute, + /*out*/ nsACString& xpcomOutParam) { - NS_ENSURE_ARG_POINTER(aTokenName); - nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) { return NS_ERROR_NOT_AVAILABLE; } - // handle removals/insertions + // Handle removals/insertions. if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { nsresult rv = refreshTokenInfo(locker); if (NS_FAILED(rv)) { return rv; } } - *aTokenName = ToNewUnicode(mTokenName); - if (!*aTokenName) return NS_ERROR_OUT_OF_MEMORY; + xpcomOutParam = attribute; return NS_OK; } NS_IMETHODIMP -nsPK11Token::GetTokenLabel(char16_t** aTokLabel) +nsPK11Token::GetTokenName(/*out*/ nsACString& tokenName) { - NS_ENSURE_ARG_POINTER(aTokLabel); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } - - // handle removals/insertions - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshTokenInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aTokLabel = ToNewUnicode(mTokenLabel); - if (!*aTokLabel) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; + return GetAttributeHelper(mTokenName, tokenName); } NS_IMETHODIMP -nsPK11Token::GetTokenManID(char16_t** aTokManID) +nsPK11Token::GetTokenLabel(/*out*/ nsACString& tokenLabel) { - NS_ENSURE_ARG_POINTER(aTokManID); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } - - // handle removals/insertions - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshTokenInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aTokManID = ToNewUnicode(mTokenManID); - if (!*aTokManID) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; + return GetAttributeHelper(mTokenLabel, tokenLabel); } NS_IMETHODIMP -nsPK11Token::GetTokenHWVersion(char16_t** aTokHWVersion) +nsPK11Token::GetTokenManID(/*out*/ nsACString& tokenManufacturerID) { - NS_ENSURE_ARG_POINTER(aTokHWVersion); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } - - // handle removals/insertions - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshTokenInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aTokHWVersion = ToNewUnicode(mTokenHWVersion); - if (!*aTokHWVersion) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; + return GetAttributeHelper(mTokenManufacturerID, tokenManufacturerID); } NS_IMETHODIMP -nsPK11Token::GetTokenFWVersion(char16_t** aTokFWVersion) +nsPK11Token::GetTokenHWVersion(/*out*/ nsACString& tokenHWVersion) { - NS_ENSURE_ARG_POINTER(aTokFWVersion); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } - - // handle removals/insertions - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshTokenInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aTokFWVersion = ToNewUnicode(mTokenFWVersion); - if (!*aTokFWVersion) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; + return GetAttributeHelper(mTokenHWVersion, tokenHWVersion); } NS_IMETHODIMP -nsPK11Token::GetTokenSerialNumber(char16_t** aTokSerialNum) +nsPK11Token::GetTokenFWVersion(/*out*/ nsACString& tokenFWVersion) { - NS_ENSURE_ARG_POINTER(aTokSerialNum); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } + return GetAttributeHelper(mTokenFWVersion, tokenFWVersion); +} - // handle removals/insertions - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - 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::GetTokenSerialNumber(/*out*/ nsACString& tokenSerialNum) +{ + return GetAttributeHelper(mTokenSerialNum, tokenSerialNum); } NS_IMETHODIMP @@ -343,18 +266,16 @@ nsPK11Token::GetNeedsUserInit(bool* aNeedsUserInit) } NS_IMETHODIMP -nsPK11Token::CheckPassword(const char16_t* password, bool* _retval) +nsPK11Token::CheckPassword(const nsACString& password, bool* _retval) { - // Note: It's OK for |password| to be null. NS_ENSURE_ARG_POINTER(_retval); nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - NS_ConvertUTF16toUTF8 utf8Password(password); SECStatus srv = - PK11_CheckUserPassword(mSlot.get(), const_cast(utf8Password.get())); + PK11_CheckUserPassword(mSlot.get(), PromiseFlatCString(password).get()); if (srv != SECSuccess) { *_retval = false; PRErrorCode error = PR_GetError(); @@ -369,16 +290,14 @@ nsPK11Token::CheckPassword(const char16_t* password, bool* _retval) } NS_IMETHODIMP -nsPK11Token::InitPassword(const char16_t* initialPassword) +nsPK11Token::InitPassword(const nsACString& initialPassword) { - // Note: It's OK for |initialPassword| to be null. nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - NS_ConvertUTF16toUTF8 utf8Password(initialPassword); return MapSECStatus( - PK11_InitPin(mSlot.get(), "", const_cast(utf8Password.get()))); + PK11_InitPin(mSlot.get(), "", PromiseFlatCString(initialPassword).get())); } NS_IMETHODIMP @@ -422,25 +341,21 @@ nsPK11Token::SetAskPasswordDefaults(const int32_t askTimes, } NS_IMETHODIMP -nsPK11Token::ChangePassword(const char16_t* oldPassword, - const char16_t* newPassword) +nsPK11Token::ChangePassword(const nsACString& oldPassword, + const nsACString& newPassword) { - // Note: It's OK for |oldPassword| and |newPassword| to be null. nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - 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(). + // PK11_ChangePW() has different semantics for the empty string and for + // nullptr. In order to support this difference, we need to check IsVoid() to + // find out if our caller supplied null/undefined args or just empty strings. // See Bug 447589. return MapSECStatus(PK11_ChangePW( mSlot.get(), - (oldPassword ? const_cast(utf8OldPassword.get()) : nullptr), - (newPassword ? const_cast(utf8NewPassword.get()) : nullptr))); + oldPassword.IsVoid() ? nullptr : PromiseFlatCString(oldPassword).get(), + newPassword.IsVoid() ? nullptr : PromiseFlatCString(newPassword).get())); } NS_IMETHODIMP @@ -525,9 +440,9 @@ nsPK11TokenDB::GetInternalKeyToken(nsIPK11Token** _retval) } NS_IMETHODIMP -nsPK11TokenDB::FindTokenByName(const char16_t* tokenName, nsIPK11Token** _retval) +nsPK11TokenDB::FindTokenByName(const nsACString& tokenName, + /*out*/ nsIPK11Token** _retval) { - // Note: It's OK for |tokenName| to be null. NS_ENSURE_ARG_POINTER(_retval); nsNSSShutDownPreventionLock locker; @@ -535,9 +450,8 @@ nsPK11TokenDB::FindTokenByName(const char16_t* tokenName, nsIPK11Token** _retval return NS_ERROR_NOT_AVAILABLE; } - NS_ConvertUTF16toUTF8 utf8TokenName(tokenName); UniquePK11SlotInfo slot( - PK11_FindSlotByName(const_cast(utf8TokenName.get()))); + PK11_FindSlotByName(PromiseFlatCString(tokenName).get())); if (!slot) { return NS_ERROR_FAILURE; } diff --git a/security/manager/ssl/nsPK11TokenDB.h b/security/manager/ssl/nsPK11TokenDB.h index 362eedf25c1f7..1d29e592fdf78 100644 --- a/security/manager/ssl/nsPK11TokenDB.h +++ b/security/manager/ssl/nsPK11TokenDB.h @@ -33,14 +33,19 @@ class nsPK11Token : public nsIPK11Token, friend class nsPK11TokenDB; nsresult refreshTokenInfo(const nsNSSShutDownPreventionLock& proofOfLock); - nsString mTokenName; - nsString mTokenLabel, mTokenManID, mTokenHWVersion, mTokenFWVersion; - nsString mTokenSerialNum; + nsCString mTokenName; + nsCString mTokenLabel; + nsCString mTokenManufacturerID; + nsCString mTokenHWVersion; + nsCString mTokenFWVersion; + nsCString mTokenSerialNum; mozilla::UniquePK11SlotInfo mSlot; int mSeries; nsCOMPtr mUIContext; virtual void virtualDestroyNSSReference() override; void destructorSafeDestroyNSSReference(); + nsresult GetAttributeHelper(const nsACString& attribute, + /*out*/ nsACString& xpcomOutParam); }; class nsPK11TokenDB : public nsIPK11TokenDB diff --git a/security/manager/ssl/nsPKCS11Slot.cpp b/security/manager/ssl/nsPKCS11Slot.cpp index f96132f162204..7703f35f6c664 100644 --- a/security/manager/ssl/nsPKCS11Slot.cpp +++ b/security/manager/ssl/nsPKCS11Slot.cpp @@ -11,6 +11,7 @@ #include "nsCOMPtr.h" #include "nsIMutableArray.h" #include "nsPK11TokenDB.h" +#include "nsPromiseFlatString.h" #include "secmod.h" using mozilla::LogLevel; @@ -44,29 +45,27 @@ nsPKCS11Slot::refreshSlotInfo(const nsNSSShutDownPreventionLock& /*proofOfLock*/ // Set the Description field const char* ccDesc = mozilla::BitwiseCast(slotInfo.slotDescription); - const nsACString& cDesc = Substring( - ccDesc, - ccDesc + PL_strnlen(ccDesc, sizeof(slotInfo.slotDescription))); - mSlotDesc = NS_ConvertUTF8toUTF16(cDesc); + // TODO(Bug 1305930): Stop using PL_strnlen() if/when all our supported + // platforms provide strnlen(). + mSlotDesc.Assign(ccDesc, PL_strnlen(ccDesc, sizeof(slotInfo.slotDescription))); mSlotDesc.Trim(" ", false, true); // Set the Manufacturer field const char* ccManID = mozilla::BitwiseCast(slotInfo.manufacturerID); - const nsACString& cManID = Substring( + mSlotManufacturerID.Assign( ccManID, - ccManID + PL_strnlen(ccManID, sizeof(slotInfo.manufacturerID))); - mSlotManID = NS_ConvertUTF8toUTF16(cManID); - mSlotManID.Trim(" ", false, true); + PL_strnlen(ccManID, sizeof(slotInfo.manufacturerID))); + mSlotManufacturerID.Trim(" ", false, true); // Set the Hardware Version field - mSlotHWVersion = EmptyString(); + mSlotHWVersion.Truncate(); mSlotHWVersion.AppendInt(slotInfo.hardwareVersion.major); mSlotHWVersion.Append('.'); mSlotHWVersion.AppendInt(slotInfo.hardwareVersion.minor); // Set the Firmware Version field - mSlotFWVersion = EmptyString(); + mSlotFWVersion.Truncate(); mSlotFWVersion.AppendInt(slotInfo.firmwareVersion.major); mSlotFWVersion.Append('.'); mSlotFWVersion.AppendInt(slotInfo.firmwareVersion.minor); @@ -96,40 +95,14 @@ nsPKCS11Slot::destructorSafeDestroyNSSReference() mSlot = nullptr; } -NS_IMETHODIMP -nsPKCS11Slot::GetName(char16_t** aName) +nsresult +nsPKCS11Slot::GetAttributeHelper(const nsACString& attribute, + /*out*/ nsACString& xpcomOutParam) { - NS_ENSURE_ARG_POINTER(aName); - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) + if (isAlreadyShutDown()) { return NS_ERROR_NOT_AVAILABLE; - - // |csn| is non-owning. - char* csn = PK11_GetSlotName(mSlot.get()); - if (*csn) { - *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(csn)); - } else if (PK11_HasRootCerts(mSlot.get())) { - // This is a workaround to an Root Module bug - the root certs module has - // no slot name. Not bothering to localize, because this is a workaround - // and for now all the slot names returned by NSS are char * anyway. - *aName = ToNewUnicode(NS_LITERAL_STRING("Root Certificates")); - } else { - // same as above, this is a catch-all - *aName = ToNewUnicode(NS_LITERAL_STRING("Unnamed Slot")); } - if (!*aName) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; -} - -NS_IMETHODIMP -nsPKCS11Slot::GetDesc(char16_t** aDesc) -{ - NS_ENSURE_ARG_POINTER(aDesc); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) - return NS_ERROR_NOT_AVAILABLE; if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { nsresult rv = refreshSlotInfo(locker); @@ -138,72 +111,56 @@ nsPKCS11Slot::GetDesc(char16_t** aDesc) } } - *aDesc = ToNewUnicode(mSlotDesc); - if (!*aDesc) return NS_ERROR_OUT_OF_MEMORY; + xpcomOutParam = attribute; return NS_OK; } NS_IMETHODIMP -nsPKCS11Slot::GetManID(char16_t** aManID) +nsPKCS11Slot::GetName(/*out*/ nsACString& name) { - NS_ENSURE_ARG_POINTER(aManID); - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { + if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - } - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshSlotInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } + // |csn| is non-owning. + char* csn = PK11_GetSlotName(mSlot.get()); + if (csn && *csn) { + name = csn; + } else if (PK11_HasRootCerts(mSlot.get())) { + // This is a workaround to an Root Module bug - the root certs module has + // no slot name. Not bothering to localize, because this is a workaround + // and for now all the slot names returned by NSS are char * anyway. + name = NS_LITERAL_CSTRING("Root Certificates"); + } else { + // same as above, this is a catch-all + name = NS_LITERAL_CSTRING("Unnamed Slot"); } - *aManID = ToNewUnicode(mSlotManID); - if (!*aManID) return NS_ERROR_OUT_OF_MEMORY; + return NS_OK; } NS_IMETHODIMP -nsPKCS11Slot::GetHWVersion(char16_t** aHWVersion) +nsPKCS11Slot::GetDesc(/*out*/ nsACString& desc) { - NS_ENSURE_ARG_POINTER(aHWVersion); - - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } - - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshSlotInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aHWVersion = ToNewUnicode(mSlotHWVersion); - if (!*aHWVersion) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; + return GetAttributeHelper(mSlotDesc, desc); } NS_IMETHODIMP -nsPKCS11Slot::GetFWVersion(char16_t** aFWVersion) +nsPKCS11Slot::GetManID(/*out*/ nsACString& manufacturerID) { - NS_ENSURE_ARG_POINTER(aFWVersion); + return GetAttributeHelper(mSlotManufacturerID, manufacturerID); +} - nsNSSShutDownPreventionLock locker; - if (isAlreadyShutDown()) { - return NS_ERROR_NOT_AVAILABLE; - } +NS_IMETHODIMP +nsPKCS11Slot::GetHWVersion(/*out*/ nsACString& hwVersion) +{ + return GetAttributeHelper(mSlotHWVersion, hwVersion); +} - if (PK11_GetSlotSeries(mSlot.get()) != mSeries) { - nsresult rv = refreshSlotInfo(locker); - if (NS_FAILED(rv)) { - return rv; - } - } - *aFWVersion = ToNewUnicode(mSlotFWVersion); - if (!*aFWVersion) return NS_ERROR_OUT_OF_MEMORY; - return NS_OK; +NS_IMETHODIMP +nsPKCS11Slot::GetFWVersion(/*out*/ nsACString& fwVersion) +{ + return GetAttributeHelper(mSlotFWVersion, fwVersion); } NS_IMETHODIMP @@ -221,16 +178,14 @@ nsPKCS11Slot::GetToken(nsIPK11Token** _retval) } NS_IMETHODIMP -nsPKCS11Slot::GetTokenName(char16_t** aName) +nsPKCS11Slot::GetTokenName(/*out*/ nsACString& tokenName) { - NS_ENSURE_ARG_POINTER(aName); - nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; if (!PK11_IsPresent(mSlot.get())) { - *aName = nullptr; + tokenName.SetIsVoid(true); return NS_OK; } @@ -241,8 +196,7 @@ nsPKCS11Slot::GetTokenName(char16_t** aName) } } - *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(PK11_GetTokenName(mSlot.get()))); - if (!*aName) return NS_ERROR_OUT_OF_MEMORY; + tokenName = PK11_GetTokenName(mSlot.get()); return NS_OK; } @@ -308,63 +262,59 @@ nsPKCS11Module::destructorSafeDestroyNSSReference() } NS_IMETHODIMP -nsPKCS11Module::GetName(char16_t** aName) +nsPKCS11Module::GetName(/*out*/ nsACString& name) { - NS_ENSURE_ARG_POINTER(aName); - nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(mModule->commonName)); + name = mModule->commonName; return NS_OK; } NS_IMETHODIMP -nsPKCS11Module::GetLibName(char16_t** aName) +nsPKCS11Module::GetLibName(/*out*/ nsACString& libName) { - NS_ENSURE_ARG_POINTER(aName); - nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - if ( mModule->dllName ) { - *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(mModule->dllName)); + if (mModule->dllName) { + libName = mModule->dllName; } else { - *aName = nullptr; + libName.SetIsVoid(true); } return NS_OK; } NS_IMETHODIMP -nsPKCS11Module::FindSlotByName(const char16_t* aName, nsIPKCS11Slot** _retval) +nsPKCS11Module::FindSlotByName(const nsACString& name, + /*out*/ nsIPKCS11Slot** _retval) { - // Note: It's OK for |aName| to be null. NS_ENSURE_ARG_POINTER(_retval); nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) return NS_ERROR_NOT_AVAILABLE; - NS_ConvertUTF16toUTF8 asciiname(aName); - MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Getting \"%s\"\n", asciiname.get())); + const nsCString& flatName = PromiseFlatCString(name); + MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Getting \"%s\"", flatName.get())); UniquePK11SlotInfo slotInfo; UniquePK11SlotList slotList(PK11_FindSlotsByNames(mModule->dllName, - asciiname.get() /*slotName*/, + flatName.get() /*slotName*/, nullptr /*tokenName*/, false)); if (!slotList) { /* name must be the token name */ slotList.reset(PK11_FindSlotsByNames(mModule->dllName, nullptr /*slotName*/, - asciiname.get() /*tokenName*/, false)); + flatName.get() /*tokenName*/, false)); } if (slotList && slotList->head && slotList->head->slot) { slotInfo.reset(PK11_ReferenceSlot(slotList->head->slot)); } if (!slotInfo) { // workaround - the builtin module has no name - if (!asciiname.EqualsLiteral("Root Certificates")) { + if (!flatName.EqualsLiteral("Root Certificates")) { // Give up. return NS_ERROR_FAILURE; } @@ -469,10 +419,9 @@ nsPKCS11ModuleDB::GetInternalFIPS(nsIPKCS11Module** _retval) } NS_IMETHODIMP -nsPKCS11ModuleDB::FindModuleByName(const char16_t* aName, - nsIPKCS11Module** _retval) +nsPKCS11ModuleDB::FindModuleByName(const nsACString& name, + /*out*/ nsIPKCS11Module** _retval) { - // Note: It's OK for |aName| to be null. NS_ENSURE_ARG_POINTER(_retval); nsNSSShutDownPreventionLock locker; @@ -480,8 +429,7 @@ nsPKCS11ModuleDB::FindModuleByName(const char16_t* aName, return NS_ERROR_NOT_AVAILABLE; } - NS_ConvertUTF16toUTF8 utf8Name(aName); - UniqueSECMODModule mod(SECMOD_FindModule(const_cast(utf8Name.get()))); + UniqueSECMODModule mod(SECMOD_FindModule(PromiseFlatCString(name).get())); if (!mod) { return NS_ERROR_FAILURE; } @@ -495,10 +443,9 @@ nsPKCS11ModuleDB::FindModuleByName(const char16_t* aName, * that it returns an nsIPKCS11Slot, which may be desired. */ NS_IMETHODIMP -nsPKCS11ModuleDB::FindSlotByName(const char16_t* aName, - nsIPKCS11Slot** _retval) +nsPKCS11ModuleDB::FindSlotByName(const nsACString& name, + /*out*/ nsIPKCS11Slot** _retval) { - // Note: It's OK for |aName| to be null. NS_ENSURE_ARG_POINTER(_retval); nsNSSShutDownPreventionLock locker; @@ -506,9 +453,8 @@ nsPKCS11ModuleDB::FindSlotByName(const char16_t* aName, return NS_ERROR_NOT_AVAILABLE; } - NS_ConvertUTF16toUTF8 utf8Name(aName); UniquePK11SlotInfo slotInfo( - PK11_FindSlotByName(const_cast(utf8Name.get()))); + PK11_FindSlotByName(PromiseFlatCString(name).get())); if (!slotInfo) { return NS_ERROR_FAILURE; } diff --git a/security/manager/ssl/nsPKCS11Slot.h b/security/manager/ssl/nsPKCS11Slot.h index f309881c5170f..978df377c4298 100644 --- a/security/manager/ssl/nsPKCS11Slot.h +++ b/security/manager/ssl/nsPKCS11Slot.h @@ -31,12 +31,17 @@ class nsPKCS11Slot : public nsIPKCS11Slot, private: mozilla::UniquePK11SlotInfo mSlot; - nsString mSlotDesc, mSlotManID, mSlotHWVersion, mSlotFWVersion; + nsCString mSlotDesc; + nsCString mSlotManufacturerID; + nsCString mSlotHWVersion; + nsCString mSlotFWVersion; int mSeries; virtual void virtualDestroyNSSReference() override; void destructorSafeDestroyNSSReference(); nsresult refreshSlotInfo(const nsNSSShutDownPreventionLock& proofOfLock); + nsresult GetAttributeHelper(const nsACString& attribute, + /*out*/ nsACString& xpcomOutParam); }; class nsPKCS11Module : public nsIPKCS11Module, diff --git a/security/manager/ssl/nsPKCS12Blob.cpp b/security/manager/ssl/nsPKCS12Blob.cpp index 5de83b7bef0a3..55dfa98622dae 100644 --- a/security/manager/ssl/nsPKCS12Blob.cpp +++ b/security/manager/ssl/nsPKCS12Blob.cpp @@ -143,7 +143,7 @@ nsPKCS12Blob::ImportFromFileHelper(nsIFile *file, SECItem unicodePw; UniquePK11SlotInfo slot; - nsXPIDLString tokenName; + nsAutoCString tokenName; unicodePw.data = nullptr; aWantRetry = rr_do_not_retry; @@ -163,11 +163,11 @@ nsPKCS12Blob::ImportFromFileHelper(nsIFile *file, } } - mToken->GetTokenName(getter_Copies(tokenName)); - { - NS_ConvertUTF16toUTF8 tokenNameCString(tokenName); - slot = UniquePK11SlotInfo(PK11_FindSlotByName(tokenNameCString.get())); + rv = mToken->GetTokenName(tokenName); + if (NS_FAILED(rv)) { + goto finish; } + slot = UniquePK11SlotInfo(PK11_FindSlotByName(tokenName.get())); if (!slot) { srv = SECFailure; goto finish; diff --git a/security/manager/ssl/tests/unit/test_pkcs11_slot.js b/security/manager/ssl/tests/unit/test_pkcs11_slot.js index 7ca11c2aa6a54..43489fcbb89be 100644 --- a/security/manager/ssl/tests/unit/test_pkcs11_slot.js +++ b/security/manager/ssl/tests/unit/test_pkcs11_slot.js @@ -23,6 +23,9 @@ function run_test() { let moduleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"] .getService(Ci.nsIPKCS11ModuleDB); let testModule = moduleDB.findModuleByName("PKCS11 Test Module"); + // TODO(Bug 1306632): Add a slot to the PSM PKCS 11 test module with a name + // with high codepoints, then add a test to ensure + // findSlotByName() is able to find the slot. let testSlot = testModule.findSlotByName("Test PKCS11 Slot"); equal(testSlot.name, "Test PKCS11 Slot", diff --git a/security/manager/ssl/tests/unit/test_pkcs11_token.js b/security/manager/ssl/tests/unit/test_pkcs11_token.js index 8c038dc114d72..f77b6fcbe277a 100644 --- a/security/manager/ssl/tests/unit/test_pkcs11_token.js +++ b/security/manager/ssl/tests/unit/test_pkcs11_token.js @@ -59,8 +59,8 @@ function checkPasswordFeaturesAndResetPassword(token, initialPW) { ok(token.checkPassword(initialPW), "checkPassword() should succeed if the correct initial password is given"); - token.changePassword(initialPW, "newPW"); - ok(token.checkPassword("newPW"), + token.changePassword(initialPW, "newPW ÿ 一二三"); + ok(token.checkPassword("newPW ÿ 一二三"), "checkPassword() should succeed if the correct new password is given"); ok(!token.checkPassword("wrongPW"),