From 71e3e09ec2ac4f022e8f9213657746d8cad5dd97 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 2 Feb 2016 14:02:27 -0800 Subject: recovery: Refactor verifier and verifier_test. Move to using std::vector and std::unique_ptr to manage key certificates to stop memory leaks. Bug: 26908001 Change-Id: Ia5f799bc8dcc036a0ffae5eaa8d9f6e09abd031c --- verifier.cpp | 265 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 124 insertions(+), 141 deletions(-) (limited to 'verifier.cpp') diff --git a/verifier.cpp b/verifier.cpp index 61e5adf0b..9a2d60c66 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -113,7 +113,7 @@ static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_d // or no key matches the signature). int verify_file(unsigned char* addr, size_t length, - const Certificate* pKeys, unsigned int numKeys) { + const std::vector& keys) { ui->SetProgress(0.0); // An archive with a whole-file signature will end in six bytes: @@ -176,8 +176,7 @@ int verify_file(unsigned char* addr, size_t length, return VERIFY_FAILURE; } - size_t i; - for (i = 4; i < eocd_size-3; ++i) { + for (size_t i = 4; i < eocd_size-3; ++i) { if (eocd[i ] == 0x50 && eocd[i+1] == 0x4b && eocd[i+2] == 0x05 && eocd[i+3] == 0x06) { // if the sequence $50 $4b $05 $06 appears anywhere after @@ -193,8 +192,8 @@ int verify_file(unsigned char* addr, size_t length, bool need_sha1 = false; bool need_sha256 = false; - for (i = 0; i < numKeys; ++i) { - switch (pKeys[i].hash_len) { + for (const auto& key : keys) { + switch (key.hash_len) { case SHA_DIGEST_SIZE: need_sha1 = true; break; case SHA256_DIGEST_SIZE: need_sha256 = true; break; } @@ -225,7 +224,7 @@ int verify_file(unsigned char* addr, size_t length, const uint8_t* sha1 = SHA_final(&sha1_ctx); const uint8_t* sha256 = SHA256_final(&sha256_ctx); - uint8_t* sig_der = NULL; + uint8_t* sig_der = nullptr; size_t sig_der_length = 0; size_t signature_size = signature_start - FOOTER_SIZE; @@ -240,9 +239,10 @@ int verify_file(unsigned char* addr, size_t length, * any key can match, we need to try each before determining a verification * failure has happened. */ - for (i = 0; i < numKeys; ++i) { + size_t i = 0; + for (const auto& key : keys) { const uint8_t* hash; - switch (pKeys[i].hash_len) { + switch (key.hash_len) { case SHA_DIGEST_SIZE: hash = sha1; break; case SHA256_DIGEST_SIZE: hash = sha256; break; default: continue; @@ -250,15 +250,15 @@ int verify_file(unsigned char* addr, size_t length, // The 6 bytes is the "(signature_start) $ff $ff (comment_size)" that // the signing tool appends after the signature itself. - if (pKeys[i].key_type == Certificate::RSA) { + if (key.key_type == Certificate::RSA) { if (sig_der_length < RSANUMBYTES) { // "signature" block isn't big enough to contain an RSA block. LOGI("signature is too short for RSA key %zu\n", i); continue; } - if (!RSA_verify(pKeys[i].rsa, sig_der, RSANUMBYTES, - hash, pKeys[i].hash_len)) { + if (!RSA_verify(key.rsa.get(), sig_der, RSANUMBYTES, + hash, key.hash_len)) { LOGI("failed to verify against RSA key %zu\n", i); continue; } @@ -266,8 +266,8 @@ int verify_file(unsigned char* addr, size_t length, LOGI("whole-file signature verified against RSA key %zu\n", i); free(sig_der); return VERIFY_SUCCESS; - } else if (pKeys[i].key_type == Certificate::EC - && pKeys[i].hash_len == SHA256_DIGEST_SIZE) { + } else if (key.key_type == Certificate::EC + && key.hash_len == SHA256_DIGEST_SIZE) { p256_int r, s; if (!dsa_sig_unpack(sig_der, sig_der_length, &r, &s)) { LOGI("Not a DSA signature block for EC key %zu\n", i); @@ -276,7 +276,7 @@ int verify_file(unsigned char* addr, size_t length, p256_int p256_hash; p256_from_bin(hash, &p256_hash); - if (!p256_ecdsa_verify(&(pKeys[i].ec->x), &(pKeys[i].ec->y), + if (!p256_ecdsa_verify(&(key.ec->x), &(key.ec->y), &p256_hash, &r, &s)) { LOGI("failed to verify against EC key %zu\n", i); continue; @@ -286,8 +286,9 @@ int verify_file(unsigned char* addr, size_t length, free(sig_der); return VERIFY_SUCCESS; } else { - LOGI("Unknown key type %d\n", pKeys[i].key_type); + LOGI("Unknown key type %d\n", key.key_type); } + i++; } free(sig_der); LOGE("failed to verify whole-file signature\n"); @@ -323,140 +324,122 @@ int verify_file(unsigned char* addr, size_t length, // 4: 2048-bit RSA key with e=65537 and SHA-256 hash // 5: 256-bit EC key using the NIST P-256 curve parameters and SHA-256 hash // -// Returns NULL if the file failed to parse, or if it contain zero keys. -Certificate* -load_keys(const char* filename, int* numKeys) { - Certificate* out = NULL; - *numKeys = 0; - - FILE* f = fopen(filename, "r"); - if (f == NULL) { +// Returns true on success, and appends the found keys (at least one) to certs. +// Otherwise returns false if the file failed to parse, or if it contains zero +// keys. The contents in certs would be unspecified on failure. +bool load_keys(const char* filename, std::vector& certs) { + std::unique_ptr f(fopen(filename, "r"), fclose); + if (!f) { LOGE("opening %s: %s\n", filename, strerror(errno)); - goto exit; + return false; } - { - int i; - bool done = false; - while (!done) { - ++*numKeys; - out = (Certificate*)realloc(out, *numKeys * sizeof(Certificate)); - Certificate* cert = out + (*numKeys - 1); - memset(cert, '\0', sizeof(Certificate)); - - char start_char; - if (fscanf(f, " %c", &start_char) != 1) goto exit; - if (start_char == '{') { - // a version 1 key has no version specifier. - cert->key_type = Certificate::RSA; - cert->rsa = (RSAPublicKey*)malloc(sizeof(RSAPublicKey)); - cert->rsa->exponent = 3; - cert->hash_len = SHA_DIGEST_SIZE; - } else if (start_char == 'v') { - int version; - if (fscanf(f, "%d {", &version) != 1) goto exit; - switch (version) { - case 2: - cert->key_type = Certificate::RSA; - cert->rsa = (RSAPublicKey*)malloc(sizeof(RSAPublicKey)); - cert->rsa->exponent = 65537; - cert->hash_len = SHA_DIGEST_SIZE; - break; - case 3: - cert->key_type = Certificate::RSA; - cert->rsa = (RSAPublicKey*)malloc(sizeof(RSAPublicKey)); - cert->rsa->exponent = 3; - cert->hash_len = SHA256_DIGEST_SIZE; - break; - case 4: - cert->key_type = Certificate::RSA; - cert->rsa = (RSAPublicKey*)malloc(sizeof(RSAPublicKey)); - cert->rsa->exponent = 65537; - cert->hash_len = SHA256_DIGEST_SIZE; - break; - case 5: - cert->key_type = Certificate::EC; - cert->ec = (ECPublicKey*)calloc(1, sizeof(ECPublicKey)); - cert->hash_len = SHA256_DIGEST_SIZE; - break; - default: - goto exit; - } + while (true) { + certs.emplace_back(0, Certificate::RSA, nullptr, nullptr); + Certificate& cert = certs.back(); + + char start_char; + if (fscanf(f.get(), " %c", &start_char) != 1) return false; + if (start_char == '{') { + // a version 1 key has no version specifier. + cert.key_type = Certificate::RSA; + cert.rsa = std::unique_ptr(new RSAPublicKey); + cert.rsa->exponent = 3; + cert.hash_len = SHA_DIGEST_SIZE; + } else if (start_char == 'v') { + int version; + if (fscanf(f.get(), "%d {", &version) != 1) return false; + switch (version) { + case 2: + cert.key_type = Certificate::RSA; + cert.rsa = std::unique_ptr(new RSAPublicKey); + cert.rsa->exponent = 65537; + cert.hash_len = SHA_DIGEST_SIZE; + break; + case 3: + cert.key_type = Certificate::RSA; + cert.rsa = std::unique_ptr(new RSAPublicKey); + cert.rsa->exponent = 3; + cert.hash_len = SHA256_DIGEST_SIZE; + break; + case 4: + cert.key_type = Certificate::RSA; + cert.rsa = std::unique_ptr(new RSAPublicKey); + cert.rsa->exponent = 65537; + cert.hash_len = SHA256_DIGEST_SIZE; + break; + case 5: + cert.key_type = Certificate::EC; + cert.ec = std::unique_ptr(new ECPublicKey); + cert.hash_len = SHA256_DIGEST_SIZE; + break; + default: + return false; } + } - if (cert->key_type == Certificate::RSA) { - RSAPublicKey* key = cert->rsa; - if (fscanf(f, " %i , 0x%x , { %u", - &(key->len), &(key->n0inv), &(key->n[0])) != 3) { - goto exit; - } - if (key->len != RSANUMWORDS) { - LOGE("key length (%d) does not match expected size\n", key->len); - goto exit; - } - for (i = 1; i < key->len; ++i) { - if (fscanf(f, " , %u", &(key->n[i])) != 1) goto exit; - } - if (fscanf(f, " } , { %u", &(key->rr[0])) != 1) goto exit; - for (i = 1; i < key->len; ++i) { - if (fscanf(f, " , %u", &(key->rr[i])) != 1) goto exit; - } - fscanf(f, " } } "); - - LOGI("read key e=%d hash=%d\n", key->exponent, cert->hash_len); - } else if (cert->key_type == Certificate::EC) { - ECPublicKey* key = cert->ec; - int key_len; - unsigned int byte; - uint8_t x_bytes[P256_NBYTES]; - uint8_t y_bytes[P256_NBYTES]; - if (fscanf(f, " %i , { %u", &key_len, &byte) != 2) goto exit; - if (key_len != P256_NBYTES) { - LOGE("Key length (%d) does not match expected size %d\n", key_len, P256_NBYTES); - goto exit; - } - x_bytes[P256_NBYTES - 1] = byte; - for (i = P256_NBYTES - 2; i >= 0; --i) { - if (fscanf(f, " , %u", &byte) != 1) goto exit; - x_bytes[i] = byte; - } - if (fscanf(f, " } , { %u", &byte) != 1) goto exit; - y_bytes[P256_NBYTES - 1] = byte; - for (i = P256_NBYTES - 2; i >= 0; --i) { - if (fscanf(f, " , %u", &byte) != 1) goto exit; - y_bytes[i] = byte; - } - fscanf(f, " } } "); - p256_from_bin(x_bytes, &key->x); - p256_from_bin(y_bytes, &key->y); - } else { - LOGE("Unknown key type %d\n", cert->key_type); - goto exit; + if (cert.key_type == Certificate::RSA) { + RSAPublicKey* key = cert.rsa.get(); + if (fscanf(f.get(), " %i , 0x%x , { %u", &(key->len), &(key->n0inv), + &(key->n[0])) != 3) { + return false; } - - // if the line ends in a comma, this file has more keys. - switch (fgetc(f)) { - case ',': - // more keys to come. - break; - - case EOF: - done = true; - break; - - default: - LOGE("unexpected character between keys\n"); - goto exit; + if (key->len != RSANUMWORDS) { + LOGE("key length (%d) does not match expected size\n", key->len); + return false; + } + for (int i = 1; i < key->len; ++i) { + if (fscanf(f.get(), " , %u", &(key->n[i])) != 1) return false; + } + if (fscanf(f.get(), " } , { %u", &(key->rr[0])) != 1) return false; + for (int i = 1; i < key->len; ++i) { + if (fscanf(f.get(), " , %u", &(key->rr[i])) != 1) return false; + } + fscanf(f.get(), " } } "); + + LOGI("read key e=%d hash=%d\n", key->exponent, cert.hash_len); + } else if (cert.key_type == Certificate::EC) { + ECPublicKey* key = cert.ec.get(); + int key_len; + unsigned int byte; + uint8_t x_bytes[P256_NBYTES]; + uint8_t y_bytes[P256_NBYTES]; + if (fscanf(f.get(), " %i , { %u", &key_len, &byte) != 2) return false; + if (key_len != P256_NBYTES) { + LOGE("Key length (%d) does not match expected size %d\n", key_len, P256_NBYTES); + return false; + } + x_bytes[P256_NBYTES - 1] = byte; + for (int i = P256_NBYTES - 2; i >= 0; --i) { + if (fscanf(f.get(), " , %u", &byte) != 1) return false; + x_bytes[i] = byte; + } + if (fscanf(f.get(), " } , { %u", &byte) != 1) return false; + y_bytes[P256_NBYTES - 1] = byte; + for (int i = P256_NBYTES - 2; i >= 0; --i) { + if (fscanf(f.get(), " , %u", &byte) != 1) return false; + y_bytes[i] = byte; } + fscanf(f.get(), " } } "); + p256_from_bin(x_bytes, &key->x); + p256_from_bin(y_bytes, &key->y); + } else { + LOGE("Unknown key type %d\n", cert.key_type); + return false; } - } - fclose(f); - return out; + // if the line ends in a comma, this file has more keys. + int ch = fgetc(f.get()); + if (ch == ',') { + // more keys to come. + continue; + } else if (ch == EOF) { + break; + } else { + LOGE("unexpected character between keys\n"); + return false; + } + } -exit: - if (f) fclose(f); - free(out); - *numKeys = 0; - return NULL; + return true; } -- cgit v1.2.3 From 8febafa67e93b2159804b1130a41f15b009de1cd Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 13 Apr 2016 16:39:56 -0700 Subject: Use BoringSSL instead of mincrypt to speed up package verification. This changes the verification code in bootable/recovery to use BoringSSL instead of mincrypt. Cherry-pick of 452df6d99c81c4eeee3d2c7b2171901e8b7bc54a, with merge conflict resolution, extra logging in verifier.cpp, and an increase in the hash chunk size from 4KiB to 1MiB. Bug: http://b/28135231 Change-Id: I1ed7efd52223dd6f6a4629cad187cbc383d5aa84 --- verifier.cpp | 320 ++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 209 insertions(+), 111 deletions(-) (limited to 'verifier.cpp') diff --git a/verifier.cpp b/verifier.cpp index 9a2d60c66..4004b0228 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -14,25 +14,26 @@ * limitations under the License. */ -#include "asn1_decoder.h" -#include "common.h" -#include "ui.h" -#include "verifier.h" - -#include "mincrypt/dsa_sig.h" -#include "mincrypt/p256.h" -#include "mincrypt/p256_ecdsa.h" -#include "mincrypt/rsa.h" -#include "mincrypt/sha.h" -#include "mincrypt/sha256.h" - #include #include #include #include +#include +#include + +#include +#include + +#include "asn1_decoder.h" +#include "common.h" +#include "ui.h" +#include "verifier.h" + extern RecoveryUI* ui; +static constexpr size_t MiB = 1024 * 1024; + /* * Simple version of PKCS#7 SignedData extraction. This extracts the * signature OCTET STRING to be used for signature verification. @@ -188,30 +189,30 @@ int verify_file(unsigned char* addr, size_t length, } } -#define BUFFER_SIZE 4096 - bool need_sha1 = false; bool need_sha256 = false; for (const auto& key : keys) { switch (key.hash_len) { - case SHA_DIGEST_SIZE: need_sha1 = true; break; - case SHA256_DIGEST_SIZE: need_sha256 = true; break; + case SHA_DIGEST_LENGTH: need_sha1 = true; break; + case SHA256_DIGEST_LENGTH: need_sha256 = true; break; } } SHA_CTX sha1_ctx; SHA256_CTX sha256_ctx; - SHA_init(&sha1_ctx); - SHA256_init(&sha256_ctx); + SHA1_Init(&sha1_ctx); + SHA256_Init(&sha256_ctx); double frac = -1.0; size_t so_far = 0; while (so_far < signed_len) { - size_t size = signed_len - so_far; - if (size > BUFFER_SIZE) size = BUFFER_SIZE; + // On a Nexus 9, experiment didn't show any performance improvement with + // larger sizes past 1MiB, and they reduce the granularity of the progress + // bar. http://b/28135231. + size_t size = std::min(signed_len - so_far, 1 * MiB); - if (need_sha1) SHA_update(&sha1_ctx, addr + so_far, size); - if (need_sha256) SHA256_update(&sha256_ctx, addr + so_far, size); + if (need_sha1) SHA1_Update(&sha1_ctx, addr + so_far, size); + if (need_sha256) SHA256_Update(&sha256_ctx, addr + so_far, size); so_far += size; double f = so_far / (double)signed_len; @@ -221,8 +222,10 @@ int verify_file(unsigned char* addr, size_t length, } } - const uint8_t* sha1 = SHA_final(&sha1_ctx); - const uint8_t* sha256 = SHA256_final(&sha256_ctx); + uint8_t sha1[SHA_DIGEST_LENGTH]; + SHA1_Final(sha1, &sha1_ctx); + uint8_t sha256[SHA256_DIGEST_LENGTH]; + SHA256_Final(sha256, &sha256_ctx); uint8_t* sig_der = nullptr; size_t sig_der_length = 0; @@ -242,23 +245,25 @@ int verify_file(unsigned char* addr, size_t length, size_t i = 0; for (const auto& key : keys) { const uint8_t* hash; + int hash_nid; switch (key.hash_len) { - case SHA_DIGEST_SIZE: hash = sha1; break; - case SHA256_DIGEST_SIZE: hash = sha256; break; - default: continue; + case SHA_DIGEST_LENGTH: + hash = sha1; + hash_nid = NID_sha1; + break; + case SHA256_DIGEST_LENGTH: + hash = sha256; + hash_nid = NID_sha256; + break; + default: + continue; } // The 6 bytes is the "(signature_start) $ff $ff (comment_size)" that // the signing tool appends after the signature itself. - if (key.key_type == Certificate::RSA) { - if (sig_der_length < RSANUMBYTES) { - // "signature" block isn't big enough to contain an RSA block. - LOGI("signature is too short for RSA key %zu\n", i); - continue; - } - - if (!RSA_verify(key.rsa.get(), sig_der, RSANUMBYTES, - hash, key.hash_len)) { + if (key.key_type == Certificate::KEY_TYPE_RSA) { + if (!RSA_verify(hash_nid, hash, key.hash_len, sig_der, + sig_der_length, key.rsa.get())) { LOGI("failed to verify against RSA key %zu\n", i); continue; } @@ -266,18 +271,10 @@ int verify_file(unsigned char* addr, size_t length, LOGI("whole-file signature verified against RSA key %zu\n", i); free(sig_der); return VERIFY_SUCCESS; - } else if (key.key_type == Certificate::EC - && key.hash_len == SHA256_DIGEST_SIZE) { - p256_int r, s; - if (!dsa_sig_unpack(sig_der, sig_der_length, &r, &s)) { - LOGI("Not a DSA signature block for EC key %zu\n", i); - continue; - } - - p256_int p256_hash; - p256_from_bin(hash, &p256_hash); - if (!p256_ecdsa_verify(&(key.ec->x), &(key.ec->y), - &p256_hash, &r, &s)) { + } else if (key.key_type == Certificate::KEY_TYPE_EC + && key.hash_len == SHA256_DIGEST_LENGTH) { + if (!ECDSA_verify(0, hash, key.hash_len, sig_der, + sig_der_length, key.ec.get())) { LOGI("failed to verify against EC key %zu\n", i); continue; } @@ -295,6 +292,144 @@ int verify_file(unsigned char* addr, size_t length, return VERIFY_FAILURE; } +std::unique_ptr parse_rsa_key(FILE* file, uint32_t exponent) { + // Read key length in words and n0inv. n0inv is a precomputed montgomery + // parameter derived from the modulus and can be used to speed up + // verification. n0inv is 32 bits wide here, assuming the verification logic + // uses 32 bit arithmetic. However, BoringSSL may use a word size of 64 bits + // internally, in which case we don't have a valid n0inv. Thus, we just + // ignore the montgomery parameters and have BoringSSL recompute them + // internally. If/When the speedup from using the montgomery parameters + // becomes relevant, we can add more sophisticated code here to obtain a + // 64-bit n0inv and initialize the montgomery parameters in the key object. + uint32_t key_len_words = 0; + uint32_t n0inv = 0; + if (fscanf(file, " %i , 0x%x", &key_len_words, &n0inv) != 2) { + return nullptr; + } + + if (key_len_words > 8192 / 32) { + LOGE("key length (%d) too large\n", key_len_words); + return nullptr; + } + + // Read the modulus. + std::unique_ptr modulus(new uint32_t[key_len_words]); + if (fscanf(file, " , { %u", &modulus[0]) != 1) { + return nullptr; + } + for (uint32_t i = 1; i < key_len_words; ++i) { + if (fscanf(file, " , %u", &modulus[i]) != 1) { + return nullptr; + } + } + + // Cconvert from little-endian array of little-endian words to big-endian + // byte array suitable as input for BN_bin2bn. + std::reverse((uint8_t*)modulus.get(), + (uint8_t*)(modulus.get() + key_len_words)); + + // The next sequence of values is the montgomery parameter R^2. Since we + // generally don't have a valid |n0inv|, we ignore this (see comment above). + uint32_t rr_value; + if (fscanf(file, " } , { %u", &rr_value) != 1) { + return nullptr; + } + for (uint32_t i = 1; i < key_len_words; ++i) { + if (fscanf(file, " , %u", &rr_value) != 1) { + return nullptr; + } + } + if (fscanf(file, " } } ") != 0) { + return nullptr; + } + + // Initialize the key. + std::unique_ptr key(RSA_new()); + if (!key) { + return nullptr; + } + + key->n = BN_bin2bn((uint8_t*)modulus.get(), + key_len_words * sizeof(uint32_t), NULL); + if (!key->n) { + return nullptr; + } + + key->e = BN_new(); + if (!key->e || !BN_set_word(key->e, exponent)) { + return nullptr; + } + + return key; +} + +struct BNDeleter { + void operator()(BIGNUM* bn) { + BN_free(bn); + } +}; + +std::unique_ptr parse_ec_key(FILE* file) { + uint32_t key_len_bytes = 0; + if (fscanf(file, " %i", &key_len_bytes) != 1) { + return nullptr; + } + + std::unique_ptr group( + EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1), EC_GROUP_free); + if (!group) { + return nullptr; + } + + // Verify that |key_len| matches the group order. + if (key_len_bytes != BN_num_bytes(EC_GROUP_get0_order(group.get()))) { + return nullptr; + } + + // Read the public key coordinates. Note that the byte order in the file is + // little-endian, so we convert to big-endian here. + std::unique_ptr bytes(new uint8_t[key_len_bytes]); + std::unique_ptr point[2]; + for (int i = 0; i < 2; ++i) { + unsigned int byte = 0; + if (fscanf(file, " , { %u", &byte) != 1) { + return nullptr; + } + bytes[key_len_bytes - 1] = byte; + + for (size_t i = 1; i < key_len_bytes; ++i) { + if (fscanf(file, " , %u", &byte) != 1) { + return nullptr; + } + bytes[key_len_bytes - i - 1] = byte; + } + + point[i].reset(BN_bin2bn(bytes.get(), key_len_bytes, nullptr)); + if (!point[i]) { + return nullptr; + } + + if (fscanf(file, " }") != 0) { + return nullptr; + } + } + + if (fscanf(file, " } ") != 0) { + return nullptr; + } + + // Create and initialize the key. + std::unique_ptr key(EC_KEY_new()); + if (!key || !EC_KEY_set_group(key.get(), group.get()) || + !EC_KEY_set_public_key_affine_coordinates(key.get(), point[0].get(), + point[1].get())) { + return nullptr; + } + + return key; +} + // Reads a file containing one or more public keys as produced by // DumpPublicKey: this is an RSAPublicKey struct as it would appear // as a C source literal, eg: @@ -335,94 +470,57 @@ bool load_keys(const char* filename, std::vector& certs) { } while (true) { - certs.emplace_back(0, Certificate::RSA, nullptr, nullptr); + certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); Certificate& cert = certs.back(); + uint32_t exponent = 0; char start_char; if (fscanf(f.get(), " %c", &start_char) != 1) return false; if (start_char == '{') { // a version 1 key has no version specifier. - cert.key_type = Certificate::RSA; - cert.rsa = std::unique_ptr(new RSAPublicKey); - cert.rsa->exponent = 3; - cert.hash_len = SHA_DIGEST_SIZE; + cert.key_type = Certificate::KEY_TYPE_RSA; + exponent = 3; + cert.hash_len = SHA_DIGEST_LENGTH; } else if (start_char == 'v') { int version; if (fscanf(f.get(), "%d {", &version) != 1) return false; switch (version) { case 2: - cert.key_type = Certificate::RSA; - cert.rsa = std::unique_ptr(new RSAPublicKey); - cert.rsa->exponent = 65537; - cert.hash_len = SHA_DIGEST_SIZE; + cert.key_type = Certificate::KEY_TYPE_RSA; + exponent = 65537; + cert.hash_len = SHA_DIGEST_LENGTH; break; case 3: - cert.key_type = Certificate::RSA; - cert.rsa = std::unique_ptr(new RSAPublicKey); - cert.rsa->exponent = 3; - cert.hash_len = SHA256_DIGEST_SIZE; + cert.key_type = Certificate::KEY_TYPE_RSA; + exponent = 3; + cert.hash_len = SHA256_DIGEST_LENGTH; break; case 4: - cert.key_type = Certificate::RSA; - cert.rsa = std::unique_ptr(new RSAPublicKey); - cert.rsa->exponent = 65537; - cert.hash_len = SHA256_DIGEST_SIZE; + cert.key_type = Certificate::KEY_TYPE_RSA; + exponent = 65537; + cert.hash_len = SHA256_DIGEST_LENGTH; break; case 5: - cert.key_type = Certificate::EC; - cert.ec = std::unique_ptr(new ECPublicKey); - cert.hash_len = SHA256_DIGEST_SIZE; + cert.key_type = Certificate::KEY_TYPE_EC; + cert.hash_len = SHA256_DIGEST_LENGTH; break; default: return false; } } - if (cert.key_type == Certificate::RSA) { - RSAPublicKey* key = cert.rsa.get(); - if (fscanf(f.get(), " %i , 0x%x , { %u", &(key->len), &(key->n0inv), - &(key->n[0])) != 3) { - return false; - } - if (key->len != RSANUMWORDS) { - LOGE("key length (%d) does not match expected size\n", key->len); - return false; - } - for (int i = 1; i < key->len; ++i) { - if (fscanf(f.get(), " , %u", &(key->n[i])) != 1) return false; + if (cert.key_type == Certificate::KEY_TYPE_RSA) { + cert.rsa = parse_rsa_key(f.get(), exponent); + if (!cert.rsa) { + return false; } - if (fscanf(f.get(), " } , { %u", &(key->rr[0])) != 1) return false; - for (int i = 1; i < key->len; ++i) { - if (fscanf(f.get(), " , %u", &(key->rr[i])) != 1) return false; - } - fscanf(f.get(), " } } "); - - LOGI("read key e=%d hash=%d\n", key->exponent, cert.hash_len); - } else if (cert.key_type == Certificate::EC) { - ECPublicKey* key = cert.ec.get(); - int key_len; - unsigned int byte; - uint8_t x_bytes[P256_NBYTES]; - uint8_t y_bytes[P256_NBYTES]; - if (fscanf(f.get(), " %i , { %u", &key_len, &byte) != 2) return false; - if (key_len != P256_NBYTES) { - LOGE("Key length (%d) does not match expected size %d\n", key_len, P256_NBYTES); - return false; - } - x_bytes[P256_NBYTES - 1] = byte; - for (int i = P256_NBYTES - 2; i >= 0; --i) { - if (fscanf(f.get(), " , %u", &byte) != 1) return false; - x_bytes[i] = byte; - } - if (fscanf(f.get(), " } , { %u", &byte) != 1) return false; - y_bytes[P256_NBYTES - 1] = byte; - for (int i = P256_NBYTES - 2; i >= 0; --i) { - if (fscanf(f.get(), " , %u", &byte) != 1) return false; - y_bytes[i] = byte; + + LOGI("read key e=%d hash=%d\n", exponent, cert.hash_len); + } else if (cert.key_type == Certificate::KEY_TYPE_EC) { + cert.ec = parse_ec_key(f.get()); + if (!cert.ec) { + return false; } - fscanf(f.get(), " } } "); - p256_from_bin(x_bytes, &key->x); - p256_from_bin(y_bytes, &key->y); } else { LOGE("Unknown key type %d\n", cert.key_type); return false; -- cgit v1.2.3 From dd895d0adaa691a078f18a95a7f5ac0eaf776cae Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 19 Apr 2016 15:24:38 -0700 Subject: Decrease OTA package verification times further. Timing from Nexus 5X: 89 MiB OTA update package: 1.4 s -> 0.6 s (decreased by 57%) 1196 MiB OTA update package: 8.0 s -> 7.5 s (decreased by 6%) Bug: http://b/28135231 Change-Id: Id91f2ad15df2bffb9f8a4b4ec5a57657a02847ec --- verifier.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'verifier.cpp') diff --git a/verifier.cpp b/verifier.cpp index 4004b0228..f5299b4a2 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -206,10 +206,10 @@ int verify_file(unsigned char* addr, size_t length, double frac = -1.0; size_t so_far = 0; while (so_far < signed_len) { - // On a Nexus 9, experiment didn't show any performance improvement with - // larger sizes past 1MiB, and they reduce the granularity of the progress - // bar. http://b/28135231. - size_t size = std::min(signed_len - so_far, 1 * MiB); + // On a Nexus 5X, experiment showed 16MiB beat 1MiB by 6% faster for a + // 1196MiB full OTA and 60% for an 89MiB incremental OTA. + // http://b/28135231. + size_t size = std::min(signed_len - so_far, 16 * MiB); if (need_sha1) SHA1_Update(&sha1_ctx, addr + so_far, size); if (need_sha256) SHA256_Update(&sha256_ctx, addr + so_far, size); -- cgit v1.2.3 From e179276f7dd94e9ef738f00c6953d251c76f22f7 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 19 Apr 2016 22:31:01 -0700 Subject: recovery: Dump the signature in the zip package. We have been occasionally seeing "signature verification failed" error message when applying an update. Make more verbose output to help debugging. Bug: 28246534 Change-Id: Id83633adc9b86b3fd36abbb504e430f0816f12e4 --- verifier.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'verifier.cpp') diff --git a/verifier.cpp b/verifier.cpp index f5299b4a2..16cc7cf03 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -27,6 +27,7 @@ #include "asn1_decoder.h" #include "common.h" +#include "print_sha1.h" #include "ui.h" #include "verifier.h" @@ -230,9 +231,14 @@ int verify_file(unsigned char* addr, size_t length, uint8_t* sig_der = nullptr; size_t sig_der_length = 0; + uint8_t* signature = eocd + eocd_size - signature_start; size_t signature_size = signature_start - FOOTER_SIZE; - if (!read_pkcs7(eocd + eocd_size - signature_start, signature_size, &sig_der, - &sig_der_length)) { + + LOGI("signature (offset: 0x%zx, length: %zu): %s\n", + length - signature_start, signature_size, + print_hex(signature, signature_size).c_str()); + + if (!read_pkcs7(signature, signature_size, &sig_der, &sig_der_length)) { LOGE("Could not find signature DER block\n"); return VERIFY_FAILURE; } @@ -287,6 +293,13 @@ int verify_file(unsigned char* addr, size_t length, } i++; } + + if (need_sha1) { + LOGI("SHA-1 digest: %s\n", print_hex(sha1, SHA_DIGEST_LENGTH).c_str()); + } + if (need_sha256) { + LOGI("SHA-256 digest: %s\n", print_hex(sha256, SHA256_DIGEST_LENGTH).c_str()); + } free(sig_der); LOGE("failed to verify whole-file signature\n"); return VERIFY_FAILURE; -- cgit v1.2.3