Bug 1029173 - Clean up nsDataSignatureVerifier. r=keeler
This patch does the following:
- Implements nsNSSShutDownObject.
- Replaces more raw pointers with smart pointers.
- Fixes other misc issues.
MozReview-Commit-ID: HulWdonEbP8
--- a/security/apps/AppSignatureVerification.cpp
+++ b/security/apps/AppSignatureVerification.cpp
@@ -658,21 +658,28 @@ VerifyCertificate(CERTCertificate* signe
return NS_OK;
}
nsresult
VerifySignature(AppTrustedRoot trustedRoot, const SECItem& buffer,
const SECItem& detachedDigest,
/*out*/ ScopedCERTCertList& builtChain)
{
+ // Currently, this function is only called within the CalculateResult() method
+ // of CryptoTasks. As such, NSS should not be shut down at this point and the
+ // CryptoTask implementation should already hold a nsNSSShutDownPreventionLock.
+ // We acquire a nsNSSShutDownPreventionLock here solely to prove we did to
+ // VerifyCMSDetachedSignatureIncludingCertificate().
+ nsNSSShutDownPreventionLock locker;
VerifyCertificateContext context = { trustedRoot, builtChain };
// XXX: missing pinArg
return VerifyCMSDetachedSignatureIncludingCertificate(buffer, detachedDigest,
VerifyCertificate,
- &context, nullptr);
+ &context, nullptr,
+ locker);
}
NS_IMETHODIMP
OpenSignedAppFile(AppTrustedRoot aTrustedRoot, nsIFile* aJarFile,
/*out, optional */ nsIZipReader** aZipReader,
/*out, optional */ nsIX509Cert** aSignerCert)
{
NS_ENSURE_ARG_POINTER(aJarFile);
--- a/security/manager/ssl/ScopedNSSTypes.h
+++ b/security/manager/ssl/ScopedNSSTypes.h
@@ -339,16 +339,19 @@ MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(Un
CERTCertList,
CERT_DestroyCertList)
MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTCertNicknames,
CERTCertNicknames,
CERT_FreeNicknames)
MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTOidSequence,
CERTOidSequence,
CERT_DestroyOidSequence)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTSubjectPublicKeyInfo,
+ CERTSubjectPublicKeyInfo,
+ SECKEY_DestroySubjectPublicKeyInfo)
MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTUserNotice,
CERTUserNotice,
CERT_DestroyUserNotice)
MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueNSSCMSMessage,
NSSCMSMessage,
NSS_CMSMessage_Destroy)
MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueNSSCMSSignedData,
--- a/security/manager/ssl/nsDataSignatureVerifier.cpp
+++ b/security/manager/ssl/nsDataSignatureVerifier.cpp
@@ -2,16 +2,17 @@
* 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 "nsDataSignatureVerifier.h"
#include "cms.h"
#include "cryptohi.h"
#include "keyhi.h"
+#include "mozilla/unused.h"
#include "nsCOMPtr.h"
#include "nsNSSComponent.h"
#include "nssb64.h"
#include "pkix/pkixtypes.h"
#include "ScopedNSSTypes.h"
#include "secerr.h"
#include "SharedCertVerifier.h"
@@ -30,95 +31,106 @@ const SEC_ASN1Template CERT_SignatureDat
{ SEC_ASN1_INLINE | SEC_ASN1_XTRN,
offsetof(CERTSignedData,signatureAlgorithm),
SEC_ASN1_SUB(SECOID_AlgorithmIDTemplate), },
{ SEC_ASN1_BIT_STRING,
offsetof(CERTSignedData,signature), },
{ 0, }
};
-NS_IMETHODIMP
-nsDataSignatureVerifier::VerifyData(const nsACString & aData,
- const nsACString & aSignature,
- const nsACString & aPublicKey,
- bool *_retval)
+nsDataSignatureVerifier::~nsDataSignatureVerifier()
{
- // Allocate an arena to handle the majority of the allocations
- UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
- if (!arena) {
- return NS_ERROR_OUT_OF_MEMORY;
- }
+ nsNSSShutDownPreventionLock locker;
+ if (isAlreadyShutDown()) {
+ return;
+ }
+
+ shutdown(calledFromObject);
+}
+
+NS_IMETHODIMP
+nsDataSignatureVerifier::VerifyData(const nsACString& aData,
+ const nsACString& aSignature,
+ const nsACString& aPublicKey,
+ bool* _retval)
+{
+ NS_ENSURE_ARG_POINTER(_retval);
- // Base 64 decode the key
- SECItem keyItem;
- PORT_Memset(&keyItem, 0, sizeof(SECItem));
- if (!NSSBase64_DecodeBuffer(arena.get(), &keyItem,
- nsPromiseFlatCString(aPublicKey).get(),
- aPublicKey.Length())) {
- return NS_ERROR_FAILURE;
- }
+ nsNSSShutDownPreventionLock locker;
+ if (isAlreadyShutDown()) {
+ return NS_ERROR_NOT_AVAILABLE;
+ }
- // Extract the public key from the data
- CERTSubjectPublicKeyInfo *pki = SECKEY_DecodeDERSubjectPublicKeyInfo(&keyItem);
- if (!pki) {
- return NS_ERROR_FAILURE;
- }
- SECKEYPublicKey *publicKey = SECKEY_ExtractPublicKey(pki);
- SECKEY_DestroySubjectPublicKeyInfo(pki);
- pki = nullptr;
+ // Allocate an arena to handle the majority of the allocations
+ UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+ if (!arena) {
+ return NS_ERROR_OUT_OF_MEMORY;
+ }
- if (!publicKey) {
- return NS_ERROR_FAILURE;
- }
+ // Base 64 decode the key
+ SECItem keyItem;
+ PORT_Memset(&keyItem, 0, sizeof(SECItem));
+ if (!NSSBase64_DecodeBuffer(arena.get(), &keyItem,
+ PromiseFlatCString(aPublicKey).get(),
+ aPublicKey.Length())) {
+ return NS_ERROR_FAILURE;
+ }
- // Base 64 decode the signature
- SECItem signatureItem;
- PORT_Memset(&signatureItem, 0, sizeof(SECItem));
- if (!NSSBase64_DecodeBuffer(arena.get(), &signatureItem,
- nsPromiseFlatCString(aSignature).get(),
- aSignature.Length())) {
- SECKEY_DestroyPublicKey(publicKey);
- return NS_ERROR_FAILURE;
- }
+ // Extract the public key from the data
+ UniqueCERTSubjectPublicKeyInfo pki(
+ SECKEY_DecodeDERSubjectPublicKeyInfo(&keyItem));
+ if (!pki) {
+ return NS_ERROR_FAILURE;
+ }
+
+ UniqueSECKEYPublicKey publicKey(SECKEY_ExtractPublicKey(pki.get()));
+ if (!publicKey) {
+ return NS_ERROR_FAILURE;
+ }
+
+ // Base 64 decode the signature
+ SECItem signatureItem;
+ PORT_Memset(&signatureItem, 0, sizeof(SECItem));
+ if (!NSSBase64_DecodeBuffer(arena.get(), &signatureItem,
+ PromiseFlatCString(aSignature).get(),
+ aSignature.Length())) {
+ return NS_ERROR_FAILURE;
+ }
- // Decode the signature and algorithm
- CERTSignedData sigData;
- PORT_Memset(&sigData, 0, sizeof(CERTSignedData));
- SECStatus ss = SEC_QuickDERDecodeItem(arena.get(), &sigData,
- CERT_SignatureDataTemplate,
- &signatureItem);
- if (ss != SECSuccess) {
- SECKEY_DestroyPublicKey(publicKey);
- return NS_ERROR_FAILURE;
- }
+ // Decode the signature and algorithm
+ CERTSignedData sigData;
+ PORT_Memset(&sigData, 0, sizeof(CERTSignedData));
+ SECStatus srv = SEC_QuickDERDecodeItem(arena.get(), &sigData,
+ CERT_SignatureDataTemplate,
+ &signatureItem);
+ if (srv != SECSuccess) {
+ return NS_ERROR_FAILURE;
+ }
- // Perform the final verification
- DER_ConvertBitString(&(sigData.signature));
- ss = VFY_VerifyDataWithAlgorithmID((const unsigned char*)nsPromiseFlatCString(aData).get(),
- aData.Length(), publicKey,
- &(sigData.signature),
- &(sigData.signatureAlgorithm),
- nullptr, nullptr);
+ // Perform the final verification
+ DER_ConvertBitString(&(sigData.signature));
+ srv = VFY_VerifyDataWithAlgorithmID(
+ reinterpret_cast<const unsigned char*>(PromiseFlatCString(aData).get()),
+ aData.Length(), publicKey.get(), &(sigData.signature),
+ &(sigData.signatureAlgorithm), nullptr, nullptr);
- // Clean up remaining objects
- SECKEY_DestroyPublicKey(publicKey);
+ *_retval = (srv == SECSuccess);
- *_retval = (ss == SECSuccess);
-
- return NS_OK;
+ return NS_OK;
}
namespace mozilla {
nsresult
VerifyCMSDetachedSignatureIncludingCertificate(
const SECItem& buffer, const SECItem& detachedDigest,
nsresult (*verifyCertificate)(CERTCertificate* cert, void* context,
void* pinArg),
- void *verifyCertificateContext, void* pinArg)
+ void* verifyCertificateContext, void* pinArg,
+ const nsNSSShutDownPreventionLock& /*proofOfLock*/)
{
// XXX: missing pinArg is tolerated.
if (NS_WARN_IF(!buffer.data && buffer.len > 0) ||
NS_WARN_IF(!detachedDigest.data && detachedDigest.len > 0) ||
(!verifyCertificate) ||
NS_WARN_IF(!verifyCertificateContext)) {
return NS_ERROR_INVALID_ARG;
}
@@ -150,34 +162,36 @@ VerifyCMSDetachedSignatureIncludingCerti
// Set digest value.
if (NSS_CMSSignedData_SetDigestValue(signedData, SEC_OID_SHA1,
const_cast<SECItem*>(&detachedDigest))) {
return NS_ERROR_CMS_VERIFY_BAD_DIGEST;
}
// Parse the certificates into CERTCertificate objects held in memory so
// verifyCertificate will be able to find them during path building.
- ScopedCERTCertList certs(CERT_NewCertList());
+ UniqueCERTCertList certs(CERT_NewCertList());
if (!certs) {
return NS_ERROR_OUT_OF_MEMORY;
}
if (signedData->rawCerts) {
for (size_t i = 0; signedData->rawCerts[i]; ++i) {
- ScopedCERTCertificate
+ UniqueCERTCertificate
cert(CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
signedData->rawCerts[i], nullptr, false,
true));
// Skip certificates that fail to parse
- if (cert) {
- if (CERT_AddCertToListTail(certs.get(), cert.get()) == SECSuccess) {
- cert.forget(); // ownership transfered
- } else {
- return NS_ERROR_OUT_OF_MEMORY;
- }
+ if (!cert) {
+ continue;
}
+
+ if (CERT_AddCertToListTail(certs.get(), cert.get()) != SECSuccess) {
+ return NS_ERROR_OUT_OF_MEMORY;
+ }
+
+ Unused << cert.release(); // Ownership transferred to the cert list.
}
}
// Get the end-entity certificate.
int numSigners = NSS_CMSSignedData_SignerInfoCount(signedData);
if (NS_WARN_IF(numSigners != 1)) {
return NS_ERROR_CMS_VERIFY_ERROR_PROCESSING;
}
@@ -252,44 +266,46 @@ VerifyCertificate(CERTCertificate* cert,
NS_IMETHODIMP
nsDataSignatureVerifier::VerifySignature(const char* aRSABuf,
uint32_t aRSABufLen,
const char* aPlaintext,
uint32_t aPlaintextLen,
int32_t* aErrorCode,
nsIX509Cert** aSigningCert)
{
- if (!aPlaintext || !aSigningCert || !aErrorCode) {
+ if (!aRSABuf || !aPlaintext || !aErrorCode || !aSigningCert) {
return NS_ERROR_INVALID_ARG;
}
+ nsNSSShutDownPreventionLock locker;
+ if (isAlreadyShutDown()) {
+ return NS_ERROR_NOT_AVAILABLE;
+ }
+
*aErrorCode = VERIFY_ERROR_OTHER;
*aSigningCert = nullptr;
- nsNSSShutDownPreventionLock locker;
-
Digest digest;
- nsresult rv = digest.DigestBuf(SEC_OID_SHA1,
- reinterpret_cast<const uint8_t*>(aPlaintext),
+ nsresult rv = digest.DigestBuf(SEC_OID_SHA1, uint8_t_ptr_cast(aPlaintext),
aPlaintextLen);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
SECItem buffer = {
siBuffer,
reinterpret_cast<uint8_t*>(const_cast<char*>(aRSABuf)),
aRSABufLen
};
VerifyCertificateContext context;
// XXX: pinArg is missing
rv = VerifyCMSDetachedSignatureIncludingCertificate(buffer, digest.get(),
VerifyCertificate,
- &context, nullptr);
+ &context, nullptr, locker);
if (NS_SUCCEEDED(rv)) {
*aErrorCode = VERIFY_OK;
} else if (NS_ERROR_GET_MODULE(rv) == NS_ERROR_MODULE_SECURITY) {
if (rv == GetXPCOMFromNSSError(SEC_ERROR_UNKNOWN_ISSUER)) {
*aErrorCode = VERIFY_ERROR_UNKNOWN_ISSUER;
} else {
*aErrorCode = VERIFY_ERROR_OTHER;
}
--- a/security/manager/ssl/nsDataSignatureVerifier.h
+++ b/security/manager/ssl/nsDataSignatureVerifier.h
@@ -1,48 +1,47 @@
/* 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/. */
-#ifndef _NS_DATASIGNATUREVERIFIER_H_
-#define _NS_DATASIGNATUREVERIFIER_H_
-
-#include "nsIDataSignatureVerifier.h"
-#include "mozilla/Attributes.h"
+#ifndef nsDataSignatureVerifier_h
+#define nsDataSignatureVerifier_h
-#include "keythi.h"
+#include "certt.h"
+#include "nsIDataSignatureVerifier.h"
+#include "nsNSSShutDown.h"
-typedef struct CERTCertificateStr CERTCertificate;
-
-// 296d76aa-275b-4f3c-af8a-30a4026c18fc
#define NS_DATASIGNATUREVERIFIER_CID \
{ 0x296d76aa, 0x275b, 0x4f3c, \
{ 0xaf, 0x8a, 0x30, 0xa4, 0x02, 0x6c, 0x18, 0xfc } }
#define NS_DATASIGNATUREVERIFIER_CONTRACTID \
"@mozilla.org/security/datasignatureverifier;1"
class nsDataSignatureVerifier final : public nsIDataSignatureVerifier
+ , public nsNSSShutDownObject
{
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIDATASIGNATUREVERIFIER
nsDataSignatureVerifier()
{
}
private:
- ~nsDataSignatureVerifier()
- {
- }
+ ~nsDataSignatureVerifier();
+
+ // Nothing to release.
+ virtual void virtualDestroyNSSReference() override {}
};
namespace mozilla {
nsresult VerifyCMSDetachedSignatureIncludingCertificate(
const SECItem& buffer, const SECItem& detachedDigest,
nsresult (*verifyCertificate)(CERTCertificate* cert, void* context,
void* pinArg),
- void* verifyCertificateContext, void* pinArg);
+ void* verifyCertificateContext, void* pinArg,
+ const nsNSSShutDownPreventionLock& proofOfLock);
} // namespace mozilla
-#endif // _NS_DATASIGNATUREVERIFIER_H_
+#endif // nsDataSignatureVerifier_h