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
This commit is contained in:
Tao Bao 2016-02-02 14:02:27 -08:00
parent 4c07a7ec05
commit 71e3e09ec2
4 changed files with 167 additions and 192 deletions

View file

@ -23,6 +23,8 @@
#include <sys/wait.h>
#include <unistd.h>
#include <vector>
#include "common.h"
#include "install.h"
#include "mincrypt/rsa.h"
@ -221,19 +223,16 @@ really_install_package(const char *path, bool* wipe_cache, bool needs_mount)
return INSTALL_CORRUPT;
}
int numKeys;
Certificate* loadedKeys = load_keys(PUBLIC_KEYS_FILE, &numKeys);
if (loadedKeys == NULL) {
std::vector<Certificate> loadedKeys;
if (!load_keys(PUBLIC_KEYS_FILE, loadedKeys)) {
LOGE("Failed to load keys\n");
return INSTALL_CORRUPT;
}
LOGI("%d key(s) loaded from %s\n", numKeys, PUBLIC_KEYS_FILE);
LOGI("%zu key(s) loaded from %s\n", loadedKeys.size(), PUBLIC_KEYS_FILE);
ui->Print("Verifying update package...\n");
int err;
err = verify_file(map.addr, map.length, loadedKeys, numKeys);
free(loadedKeys);
int err = verify_file(map.addr, map.length, loadedKeys);
LOGI("verify_file returned %d\n", err);
if (err != VERIFY_SUCCESS) {
LOGE("signature verification failed\n");

View file

@ -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<Certificate>& 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<Certificate>& certs) {
std::unique_ptr<FILE, decltype(&fclose)> 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));
while (true) {
certs.emplace_back(0, Certificate::RSA, nullptr, nullptr);
Certificate& cert = certs.back();
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;
}
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<RSAPublicKey>(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<RSAPublicKey>(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<RSAPublicKey>(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<RSAPublicKey>(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<ECPublicKey>(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;
}
// 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;
}
}
fclose(f);
return out;
exit:
if (f) fclose(f);
free(out);
*numKeys = 0;
return NULL;
return true;
}

View file

@ -17,6 +17,9 @@
#ifndef _RECOVERY_VERIFIER_H
#define _RECOVERY_VERIFIER_H
#include <memory>
#include <vector>
#include "mincrypt/p256.h"
#include "mincrypt/rsa.h"
@ -25,17 +28,25 @@ typedef struct {
p256_int y;
} ECPublicKey;
typedef struct {
struct Certificate {
typedef enum {
RSA,
EC,
} KeyType;
Certificate(int hash_len_, KeyType key_type_,
std::unique_ptr<RSAPublicKey>&& rsa_,
std::unique_ptr<ECPublicKey>&& ec_) :
hash_len(hash_len_),
key_type(key_type_),
rsa(std::move(rsa_)),
ec(std::move(ec_)) { }
int hash_len; // SHA_DIGEST_SIZE (SHA-1) or SHA256_DIGEST_SIZE (SHA-256)
KeyType key_type;
RSAPublicKey* rsa;
ECPublicKey* ec;
} Certificate;
std::unique_ptr<RSAPublicKey> rsa;
std::unique_ptr<ECPublicKey> ec;
};
/* addr and length define a an update package file that has been
* loaded (or mmap'ed, or whatever) into memory. Verify that the file
@ -43,9 +54,9 @@ typedef struct {
* one of the constants below.
*/
int verify_file(unsigned char* addr, size_t length,
const Certificate *pKeys, unsigned int numKeys);
const std::vector<Certificate>& keys);
Certificate* load_keys(const char* filename, int* numKeys);
bool load_keys(const char* filename, std::vector<Certificate>& certs);
#define VERIFY_SUCCESS 0
#define VERIFY_FAILURE 1

View file

