From 40ea8bc1eb566585f8577e83124f84e5142b4871 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 19 Aug 2020 17:00:38 -0400 Subject: [PATCH] libcrypto_utils: Use accessors to manipulate RSA keys. This removes a blocker to BoringSSL marking the RSA struct opaque. Sadly, OpenSSL's RSA_set0_key setter is a little tedious to use correctly with error-handling, but it works. Along the way, replace the byte reversing code with BoringSSL's native little-endian BIGNUM conversion functions. Test: libcrypto_utils_test passes Change-Id: I298a6360d76fc7793462cd55bd57bc673c4975b3 --- libcrypto_utils/.clang-format | 1 + libcrypto_utils/android_pubkey.c | 58 +++++++++++++------------------- 2 files changed, 24 insertions(+), 35 deletions(-) create mode 120000 libcrypto_utils/.clang-format diff --git a/libcrypto_utils/.clang-format b/libcrypto_utils/.clang-format new file mode 120000 index 000000000..fd0645fdf --- /dev/null +++ b/libcrypto_utils/.clang-format @@ -0,0 +1 @@ +../.clang-format-2 \ No newline at end of file diff --git a/libcrypto_utils/android_pubkey.c b/libcrypto_utils/android_pubkey.c index 3052e52bf..188ffcbe1 100644 --- a/libcrypto_utils/android_pubkey.c +++ b/libcrypto_utils/android_pubkey.c @@ -45,27 +45,19 @@ typedef struct RSAPublicKey { // RSA modulus as a little-endian array. uint8_t modulus[ANDROID_PUBKEY_MODULUS_SIZE]; - // Montgomery parameter R^2 as a little-endian array of little-endian words. + // Montgomery parameter R^2 as a little-endian array. uint8_t rr[ANDROID_PUBKEY_MODULUS_SIZE]; // RSA modulus: 3 or 65537 uint32_t exponent; } RSAPublicKey; -// Reverses byte order in |buffer|. -static void reverse_bytes(uint8_t* buffer, size_t size) { - for (size_t i = 0; i < (size + 1) / 2; ++i) { - uint8_t tmp = buffer[i]; - buffer[i] = buffer[size - i - 1]; - buffer[size - i - 1] = tmp; - } -} - bool android_pubkey_decode(const uint8_t* key_buffer, size_t size, RSA** key) { const RSAPublicKey* key_struct = (RSAPublicKey*)key_buffer; bool ret = false; - uint8_t modulus_buffer[ANDROID_PUBKEY_MODULUS_SIZE]; RSA* new_key = RSA_new(); + BIGNUM* n = NULL; + BIGNUM* e = NULL; if (!new_key) { goto cleanup; } @@ -79,19 +71,24 @@ bool android_pubkey_decode(const uint8_t* key_buffer, size_t size, RSA** key) { } // Convert the modulus to big-endian byte order as expected by BN_bin2bn. - memcpy(modulus_buffer, key_struct->modulus, sizeof(modulus_buffer)); - reverse_bytes(modulus_buffer, sizeof(modulus_buffer)); - new_key->n = BN_bin2bn(modulus_buffer, sizeof(modulus_buffer), NULL); - if (!new_key->n) { + n = BN_le2bn(key_struct->modulus, ANDROID_PUBKEY_MODULUS_SIZE, NULL); + if (!n) { goto cleanup; } // Read the exponent. - new_key->e = BN_new(); - if (!new_key->e || !BN_set_word(new_key->e, key_struct->exponent)) { + e = BN_new(); + if (!e || !BN_set_word(e, key_struct->exponent)) { goto cleanup; } + if (!RSA_set0_key(new_key, n, e, NULL)) { + goto cleanup; + } + // RSA_set0_key takes ownership of its inputs on success. + n = NULL; + e = NULL; + // Note that we don't extract the montgomery parameters n0inv and rr from // the RSAPublicKey structure. They assume a word size of 32 bits, but // BoringSSL may use a word size of 64 bits internally, so we're lacking the @@ -101,24 +98,16 @@ bool android_pubkey_decode(const uint8_t* key_buffer, size_t size, RSA** key) { // pre-computed montgomery parameters. *key = new_key; + new_key = NULL; ret = true; cleanup: - if (!ret && new_key) { - RSA_free(new_key); - } + RSA_free(new_key); + BN_free(n); + BN_free(e); return ret; } -static bool android_pubkey_encode_bignum(const BIGNUM* num, uint8_t* buffer) { - if (!BN_bn2bin_padded(buffer, ANDROID_PUBKEY_MODULUS_SIZE, num)) { - return false; - } - - reverse_bytes(buffer, ANDROID_PUBKEY_MODULUS_SIZE); - return true; -} - bool android_pubkey_encode(const RSA* key, uint8_t* key_buffer, size_t size) { RSAPublicKey* key_struct = (RSAPublicKey*)key_buffer; bool ret = false; @@ -136,27 +125,26 @@ bool android_pubkey_encode(const RSA* key, uint8_t* key_buffer, size_t size) { key_struct->modulus_size_words = ANDROID_PUBKEY_MODULUS_SIZE_WORDS; // Compute and store n0inv = -1 / N[0] mod 2^32. - if (!ctx || !r32 || !n0inv || !BN_set_bit(r32, 32) || - !BN_mod(n0inv, key->n, r32, ctx) || + if (!ctx || !r32 || !n0inv || !BN_set_bit(r32, 32) || !BN_mod(n0inv, RSA_get0_n(key), r32, ctx) || !BN_mod_inverse(n0inv, n0inv, r32, ctx) || !BN_sub(n0inv, r32, n0inv)) { goto cleanup; } key_struct->n0inv = (uint32_t)BN_get_word(n0inv); // Store the modulus. - if (!android_pubkey_encode_bignum(key->n, key_struct->modulus)) { + if (!BN_bn2le_padded(key_struct->modulus, ANDROID_PUBKEY_MODULUS_SIZE, RSA_get0_n(key))) { goto cleanup; } // Compute and store rr = (2^(rsa_size)) ^ 2 mod N. if (!ctx || !rr || !BN_set_bit(rr, ANDROID_PUBKEY_MODULUS_SIZE * 8) || - !BN_mod_sqr(rr, rr, key->n, ctx) || - !android_pubkey_encode_bignum(rr, key_struct->rr)) { + !BN_mod_sqr(rr, rr, RSA_get0_n(key), ctx) || + !BN_bn2le_padded(key_struct->rr, ANDROID_PUBKEY_MODULUS_SIZE, rr)) { goto cleanup; } // Store the exponent. - key_struct->exponent = (uint32_t)BN_get_word(key->e); + key_struct->exponent = (uint32_t)BN_get_word(RSA_get0_e(key)); ret = true;