Use accessors for certificates and RSA keys.
The upstream RSA APIs are annoyingly tedious, but ah well. Note
X509_set1_signature_algo sets both copies of the signature algorithm.
This also fixes an EVP_PKEY leak in some error paths.
Test: mm
Change-Id: Ifa6f130e9d7dce328c649aa241057dbe5c0e5e66
diff --git a/keystore2/src/crypto/certificate_utils.cpp b/keystore2/src/crypto/certificate_utils.cpp
index 31c7fb4..d0508c8 100644
--- a/keystore2/src/crypto/certificate_utils.cpp
+++ b/keystore2/src/crypto/certificate_utils.cpp
@@ -19,6 +19,7 @@
#include <openssl/err.h>
#include <openssl/evp.h>
#include <openssl/mem.h>
+#include <openssl/ossl_typ.h>
#include <openssl/x509v3.h>
#include <functional>
@@ -512,10 +513,7 @@
return ASN1_STRING_Ptr(algo_str);
}
-CertUtilsError makeAndSetAlgo(X509_ALGOR* algo_field, Algo algo, Padding padding, Digest digest) {
- if (algo_field == nullptr) {
- return CertUtilsError::UnexpectedNullPointer;
- }
+std::variant<CertUtilsError, X509_ALGOR_Ptr> makeAlgo(Algo algo, Padding padding, Digest digest) {
ASN1_STRING_Ptr param;
int param_type = V_ASN1_UNDEF;
int nid = 0;
@@ -584,23 +582,29 @@
return CertUtilsError::InvalidArgument;
}
- if (!X509_ALGOR_set0(algo_field, OBJ_nid2obj(nid), param_type, param.get())) {
+ X509_ALGOR_Ptr result(X509_ALGOR_new());
+ if (!result) {
+ return CertUtilsError::MemoryAllocation;
+ }
+ if (!X509_ALGOR_set0(result.get(), OBJ_nid2obj(nid), param_type, param.get())) {
return CertUtilsError::Encoding;
}
// The X509 struct took ownership.
param.release();
- return CertUtilsError::Ok;
+ return result;
}
// This function allows for signing a
CertUtilsError signCertWith(X509* certificate,
std::function<std::vector<uint8_t>(const uint8_t*, size_t)> sign,
Algo algo, Padding padding, Digest digest) {
- if (auto error = makeAndSetAlgo(certificate->sig_alg, algo, padding, digest)) {
- return error;
+ auto algo_objV = makeAlgo(algo, padding, digest);
+ if (auto error = std::get_if<CertUtilsError>(&algo_objV)) {
+ return *error;
}
- if (auto error = makeAndSetAlgo(certificate->cert_info->signature, algo, padding, digest)) {
- return error;
+ auto& algo_obj = std::get<X509_ALGOR_Ptr>(algo_objV);
+ if (!X509_set1_signature_algo(certificate, algo_obj.get())) {
+ return CertUtilsError::BoringSsl;
}
uint8_t* cert_buf = nullptr;
@@ -615,13 +619,10 @@
return CertUtilsError::SignatureFailed;
}
- if (!ASN1_STRING_set(certificate->signature, signature.data(), signature.size())) {
+ if (!X509_set1_signature_value(certificate, signature.data(), signature.size())) {
return CertUtilsError::BoringSsl;
}
- certificate->signature->flags &= ~(0x07);
- certificate->signature->flags |= ASN1_STRING_FLAG_BITS_LEFT;
-
return CertUtilsError::Ok;
}
diff --git a/keystore2/src/crypto/include/certificate_utils.h b/keystore2/src/crypto/include/certificate_utils.h
index 6c25b9a..b43a830 100644
--- a/keystore2/src/crypto/include/certificate_utils.h
+++ b/keystore2/src/crypto/include/certificate_utils.h
@@ -39,6 +39,7 @@
DEFINE_OPENSSL_OBJECT_POINTER(ASN1_TIME);
DEFINE_OPENSSL_OBJECT_POINTER(EVP_PKEY);
DEFINE_OPENSSL_OBJECT_POINTER(X509);
+DEFINE_OPENSSL_OBJECT_POINTER(X509_ALGOR);
DEFINE_OPENSSL_OBJECT_POINTER(X509_EXTENSION);
DEFINE_OPENSSL_OBJECT_POINTER(X509_NAME);
DEFINE_OPENSSL_OBJECT_POINTER(EVP_PKEY_CTX);
diff --git a/ondevice-signing/CertUtils.cpp b/ondevice-signing/CertUtils.cpp
index b0b75a6..14dd6d0 100644
--- a/ondevice-signing/CertUtils.cpp
+++ b/ondevice-signing/CertUtils.cpp
@@ -21,6 +21,7 @@
#include <openssl/crypto.h>
#include <openssl/pkcs7.h>
#include <openssl/rsa.h>
+#include <openssl/x509.h>
#include <openssl/x509v3.h>
#include <fcntl.h>
@@ -56,12 +57,17 @@
}
Result<bssl::UniquePtr<RSA>> getRsa(const std::vector<uint8_t>& publicKey) {
+ bssl::UniquePtr<BIGNUM> n(BN_new());
+ bssl::UniquePtr<BIGNUM> e(BN_new());
bssl::UniquePtr<RSA> rsaPubkey(RSA_new());
- rsaPubkey->n = BN_new();
- rsaPubkey->e = BN_new();
-
- BN_bin2bn(publicKey.data(), publicKey.size(), rsaPubkey->n);
- BN_set_word(rsaPubkey->e, kRsaKeyExponent);
+ if (!n || !e || !rsaPubkey || !BN_bin2bn(publicKey.data(), publicKey.size(), n.get()) ||
+ !BN_set_word(e.get(), kRsaKeyExponent) ||
+ !RSA_set0_key(rsaPubkey.get(), n.get(), e.get(), /*d=*/nullptr)) {
+ return Error() << "Failed to create RSA key";
+ }
+ // RSA_set0_key takes ownership of |n| and |e| on success.
+ (void)n.release();
+ (void)e.release();
return rsaPubkey;
}
@@ -69,6 +75,9 @@
Result<void> verifySignature(const std::string& message, const std::string& signature,
const std::vector<uint8_t>& publicKey) {
auto rsaKey = getRsa(publicKey);
+ if (!rsaKey.ok()) {
+ return rsaKey.error();
+ }
uint8_t hashBuf[SHA256_DIGEST_LENGTH];
SHA256(const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(message.c_str())),
message.length(), hashBuf);
@@ -99,11 +108,14 @@
// "publicKey" corresponds to the raw public key bytes - need to create
// a new RSA key with the correct exponent.
auto rsaPubkey = getRsa(publicKey);
+ if (!rsaPubkey.ok()) {
+ return rsaPubkey.error();
+ }
- EVP_PKEY* public_key = EVP_PKEY_new();
- EVP_PKEY_assign_RSA(public_key, rsaPubkey->release());
+ bssl::UniquePtr<EVP_PKEY> public_key(EVP_PKEY_new());
+ EVP_PKEY_assign_RSA(public_key.get(), rsaPubkey->release());
- if (!X509_set_pubkey(x509.get(), public_key)) {
+ if (!X509_set_pubkey(x509.get(), public_key.get())) {
return Error() << "Unable to set x509 public key";
}
@@ -126,27 +138,30 @@
add_ext(x509.get(), NID_subject_key_identifier, kSubjectKeyIdentifier);
add_ext(x509.get(), NID_authority_key_identifier, "keyid:always");
- X509_ALGOR_set0(x509->cert_info->signature, OBJ_nid2obj(NID_sha256WithRSAEncryption),
- V_ASN1_NULL, NULL);
- X509_ALGOR_set0(x509->sig_alg, OBJ_nid2obj(NID_sha256WithRSAEncryption), V_ASN1_NULL, NULL);
+ bssl::UniquePtr<X509_ALGOR> algor(X509_ALGOR_new());
+ if (!algor ||
+ !X509_ALGOR_set0(algor.get(), OBJ_nid2obj(NID_sha256WithRSAEncryption), V_ASN1_NULL,
+ NULL) ||
+ !X509_set1_signature_algo(x509.get(), algor.get())) {
+ return Error() << "Unable to set x509 signature algorithm";
+ }
// Get the data to be signed
- char* to_be_signed_buf(nullptr);
- size_t to_be_signed_length = i2d_re_X509_tbs(x509.get(), (unsigned char**)&to_be_signed_buf);
+ unsigned char* to_be_signed_buf(nullptr);
+ size_t to_be_signed_length = i2d_re_X509_tbs(x509.get(), &to_be_signed_buf);
- auto signed_data = signFunction(std::string(to_be_signed_buf, to_be_signed_length));
+ auto signed_data = signFunction(
+ std::string(reinterpret_cast<const char*>(to_be_signed_buf), to_be_signed_length));
if (!signed_data.ok()) {
return signed_data.error();
}
- // This is the only part that doesn't use boringssl default functions - we manually copy in the
- // signature that was provided to us.
- x509->signature->data = (unsigned char*)OPENSSL_malloc(signed_data->size());
- memcpy(x509->signature->data, signed_data->c_str(), signed_data->size());
- x509->signature->length = signed_data->size();
+ if (!X509_set1_signature_value(x509.get(),
+ reinterpret_cast<const uint8_t*>(signed_data->data()),
+ signed_data->size())) {
+ return Error() << "Unable to set x509 signature";
+ }
- x509->signature->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07);
- x509->signature->flags |= ASN1_STRING_FLAG_BITS_LEFT;
auto f = fopen(path.c_str(), "wbe");
if (f == nullptr) {
return Error() << "Failed to open " << path;
@@ -154,7 +169,6 @@
i2d_X509_fp(f, x509.get());
fclose(f);
- EVP_PKEY_free(public_key);
return {};
}
@@ -163,15 +177,14 @@
return Error() << "Failed to extract public key from x509 cert";
}
- if (EVP_PKEY_type(pkey->type) != EVP_PKEY_RSA) {
+ if (EVP_PKEY_id(pkey) != EVP_PKEY_RSA) {
return Error() << "The public key is not an RSA key";
}
- RSA* rsa = EVP_PKEY_get1_RSA(pkey);
- auto num_bytes = BN_num_bytes(rsa->n);
+ RSA* rsa = EVP_PKEY_get0_RSA(pkey);
+ auto num_bytes = BN_num_bytes(RSA_get0_n(rsa));
std::vector<uint8_t> pubKey(num_bytes);
- int res = BN_bn2bin(rsa->n, pubKey.data());
- RSA_free(rsa);
+ int res = BN_bn2bin(RSA_get0_n(rsa), pubKey.data());
if (!res) {
return Error() << "Failed to convert public key to bytes";