From b5110de1b3a159dad898b0b0636bc7fb8f582faf Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Tue, 23 Oct 2018 23:31:43 -0700 Subject: Add sanity check when loading public keys for OTA package For RSA keys, check if it has a 2048 bits modulus, and its public exponent is 3 or 65537. For EC keys, check if the field size is 256 bits for its curve. Bug: 116655889 Test: unit tests pass Change-Id: I5c00f4d2b61c98c434f0b49db232155d5d0770ec --- tests/component/verifier_test.cpp | 32 ++++++++++++++++++++++++ verifier.cpp | 52 ++++++++++++++++++++++++++++++++++----- verifier.h | 6 +++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index d110c37e0..14b6060c3 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -30,6 +30,9 @@ #include #include #include +#include +#include +#include #include #include "common/test_constants.h" @@ -148,6 +151,35 @@ TEST(VerifierTest, LoadCertificateFromBuffer_sha256_ec256bits) { VerifyPackageWithSingleCertificate("otasigned_v5.zip", std::move(cert)); } +TEST(VerifierTest, LoadCertificateFromBuffer_check_rsa_keys) { + std::unique_ptr rsa(RSA_new()); + std::unique_ptr exponent(BN_new(), BN_free); + BN_set_word(exponent.get(), 3); + RSA_generate_key_ex(rsa.get(), 2048, exponent.get(), nullptr); + ASSERT_TRUE(CheckRSAKey(rsa)); + + // Exponent is expected to be 3 or 65537 + BN_set_word(exponent.get(), 17); + RSA_generate_key_ex(rsa.get(), 2048, exponent.get(), nullptr); + ASSERT_FALSE(CheckRSAKey(rsa)); + + // Modulus is expected to be 2048. + BN_set_word(exponent.get(), 3); + RSA_generate_key_ex(rsa.get(), 1024, exponent.get(), nullptr); + ASSERT_FALSE(CheckRSAKey(rsa)); +} + +TEST(VerifierTest, LoadCertificateFromBuffer_check_ec_keys) { + std::unique_ptr ec(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)); + ASSERT_EQ(1, EC_KEY_generate_key(ec.get())); + ASSERT_TRUE(CheckECKey(ec)); + + // Expects 256-bit EC key with curve NIST P-256 + ec.reset(EC_KEY_new_by_curve_name(NID_secp224r1)); + ASSERT_EQ(1, EC_KEY_generate_key(ec.get())); + ASSERT_FALSE(CheckECKey(ec)); +} + TEST(VerifierTest, LoadKeysFromZipfile_empty_archive) { TemporaryFile otacerts; BuildCertificateArchive({}, otacerts.release()); diff --git a/verifier.cpp b/verifier.cpp index 2101dcb41..2dfc20808 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -500,6 +500,48 @@ std::vector LoadKeysFromZipfile(const std::string& zip_name) { return result; } +bool CheckRSAKey(const std::unique_ptr& rsa) { + if (!rsa) { + return false; + } + + const BIGNUM* out_n; + const BIGNUM* out_e; + RSA_get0_key(rsa.get(), &out_n, &out_e, nullptr /* private exponent */); + auto modulus_bits = BN_num_bits(out_n); + if (modulus_bits != 2048) { + LOG(ERROR) << "Modulus should be 2048 bits long, actual: " << modulus_bits; + return false; + } + + BN_ULONG exponent = BN_get_word(out_e); + if (exponent != 3 && exponent != 65537) { + LOG(ERROR) << "Public exponent should be 3 or 65537, actual: " << exponent; + return false; + } + + return true; +} + +bool CheckECKey(const std::unique_ptr& ec_key) { + if (!ec_key) { + return false; + } + + const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key.get()); + if (!ec_group) { + LOG(ERROR) << "Failed to get the ec_group from the ec_key"; + return false; + } + auto degree = EC_GROUP_get_degree(ec_group); + if (degree != 256) { + LOG(ERROR) << "Field size of the ec key should be 256 bits long, actual: " << degree; + return false; + } + + return true; +} + bool LoadCertificateFromBuffer(const std::vector& pem_content, Certificate* cert) { std::unique_ptr content( BIO_new_mem_buf(pem_content.data(), pem_content.size()), BIO_free); @@ -538,22 +580,20 @@ bool LoadCertificateFromBuffer(const std::vector& pem_content, Certific } int key_type = EVP_PKEY_id(public_key.get()); - // TODO(xunchang) check the rsa key has exponent 3 or 65537 with RSA_get0_key; and ec key is - // 256 bits. if (key_type == EVP_PKEY_RSA) { cert->key_type = Certificate::KEY_TYPE_RSA; cert->ec.reset(); cert->rsa.reset(EVP_PKEY_get1_RSA(public_key.get())); - if (!cert->rsa) { - LOG(ERROR) << "Failed to get the rsa key info from public key"; + if (!cert->rsa || !CheckRSAKey(cert->rsa)) { + LOG(ERROR) << "Failed to validate the rsa key info from public key"; return false; } } else if (key_type == EVP_PKEY_EC) { cert->key_type = Certificate::KEY_TYPE_EC; cert->rsa.reset(); cert->ec.reset(EVP_PKEY_get1_EC_KEY(public_key.get())); - if (!cert->ec) { - LOG(ERROR) << "Failed to get the ec key info from the public key"; + if (!cert->ec || !CheckECKey(cert->ec)) { + LOG(ERROR) << "Failed to validate the ec key info from the public key"; return false; } } else { diff --git a/verifier.h b/verifier.h index b7924c71f..944823293 100644 --- a/verifier.h +++ b/verifier.h @@ -72,6 +72,12 @@ int verify_file(const unsigned char* addr, size_t length, const std::vector& certs); +// Checks that the RSA key has a modulus of 2048 bits long, and public exponent is 3 or 65537. +bool CheckRSAKey(const std::unique_ptr& rsa); + +// Checks that the field size of the curve for the EC key is 256 bits. +bool CheckECKey(const std::unique_ptr& ec_key); + // Parses a PEM-encoded x509 certificate from the given buffer and saves it into |cert|. Returns // false if there is a parsing failure or the signature's encryption algorithm is not supported. bool LoadCertificateFromBuffer(const std::vector& pem_content, Certificate* cert); -- cgit v1.2.3