Skip to content

Commit

Permalink
Eliminate ScopedOpenSSL in favour of scoped_ptr<> specializations.
Browse files Browse the repository at this point in the history
Match the NSS, CryptoAPI (Win) and Security (OS X) approaches by
declaring the scoped types as specializations of our existing scoped
classes.

Like NSS, this requires an intermediate helper type, because our
scoped_ptr<> doesn't accept deleter functions as template
arguments (though they are valid in C++11's unique_ptr<>). A few base
cryptographic (non-certificate) types are used in
scoped_openssl_types.h, while the remainder are left for
implementations to specialize as needed.

In an ideal world, this would be scoped_ptr<FOO, FOO_free>, but that
will require unique_ptr<> support.

BUG=388904

Review URL: https://codereview.chromium.org/361193003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282257 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rsleevi@chromium.org committed Jul 10, 2014
1 parent 253a241 commit cd9b75b
Show file tree
Hide file tree
Showing 29 changed files with 274 additions and 253 deletions.
46 changes: 24 additions & 22 deletions chrome/common/net/x509_certificate_model_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/strings/utf_string_conversions.h"
#include "crypto/openssl_bio_string.h"
#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"
#include "grit/generated_resources.h"
#include "net/base/net_util.h"
#include "net/cert/x509_util_openssl.h"
Expand Down Expand Up @@ -463,7 +464,7 @@ std::string ProcessNSCertTypeExtension(X509_EXTENSION* ex) {
{NS_OBJSIGN_CA, IDS_CERT_USAGE_OBJECT_SIGNER},
};

crypto::ScopedOpenSSL<ASN1_BIT_STRING, ASN1_BIT_STRING_free> value(
crypto::ScopedOpenSSL<ASN1_BIT_STRING, ASN1_BIT_STRING_free>::Type value(
reinterpret_cast<ASN1_BIT_STRING*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand All @@ -486,7 +487,7 @@ std::string ProcessKeyUsageExtension(X509_EXTENSION* ex) {
{KU_DECIPHER_ONLY, IDS_CERT_X509_KEY_USAGE_DECIPHER_ONLY},
};

crypto::ScopedOpenSSL<ASN1_BIT_STRING, ASN1_BIT_STRING_free> value(
crypto::ScopedOpenSSL<ASN1_BIT_STRING, ASN1_BIT_STRING_free>::Type value(
reinterpret_cast<ASN1_BIT_STRING*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand All @@ -498,7 +499,7 @@ std::string ProcessKeyUsageExtension(X509_EXTENSION* ex) {

std::string ProcessBasicConstraints(X509_EXTENSION* ex) {
std::string rv;
crypto::ScopedOpenSSL<BASIC_CONSTRAINTS, BASIC_CONSTRAINTS_free> value(
crypto::ScopedOpenSSL<BASIC_CONSTRAINTS, BASIC_CONSTRAINTS_free>::Type value(
reinterpret_cast<BASIC_CONSTRAINTS*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand All @@ -523,8 +524,8 @@ std::string ProcessBasicConstraints(X509_EXTENSION* ex) {

std::string ProcessExtKeyUsage(X509_EXTENSION* ex) {
std::string rv;
crypto::ScopedOpenSSL<EXTENDED_KEY_USAGE, EXTENDED_KEY_USAGE_free> value(
reinterpret_cast<EXTENDED_KEY_USAGE*>(X509V3_EXT_d2i(ex)));
crypto::ScopedOpenSSL<EXTENDED_KEY_USAGE, EXTENDED_KEY_USAGE_free>::Type
value(reinterpret_cast<EXTENDED_KEY_USAGE*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
for (int i = 0; i < sk_ASN1_OBJECT_num(value.get()); i++) {
Expand Down Expand Up @@ -672,7 +673,7 @@ std::string ProcessGeneralNames(GENERAL_NAMES* names) {
}

std::string ProcessAltName(X509_EXTENSION* ex) {
crypto::ScopedOpenSSL<GENERAL_NAMES, GENERAL_NAMES_free> alt_names(
crypto::ScopedOpenSSL<GENERAL_NAMES, GENERAL_NAMES_free>::Type alt_names(
reinterpret_cast<GENERAL_NAMES*>(X509V3_EXT_d2i(ex)));
if (!alt_names.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand All @@ -681,7 +682,7 @@ std::string ProcessAltName(X509_EXTENSION* ex) {
}

std::string ProcessSubjectKeyId(X509_EXTENSION* ex) {
crypto::ScopedOpenSSL<ASN1_OCTET_STRING, ASN1_OCTET_STRING_free> value(
crypto::ScopedOpenSSL<ASN1_OCTET_STRING, ASN1_OCTET_STRING_free>::Type value(
reinterpret_cast<ASN1_OCTET_STRING*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand All @@ -693,7 +694,7 @@ std::string ProcessSubjectKeyId(X509_EXTENSION* ex) {

std::string ProcessAuthKeyId(X509_EXTENSION* ex) {
std::string rv;
crypto::ScopedOpenSSL<AUTHORITY_KEYID, AUTHORITY_KEYID_free> value(
crypto::ScopedOpenSSL<AUTHORITY_KEYID, AUTHORITY_KEYID_free>::Type value(
reinterpret_cast<AUTHORITY_KEYID*>(X509V3_EXT_d2i(ex)));
if (!value.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand Down Expand Up @@ -749,8 +750,8 @@ std::string ProcessUserNotice(USERNOTICE* notice) {

std::string ProcessCertificatePolicies(X509_EXTENSION* ex) {
std::string rv;
crypto::ScopedOpenSSL<CERTIFICATEPOLICIES, CERTIFICATEPOLICIES_free> policies(
reinterpret_cast<CERTIFICATEPOLICIES*>(X509V3_EXT_d2i(ex)));
crypto::ScopedOpenSSL<CERTIFICATEPOLICIES, CERTIFICATEPOLICIES_free>::Type
policies(reinterpret_cast<CERTIFICATEPOLICIES*>(X509V3_EXT_d2i(ex)));

if (!policies.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand Down Expand Up @@ -820,8 +821,8 @@ std::string ProcessCrlDistPoints(X509_EXTENSION* ex) {
const int kDistPointRelativeName = 1;

std::string rv;
crypto::ScopedOpenSSL<CRL_DIST_POINTS, CRL_DIST_POINTS_free> dist_points(
reinterpret_cast<CRL_DIST_POINTS*>(X509V3_EXT_d2i(ex)));
crypto::ScopedOpenSSL<CRL_DIST_POINTS, CRL_DIST_POINTS_free>::Type
dist_points(reinterpret_cast<CRL_DIST_POINTS*>(X509V3_EXT_d2i(ex)));

if (!dist_points.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand Down Expand Up @@ -861,8 +862,8 @@ std::string ProcessCrlDistPoints(X509_EXTENSION* ex) {

std::string ProcessAuthInfoAccess(X509_EXTENSION* ex) {
std::string rv;
crypto::ScopedOpenSSL<AUTHORITY_INFO_ACCESS, AUTHORITY_INFO_ACCESS_free> aia(
reinterpret_cast<AUTHORITY_INFO_ACCESS*>(X509V3_EXT_d2i(ex)));
crypto::ScopedOpenSSL<AUTHORITY_INFO_ACCESS, AUTHORITY_INFO_ACCESS_free>::Type
aia(reinterpret_cast<AUTHORITY_INFO_ACCESS*>(X509V3_EXT_d2i(ex)));

if (!aia.get())
return l10n_util::GetStringUTF8(IDS_CERT_EXTENSION_DUMP_ERROR);
Expand Down Expand Up @@ -894,7 +895,7 @@ std::string ProcessAuthInfoAccess(X509_EXTENSION* ex) {

std::string ProcessIA5StringData(ASN1_OCTET_STRING* asn1_string) {
const unsigned char* data = ASN1_STRING_data(asn1_string);
crypto::ScopedOpenSSL<ASN1_IA5STRING, ASN1_IA5STRING_free> ia5_string(
crypto::ScopedOpenSSL<ASN1_IA5STRING, ASN1_IA5STRING_free>::Type ia5_string(
d2i_ASN1_IA5STRING(NULL, &data, ASN1_STRING_length(asn1_string)));

if (!ia5_string.get())
Expand All @@ -907,7 +908,7 @@ std::string ProcessIA5StringData(ASN1_OCTET_STRING* asn1_string) {

std::string ProcessBMPStringData(ASN1_OCTET_STRING* asn1_string) {
const unsigned char* data = ASN1_STRING_data(asn1_string);
crypto::ScopedOpenSSL<ASN1_BMPSTRING, ASN1_BMPSTRING_free> bmp_string(
crypto::ScopedOpenSSL<ASN1_BMPSTRING, ASN1_BMPSTRING_free>::Type bmp_string(
d2i_ASN1_BMPSTRING(NULL, &data, ASN1_STRING_length(asn1_string)));

if (!bmp_string.get())
Expand Down Expand Up @@ -970,7 +971,7 @@ std::string GetCertNameOrNickname(X509Certificate::OSCertHandle cert_handle) {
if (!name.empty())
return name;

crypto::ScopedOpenSSL<BIO, BIO_free_all> bio(crypto::BIO_new_string(&name));
crypto::ScopedBIO bio(crypto::BIO_new_string(&name));
if (!bio.get())
return name;
X509_NAME_print_ex(bio.get(),
Expand Down Expand Up @@ -1080,7 +1081,7 @@ std::string GetTitle(net::X509Certificate::OSCertHandle cert_handle) {
if (!title.empty())
return title;

crypto::ScopedOpenSSL<BIO, BIO_free_all> bio(crypto::BIO_new_string(&title));
crypto::ScopedBIO bio(crypto::BIO_new_string(&title));
if (!bio.get())
return title;
X509_NAME_print_ex(bio.get(),
Expand Down Expand Up @@ -1150,7 +1151,7 @@ void DestroyCertChain(net::X509Certificate::OSCertHandles* cert_handles) {
std::string GetCMSString(const net::X509Certificate::OSCertHandles& cert_chain,
size_t start, size_t end) {
std::string rv;
crypto::ScopedOpenSSL<PKCS7, PKCS7_free> p7(PKCS7_new());
crypto::ScopedOpenSSL<PKCS7, PKCS7_free>::Type p7(PKCS7_new());
if (!p7.get())
return rv;
if (!PKCS7_set_type(p7.get(), NID_pkcs7_signed))
Expand All @@ -1161,7 +1162,8 @@ std::string GetCMSString(const net::X509Certificate::OSCertHandles& cert_chain,
return rv;
}

crypto::ScopedOpenSSL<BIO, BIO_free_all> bio(crypto::BIO_new_string(&rv));
crypto::ScopedOpenSSL<BIO, BIO_free_all>::Type bio(
crypto::BIO_new_string(&rv));
if (!bio.get())
return rv;

Expand Down Expand Up @@ -1192,13 +1194,13 @@ std::string ProcessSecAlgorithmSignatureWrap(
std::string ProcessSubjectPublicKeyInfo(
net::X509Certificate::OSCertHandle cert_handle) {
std::string rv;
crypto::ScopedOpenSSL<EVP_PKEY, EVP_PKEY_free> public_key(
crypto::ScopedOpenSSL<EVP_PKEY, EVP_PKEY_free>::Type public_key(
X509_get_pubkey(cert_handle));
if (!public_key.get())
return rv;
switch (EVP_PKEY_type(public_key.get()->type)) {
case EVP_PKEY_RSA: {
crypto::ScopedOpenSSL<RSA, RSA_free> rsa_key(
crypto::ScopedOpenSSL<RSA, RSA_free>::Type rsa_key(
EVP_PKEY_get1_RSA(public_key.get()));
if (!rsa_key.get())
return rv;
Expand Down
8 changes: 5 additions & 3 deletions content/child/webcrypto/platform_crypto_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "content/child/webcrypto/status.h"
#include "content/child/webcrypto/webcrypto_util.h"
#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"
#include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
#include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
#include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
Expand Down Expand Up @@ -99,7 +100,7 @@ Status AesCbcEncryptDecrypt(EncryptOrDecrypt mode,
}

// Note: PKCS padding is enabled by default
crypto::ScopedOpenSSL<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free> context(
crypto::ScopedOpenSSL<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free>::Type context(
EVP_CIPHER_CTX_new());

if (!context.get())
Expand Down Expand Up @@ -233,7 +234,7 @@ class DigestorOpenSSL : public blink::WebCryptoDigestor {
}

bool initialized_;
crypto::ScopedOpenSSL<EVP_MD_CTX, EVP_MD_CTX_destroy> digest_context_;
crypto::ScopedEVP_MD_CTX digest_context_;
blink::WebCryptoAlgorithmId algorithm_id_;
unsigned char result_[EVP_MAX_MD_SIZE];
};
Expand Down Expand Up @@ -435,7 +436,8 @@ Status EncryptDecryptAesGcm(EncryptOrDecrypt mode,
return Status::OperationError();
}

crypto::ScopedOpenSSL<EVP_AEAD_CTX, EVP_AEAD_CTX_cleanup> ctx_cleanup(&ctx);
crypto::ScopedOpenSSL<EVP_AEAD_CTX, EVP_AEAD_CTX_cleanup>::Type ctx_cleanup(
&ctx);

ssize_t len;

Expand Down
46 changes: 22 additions & 24 deletions crypto/ec_private_key_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"

namespace crypto {

Expand All @@ -24,14 +25,18 @@ namespace {
// style guide, hence the unusual parameter placement / types.
typedef int (*ExportBioFunction)(BIO* bio, const void* key);

typedef ScopedOpenSSL<PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO_free>::Type
ScopedPKCS8_PRIV_KEY_INFO;
typedef ScopedOpenSSL<X509_SIG, X509_SIG_free>::Type ScopedX509_SIG;

// Helper to export |key| into |output| via the specified ExportBioFunction.
bool ExportKeyWithBio(const void* key,
ExportBioFunction export_fn,
std::vector<uint8>* output) {
if (!key)
return false;

ScopedOpenSSL<BIO, BIO_free_all> bio(BIO_new(BIO_s_mem()));
ScopedBIO bio(BIO_new(BIO_s_mem()));
if (!bio.get())
return false;

Expand Down Expand Up @@ -87,8 +92,7 @@ bool ECPrivateKey::IsSupported() { return true; }
ECPrivateKey* ECPrivateKey::Create() {
OpenSSLErrStackTracer err_tracer(FROM_HERE);

ScopedOpenSSL<EC_KEY, EC_KEY_free> ec_key(
EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
ScopedEC_KEY ec_key(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
if (!ec_key.get() || !EC_KEY_generate_key(ec_key.get()))
return NULL;

Expand Down Expand Up @@ -118,21 +122,17 @@ ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
const_cast<uint8*>(&encrypted_private_key_info[0]));
int private_key_data_len =
static_cast<int>(encrypted_private_key_info.size());
ScopedOpenSSL<BIO, BIO_free_all> bio(
BIO_new_mem_buf(private_key_data, private_key_data_len));
ScopedBIO bio(BIO_new_mem_buf(private_key_data, private_key_data_len));
if (!bio.get())
return NULL;

// Convert it, then decrypt it into a PKCS#8 object.
ScopedOpenSSL<X509_SIG, X509_SIG_free> p8_encrypted(
d2i_PKCS8_bio(bio.get(), NULL));
ScopedX509_SIG p8_encrypted(d2i_PKCS8_bio(bio.get(), NULL));
if (!p8_encrypted.get())
return NULL;

ScopedOpenSSL<PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO_free> p8_decrypted(
PKCS8_decrypt(p8_encrypted.get(),
password.c_str(),
static_cast<int>(password.size())));
ScopedPKCS8_PRIV_KEY_INFO p8_decrypted(PKCS8_decrypt(
p8_encrypted.get(), password.c_str(), static_cast<int>(password.size())));
if (!p8_decrypted.get() && password.empty()) {
// Hack for reading keys generated by ec_private_key_nss. Passing NULL
// causes OpenSSL to use an empty password instead of "\0\0".
Expand All @@ -156,24 +156,22 @@ bool ECPrivateKey::ExportEncryptedPrivateKey(
std::vector<uint8>* output) {
OpenSSLErrStackTracer err_tracer(FROM_HERE);
// Convert into a PKCS#8 object.
ScopedOpenSSL<PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO_free> pkcs8(
EVP_PKEY2PKCS8(key_));
ScopedPKCS8_PRIV_KEY_INFO pkcs8(EVP_PKEY2PKCS8(key_));
if (!pkcs8.get())
return false;

// Encrypt the object.
// NOTE: NSS uses SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC
// so use NID_pbe_WithSHA1And3_Key_TripleDES_CBC which should be the OpenSSL
// equivalent.
ScopedOpenSSL<X509_SIG, X509_SIG_free> encrypted(
PKCS8_encrypt(NID_pbe_WithSHA1And3_Key_TripleDES_CBC,
NULL,
password.c_str(),
static_cast<int>(password.size()),
NULL,
0,
iterations,
pkcs8.get()));
ScopedX509_SIG encrypted(PKCS8_encrypt(NID_pbe_WithSHA1And3_Key_TripleDES_CBC,
NULL,
password.c_str(),
static_cast<int>(password.size()),
NULL,
0,
iterations,
pkcs8.get()));
if (!encrypted.get())
return false;

Expand Down Expand Up @@ -211,15 +209,15 @@ bool ECPrivateKey::ExportRawPublicKey(std::string* output) {

bool ECPrivateKey::ExportValue(std::vector<uint8>* output) {
OpenSSLErrStackTracer err_tracer(FROM_HERE);
ScopedOpenSSL<EC_KEY, EC_KEY_free> ec_key(EVP_PKEY_get1_EC_KEY(key_));
ScopedEC_KEY ec_key(EVP_PKEY_get1_EC_KEY(key_));
return ExportKey(ec_key.get(),
reinterpret_cast<ExportDataFunction>(i2d_ECPrivateKey),
output);
}

bool ECPrivateKey::ExportECParams(std::vector<uint8>* output) {
OpenSSLErrStackTracer err_tracer(FROM_HERE);
ScopedOpenSSL<EC_KEY, EC_KEY_free> ec_key(EVP_PKEY_get1_EC_KEY(key_));
ScopedEC_KEY ec_key(EVP_PKEY_get1_EC_KEY(key_));
return ExportKey(ec_key.get(),
reinterpret_cast<ExportDataFunction>(i2d_ECParameters),
output);
Expand Down
11 changes: 9 additions & 2 deletions crypto/ec_signature_creator_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@
#include "base/logging.h"
#include "crypto/ec_private_key.h"
#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"

namespace crypto {

namespace {

typedef ScopedOpenSSL<ECDSA_SIG, ECDSA_SIG_free>::Type ScopedECDSA_SIG;

} // namespace

ECSignatureCreatorImpl::ECSignatureCreatorImpl(ECPrivateKey* key)
: key_(key), signature_len_(0) {
EnsureOpenSSLInit();
Expand All @@ -27,7 +34,7 @@ bool ECSignatureCreatorImpl::Sign(const uint8* data,
int data_len,
std::vector<uint8>* signature) {
OpenSSLErrStackTracer err_tracer(FROM_HERE);
ScopedOpenSSL<EVP_MD_CTX, EVP_MD_CTX_destroy> ctx(EVP_MD_CTX_create());
ScopedEVP_MD_CTX ctx(EVP_MD_CTX_create());
size_t sig_len = 0;
if (!ctx.get() ||
!EVP_DigestSignInit(ctx.get(), NULL, EVP_sha256(), NULL, key_->key()) ||
Expand All @@ -52,7 +59,7 @@ bool ECSignatureCreatorImpl::DecodeSignature(const std::vector<uint8>& der_sig,
OpenSSLErrStackTracer err_tracer(FROM_HERE);
// Create ECDSA_SIG object from DER-encoded data.
const unsigned char* der_data = &der_sig.front();
ScopedOpenSSL<ECDSA_SIG, ECDSA_SIG_free> ecdsa_sig(
ScopedECDSA_SIG ecdsa_sig(
d2i_ECDSA_SIG(NULL, &der_data, static_cast<long>(der_sig.size())));
if (!ecdsa_sig.get())
return false;
Expand Down
10 changes: 7 additions & 3 deletions crypto/openssl_bio_string_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@

#include <openssl/bio.h>

#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace crypto {

TEST(OpenSSLBIOString, TestWrite) {
std::string s;
const std::string expected1("a one\nb 2\n");
const std::string expected2("c d e f");
const std::string expected3("g h i");
{
crypto::ScopedOpenSSL<BIO, BIO_free_all> bio(crypto::BIO_new_string(&s));
ScopedBIO bio(BIO_new_string(&s));
ASSERT_TRUE(bio.get());

EXPECT_EQ(static_cast<int>(expected1.size()),
Expand Down Expand Up @@ -48,7 +50,7 @@ TEST(OpenSSLBIOString, TestReset) {
const std::string expected1("a b c\n");
const std::string expected2("d e f g\n");
{
crypto::ScopedOpenSSL<BIO, BIO_free_all> bio(crypto::BIO_new_string(&s));
ScopedBIO bio(BIO_new_string(&s));
ASSERT_TRUE(bio.get());

EXPECT_EQ(static_cast<int>(expected1.size()),
Expand All @@ -64,3 +66,5 @@ TEST(OpenSSLBIOString, TestReset) {
}
EXPECT_EQ(expected2, s);
}

} // namespace crypto
Loading

0 comments on commit cd9b75b

Please sign in to comment.