@ -23,6 +23,9 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <memory>
#include <vector>
#include "common.h"
#include "verifier.h"
#include "ui.h"
@ -163,56 +166,43 @@ ui_print(const char* format, ...) {
va_end(ap);
}
static Certificate* add_certificate(Certificate** certsp, int* num_keys,
Certificate::KeyType key_type) {
int i = *num_keys;
*num_keys = *num_keys + 1;
*certsp = (Certificate*) realloc(*certsp, *num_keys * sizeof(Certificate));
Certificate* certs = *certsp;
certs[i].rsa = NULL;
certs[i].ec = NULL;
certs[i].key_type = key_type;
certs[i].hash_len = SHA_DIGEST_SIZE;
return &certs[i];
}
int main(int argc, char **argv) {
int main(int argc, char** argv) {
if (argc < 2) {
fprintf(stderr, "Usage: %s [-sha256] [-ec | -f4 | -file <keys>] <package>\n", argv[0]);
return 2;
}
Certificate* certs = NULL;
int num_keys = 0;
std::vector<Certificate> certs;
int argn = 1;
while (argn < argc) {
if (strcmp(argv[argn], "-sha256") == 0) {
if (num_keys == 0) {
if (certs.empty()) {
fprintf(stderr, "May only specify -sha256 after key type\n");
return 2;
}
++argn;
Certificate* cert = &certs[num_keys - 1];
cert->hash_len = SHA256_DIGEST_SIZE;
certs.back().hash_len = SHA256_DIGEST_SIZE;
} else if (strcmp(argv[argn], "-ec") == 0) {
++argn;
Certificate* cert = add_certificate(&certs, &num_keys, Certificate::EC);
cert->ec = &test_ec_key;
certs.emplace_back(SHA_DIGEST_SIZE, Certificate::EC,
nullptr, std::unique_ptr<ECPublicKey>(new ECPublicKey(test_ec_key)));
} else if (strcmp(argv[argn], "-e3") == 0) {
++argn;
Certificate* cert = add_certificate(&certs, &num_keys, Certificate::RSA);
cert->rsa = &test_key;
certs.emplace_back(SHA_DIGEST_SIZE, Certificate::RSA,
std::unique_ptr<RSAPublicKey>(new RSAPublicKey(test_key)), nullptr);
} else if (strcmp(argv[argn], "-f4") == 0) {
++argn;
Certificate* cert = add_certificate(&certs, &num_keys, Certificate::RSA);
cert->rsa = &test_f4_key;
certs.emplace_back(SHA_DIGEST_SIZE, Certificate::RSA,
std::unique_ptr<RSAPublicKey>(new RSAPublicKey(test_f4_key)), nullptr);
} else if (strcmp(argv[argn], "-file") == 0) {
if (certs != NULL) {
if (!certs.empty()) {
fprintf(stderr, "Cannot specify -file with other certs specified\n");
return 2;
}
++argn;
certs = load_keys(argv[argn], &num_keys);
if (!load_keys(argv[argn], certs)) {
fprintf(stderr, "Cannot load keys from %s\n", argv[argn]);
}
++argn;
} else if (argv[argn][0] == '-') {
fprintf(stderr, "Unknown argument %s\n", argv[argn]);
@ -227,17 +217,9 @@ int main(int argc, char **argv) {
return 2;
}
if (num_keys == 0) {
certs = (Certificate*) calloc(1, sizeof(Certificate));
if (certs == NULL) {
fprintf(stderr, "Failure allocating memory for default certificate\n");
return 1;
}
certs->key_type = Certificate::RSA;
certs->rsa = &test_key;
certs->ec = NULL;
certs->hash_len = SHA_DIGEST_SIZE;
num_keys = 1;
if (certs.empty()) {
certs.emplace_back(SHA_DIGEST_SIZE, Certificate::RSA,
std::unique_ptr<RSAPublicKey>(new RSAPublicKey(test_key)), nullptr);
}
ui = new FakeUI();
@ -248,7 +230,7 @@ int main(int argc, char **argv) {
return 4;
}
int result = verify_file(map.addr, map.length, certs, num_keys);
int result = verify_file(map.addr, map.length, certs);
if (result == VERIFY_SUCCESS) {
printf("VERIFIED\n");
return 0;