Bug 1287290 - Use ScopedAutoSECItem in PSM more.
ScopedAutoSECItem is useful for:
1. Removing manual memory management.
2. Getting rid of this pattern:
> UniqueSECItem item(SECITEM_AllocItem(nullptr, nullptr, 0));
While this pattern works, ScopedAutoSECItem is slightly superior in that it
doesn't unnecessarily cause a SECItem to be allocated from the heap.
MozReview-Commit-ID: 8DPD9gtzeru
--- a/security/manager/ssl/ContentSignatureVerifier.cpp
+++ b/security/manager/ssl/ContentSignatureVerifier.cpp
@@ -7,16 +7,17 @@
#include "ContentSignatureVerifier.h"
#include "BRNameMatchingPolicy.h"
#include "SharedCertVerifier.h"
#include "cryptohi.h"
#include "keyhi.h"
#include "mozilla/Assertions.h"
#include "mozilla/Casting.h"
+#include "mozilla/unused.h"
#include "nsCOMPtr.h"
#include "nsContentUtils.h"
#include "nsISupportsPriority.h"
#include "nsIURI.h"
#include "nsNSSComponent.h"
#include "nsSecurityHeaderParser.h"
#include "nsStreamUtils.h"
#include "nsWhitespaceTokenizer.h"
@@ -94,35 +95,34 @@ ReadChainIntoCertList(const nsACString&
if (token.IsEmpty()) {
continue;
}
if (inBlock) {
if (token.Equals(footer)) {
inBlock = false;
certFound = true;
// base64 decode data, make certs, append to chain
- UniqueSECItem der(::SECITEM_AllocItem(nullptr, nullptr, 0));
- if (!der || !NSSBase64_DecodeBuffer(nullptr, der.get(),
- blockData.BeginReading(),
- blockData.Length())) {
+ ScopedAutoSECItem der;
+ if (!NSSBase64_DecodeBuffer(nullptr, &der, blockData.BeginReading(),
+ blockData.Length())) {
CSVerifier_LOG(("CSVerifier: decoding the signature failed\n"));
return NS_ERROR_FAILURE;
}
- CERTCertificate* tmpCert =
- CERT_NewTempCertificate(CERT_GetDefaultCertDB(), der.get(), nullptr,
- false, true);
+ UniqueCERTCertificate tmpCert(
+ CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &der, nullptr, false,
+ true));
if (!tmpCert) {
return NS_ERROR_FAILURE;
}
// if adding tmpCert succeeds, tmpCert will now be owned by aCertList
- SECStatus res = CERT_AddCertToListTail(aCertList, tmpCert);
+ SECStatus res = CERT_AddCertToListTail(aCertList, tmpCert.get());
if (res != SECSuccess) {
- CERT_DestroyCertificate(tmpCert);
return MapSECStatus(res);
}
+ Unused << tmpCert.release();
} else {
blockData.Append(token);
}
} else if (token.Equals(header)) {
inBlock = true;
blockData = "";
}
}
@@ -209,48 +209,43 @@ ContentSignatureVerifier::CreateContextI
// in case we were not able to extract a key
if (!mKey) {
CSVerifier_LOG(("CSVerifier: unable to extract a key\n"));
return NS_ERROR_INVALID_SIGNATURE;
}
// Base 64 decode the signature
- UniqueSECItem rawSignatureItem(::SECITEM_AllocItem(nullptr, nullptr, 0));
- if (!rawSignatureItem ||
- !NSSBase64_DecodeBuffer(nullptr, rawSignatureItem.get(),
- mSignature.get(), mSignature.Length())) {
+ ScopedAutoSECItem rawSignatureItem;
+ if (!NSSBase64_DecodeBuffer(nullptr, &rawSignatureItem, mSignature.get(),
+ mSignature.Length())) {
CSVerifier_LOG(("CSVerifier: decoding the signature failed\n"));
return NS_ERROR_FAILURE;
}
// get signature object
- UniqueSECItem signatureItem(::SECITEM_AllocItem(nullptr, nullptr, 0));
- if (!signatureItem) {
- return NS_ERROR_FAILURE;
- }
+ ScopedAutoSECItem signatureItem;
// We have a raw ecdsa signature r||s so we have to DER-encode it first
// Note that we have to check rawSignatureItem->len % 2 here as
// DSAU_EncodeDerSigWithLen asserts this
- if (rawSignatureItem->len == 0 || rawSignatureItem->len % 2 != 0) {
+ if (rawSignatureItem.len == 0 || rawSignatureItem.len % 2 != 0) {
CSVerifier_LOG(("CSVerifier: signature length is bad\n"));
return NS_ERROR_FAILURE;
}
- if (DSAU_EncodeDerSigWithLen(signatureItem.get(), rawSignatureItem.get(),
- rawSignatureItem->len) != SECSuccess) {
+ if (DSAU_EncodeDerSigWithLen(&signatureItem, &rawSignatureItem,
+ rawSignatureItem.len) != SECSuccess) {
CSVerifier_LOG(("CSVerifier: encoding the signature failed\n"));
return NS_ERROR_FAILURE;
}
// this is the only OID we support for now
SECOidTag oid = SEC_OID_ANSIX962_ECDSA_SHA384_SIGNATURE;
mCx = UniqueVFYContext(
- VFY_CreateContext(mKey.get(), signatureItem.get(), oid, nullptr));
-
+ VFY_CreateContext(mKey.get(), &signatureItem, oid, nullptr));
if (!mCx) {
return NS_ERROR_INVALID_SIGNATURE;
}
if (VFY_Begin(mCx.get()) != SECSuccess) {
return NS_ERROR_INVALID_SIGNATURE;
}
--- a/security/manager/ssl/nsNSSU2FToken.cpp
+++ b/security/manager/ssl/nsNSSU2FToken.cpp
@@ -598,44 +598,39 @@ nsNSSU2FToken::Register(uint8_t* aApplic
// It's OK to ignore the return values here because we're writing into
// pre-allocated space
signedDataBuf.AppendElement(0x00, mozilla::fallible);
signedDataBuf.AppendElements(aApplication, aApplicationLen, mozilla::fallible);
signedDataBuf.AppendElements(aChallenge, aChallengeLen, mozilla::fallible);
signedDataBuf.AppendSECItem(keyHandleItem.get());
signedDataBuf.AppendSECItem(pubKey->u.ec.publicValue);
- UniqueSECItem signatureItem(::SECITEM_AllocItem(/* default arena */ nullptr,
- /* no buffer */ nullptr, 0));
- if (!signatureItem) {
- return NS_ERROR_OUT_OF_MEMORY;
- }
-
- SECStatus srv = SEC_SignData(signatureItem.get(), signedDataBuf.Elements(),
+ ScopedAutoSECItem signatureItem;
+ SECStatus srv = SEC_SignData(&signatureItem, signedDataBuf.Elements(),
signedDataBuf.Length(), attestPrivKey.get(),
SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE);
if (srv != SECSuccess) {
MOZ_LOG(gNSSTokenLog, LogLevel::Warning,
("Signature failure: %d", PORT_GetError()));
return NS_ERROR_FAILURE;
}
// Serialize the registration data
mozilla::dom::CryptoBuffer registrationBuf;
if (!registrationBuf.SetCapacity(1 + kPublicKeyLen + 1 + keyHandleItem->len +
attestCert.get()->derCert.len +
- signatureItem->len, mozilla::fallible)) {
+ signatureItem.len, mozilla::fallible)) {
return NS_ERROR_OUT_OF_MEMORY;
}
registrationBuf.AppendElement(0x05, mozilla::fallible);
registrationBuf.AppendSECItem(pubKey->u.ec.publicValue);
registrationBuf.AppendElement(keyHandleItem->len, mozilla::fallible);
registrationBuf.AppendSECItem(keyHandleItem.get());
registrationBuf.AppendSECItem(attestCert.get()->derCert);
- registrationBuf.AppendSECItem(signatureItem.get());
+ registrationBuf.AppendSECItem(signatureItem);
if (!registrationBuf.ToNewUnsignedBuffer(aRegistration, aRegistrationLen)) {
return NS_ERROR_FAILURE;
}
return NS_OK;
}
// A U2F Sign operation creates a signature over the "param" arguments (plus
@@ -722,41 +717,36 @@ nsNSSU2FToken::Sign(uint8_t* aApplicatio
// It's OK to ignore the return values here because we're writing into
// pre-allocated space
signedDataBuf.AppendElements(aApplication, aApplicationLen, mozilla::fallible);
signedDataBuf.AppendElement(0x01, mozilla::fallible);
signedDataBuf.AppendSECItem(counterItem);
signedDataBuf.AppendElements(aChallenge, aChallengeLen, mozilla::fallible);
- UniqueSECItem signatureItem(::SECITEM_AllocItem(/* default arena */ nullptr,
- /* no buffer */ nullptr, 0));
- if (!signatureItem) {
- return NS_ERROR_OUT_OF_MEMORY;
- }
-
- SECStatus srv = SEC_SignData(signatureItem.get(), signedDataBuf.Elements(),
+ ScopedAutoSECItem signatureItem;
+ SECStatus srv = SEC_SignData(&signatureItem, signedDataBuf.Elements(),
signedDataBuf.Length(), privKey.get(),
SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE);
if (srv != SECSuccess) {
MOZ_LOG(gNSSTokenLog, LogLevel::Warning,
("Signature failure: %d", PORT_GetError()));
return NS_ERROR_FAILURE;
}
// Assemble the signature data into a buffer for return
mozilla::dom::CryptoBuffer signatureBuf;
- if (!signatureBuf.SetCapacity(1 + counterItem.len + signatureItem->len,
+ if (!signatureBuf.SetCapacity(1 + counterItem.len + signatureItem.len,
mozilla::fallible)) {
return NS_ERROR_OUT_OF_MEMORY;
}
// It's OK to ignore the return values here because we're writing into
// pre-allocated space
signatureBuf.AppendElement(0x01, mozilla::fallible);
signatureBuf.AppendSECItem(counterItem);
- signatureBuf.AppendSECItem(signatureItem.get());
+ signatureBuf.AppendSECItem(signatureItem);
if (!signatureBuf.ToNewUnsignedBuffer(aSignature, aSignatureLen)) {
return NS_ERROR_FAILURE;
}
return NS_OK;
}
--- a/security/manager/ssl/tests/unit/tlsserver/lib/TLSServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/lib/TLSServer.cpp
@@ -178,26 +178,25 @@ DecodeCertCallback(void* arg, SECItem**
SECStatus
AddCertificateFromFile(const char* basePath, const char* filename)
{
char buf[16384] = { 0 };
SECStatus rv = ReadFileToBuffer(basePath, filename, buf);
if (rv != SECSuccess) {
return rv;
}
- SECItem certDER;
+ ScopedAutoSECItem certDER;
rv = CERT_DecodeCertPackage(buf, strlen(buf), DecodeCertCallback, &certDER);
if (rv != SECSuccess) {
PrintPRError("CERT_DecodeCertPackage failed");
return rv;
}
UniqueCERTCertificate cert(CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
&certDER, nullptr, false,
true));
- PORT_Free(certDER.data);
if (!cert) {
PrintPRError("CERT_NewTempCertificate failed");
return SECFailure;
}
UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
if (!slot) {
PrintPRError("PK11_GetInternalKeySlot failed");
return SECFailure;