summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTao Bao <tbao@google.com>2017-03-21 20:16:39 +0100
committerandroid-build-merger <android-build-merger@google.com>2017-03-21 20:16:39 +0100
commit9d9f310f4ec3ab5c38b127a6886948571ca55666 (patch)
tree8da46ba9ab10ebc36c7252f8ccab10e6471bc0db
parentMerge "Add testcases for load_keys()." am: 110102f37e am: 43f5ad6e90 (diff)
parentMerge "verify_file: Add constness to a few addresses." am: 5b2bf90e13 (diff)
downloadandroid_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar.gz
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar.bz2
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar.lz
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar.xz
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.tar.zst
android_bootable_recovery-9d9f310f4ec3ab5c38b127a6886948571ca55666.zip
-rw-r--r--asn1_decoder.cpp12
-rw-r--r--asn1_decoder.h6
-rw-r--r--install.cpp2
-rw-r--r--tests/unit/asn1_decoder_test.cpp28
-rw-r--r--verifier.cpp107
-rw-r--r--verifier.h2
6 files changed, 78 insertions, 79 deletions
diff --git a/asn1_decoder.cpp b/asn1_decoder.cpp
index e7aef781c..ca4ee5267 100644
--- a/asn1_decoder.cpp
+++ b/asn1_decoder.cpp
@@ -22,9 +22,9 @@
typedef struct asn1_context {
- size_t length;
- uint8_t* p;
- int app_type;
+ size_t length;
+ const uint8_t* p;
+ int app_type;
} asn1_context_t;
@@ -38,7 +38,7 @@ static const int kTagSequence = 0x30;
static const int kTagSet = 0x31;
static const int kTagConstructed = 0xA0;
-asn1_context_t* asn1_context_new(uint8_t* buffer, size_t length) {
+asn1_context_t* asn1_context_new(const uint8_t* buffer, size_t length) {
asn1_context_t* ctx = (asn1_context_t*) calloc(1, sizeof(asn1_context_t));
if (ctx == NULL) {
return NULL;
@@ -168,7 +168,7 @@ bool asn1_sequence_next(asn1_context_t* ctx) {
return true;
}
-bool asn1_oid_get(asn1_context_t* ctx, uint8_t** oid, size_t* length) {
+bool asn1_oid_get(asn1_context_t* ctx, const uint8_t** oid, size_t* length) {
if (get_byte(ctx) != kTagOid) {
return false;
}
@@ -179,7 +179,7 @@ bool asn1_oid_get(asn1_context_t* ctx, uint8_t** oid, size_t* length) {
return true;
}
-bool asn1_octet_string_get(asn1_context_t* ctx, uint8_t** octet_string, size_t* length) {
+bool asn1_octet_string_get(asn1_context_t* ctx, const uint8_t** octet_string, size_t* length) {
if (get_byte(ctx) != kTagOctetString) {
return false;
}
diff --git a/asn1_decoder.h b/asn1_decoder.h
index b17141c44..fbd118f90 100644
--- a/asn1_decoder.h
+++ b/asn1_decoder.h
@@ -22,7 +22,7 @@
typedef struct asn1_context asn1_context_t;
-asn1_context_t* asn1_context_new(uint8_t* buffer, size_t length);
+asn1_context_t* asn1_context_new(const uint8_t* buffer, size_t length);
void asn1_context_free(asn1_context_t* ctx);
asn1_context_t* asn1_constructed_get(asn1_context_t* ctx);
bool asn1_constructed_skip_all(asn1_context_t* ctx);
@@ -30,7 +30,7 @@ int asn1_constructed_type(asn1_context_t* ctx);
asn1_context_t* asn1_sequence_get(asn1_context_t* ctx);
asn1_context_t* asn1_set_get(asn1_context_t* ctx);
bool asn1_sequence_next(asn1_context_t* seq);
-bool asn1_oid_get(asn1_context_t* ctx, uint8_t** oid, size_t* length);
-bool asn1_octet_string_get(asn1_context_t* ctx, uint8_t** octet_string, size_t* length);
+bool asn1_oid_get(asn1_context_t* ctx, const uint8_t** oid, size_t* length);
+bool asn1_octet_string_get(asn1_context_t* ctx, const uint8_t** octet_string, size_t* length);
#endif /* ASN1_DECODER_H_ */
diff --git a/install.cpp b/install.cpp
index 68d3f54bc..a256d9b13 100644
--- a/install.cpp
+++ b/install.cpp
@@ -589,7 +589,7 @@ bool verify_package(const unsigned char* package_data, size_t package_size) {
// Verify package.
ui->Print("Verifying update package...\n");
auto t0 = std::chrono::system_clock::now();
- int err = verify_file(const_cast<unsigned char*>(package_data), package_size, loadedKeys,
+ int err = verify_file(package_data, package_size, loadedKeys,
std::bind(&RecoveryUI::SetProgress, ui, std::placeholders::_1));
std::chrono::duration<double> duration = std::chrono::system_clock::now() - t0;
ui->Print("Update package verification took %.1f s (result %d).\n", duration.count(), err);
diff --git a/tests/unit/asn1_decoder_test.cpp b/tests/unit/asn1_decoder_test.cpp
index af96d87d2..997639d8a 100644
--- a/tests/unit/asn1_decoder_test.cpp
+++ b/tests/unit/asn1_decoder_test.cpp
@@ -39,7 +39,7 @@ TEST_F(Asn1DecoderTest, Empty_Failure) {
EXPECT_EQ(NULL, asn1_set_get(ctx));
EXPECT_FALSE(asn1_sequence_next(ctx));
- uint8_t* junk;
+ const uint8_t* junk;
size_t length;
EXPECT_FALSE(asn1_oid_get(ctx, &junk, &length));
EXPECT_FALSE(asn1_octet_string_get(ctx, &junk, &length));
@@ -68,7 +68,7 @@ TEST_F(Asn1DecoderTest, ConstructedGet_TooSmallForChild_Failure) {
asn1_context_t* ptr = asn1_constructed_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
EXPECT_EQ(5, asn1_constructed_type(ptr));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
EXPECT_FALSE(asn1_oid_get(ptr, &oid, &length));
asn1_context_free(ptr);
@@ -81,7 +81,7 @@ TEST_F(Asn1DecoderTest, ConstructedGet_Success) {
asn1_context_t* ptr = asn1_constructed_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
EXPECT_EQ(5, asn1_constructed_type(ptr));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
ASSERT_TRUE(asn1_oid_get(ptr, &oid, &length));
EXPECT_EQ(1U, length);
@@ -103,7 +103,7 @@ TEST_F(Asn1DecoderTest, ConstructedSkipAll_Success) {
0x06, 0x01, 0xA5, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
ASSERT_TRUE(asn1_constructed_skip_all(ctx));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
ASSERT_TRUE(asn1_oid_get(ctx, &oid, &length));
EXPECT_EQ(1U, length);
@@ -123,7 +123,7 @@ TEST_F(Asn1DecoderTest, SequenceGet_TooSmallForChild_Failure) {
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
asn1_context_t* ptr = asn1_sequence_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
EXPECT_FALSE(asn1_oid_get(ptr, &oid, &length));
asn1_context_free(ptr);
@@ -135,7 +135,7 @@ TEST_F(Asn1DecoderTest, SequenceGet_Success) {
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
asn1_context_t* ptr = asn1_sequence_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
ASSERT_TRUE(asn1_oid_get(ptr, &oid, &length));
EXPECT_EQ(1U, length);
@@ -156,7 +156,7 @@ TEST_F(Asn1DecoderTest, SetGet_TooSmallForChild_Failure) {
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
asn1_context_t* ptr = asn1_set_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
EXPECT_FALSE(asn1_oid_get(ptr, &oid, &length));
asn1_context_free(ptr);
@@ -168,7 +168,7 @@ TEST_F(Asn1DecoderTest, SetGet_Success) {
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
asn1_context_t* ptr = asn1_set_get(ctx);
ASSERT_NE((asn1_context_t*)NULL, ptr);
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
ASSERT_TRUE(asn1_oid_get(ptr, &oid, &length));
EXPECT_EQ(1U, length);
@@ -180,7 +180,7 @@ TEST_F(Asn1DecoderTest, SetGet_Success) {
TEST_F(Asn1DecoderTest, OidGet_LengthZero_Failure) {
uint8_t data[] = { 0x06, 0x00, 0x01, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
EXPECT_FALSE(asn1_oid_get(ctx, &oid, &length));
asn1_context_free(ctx);
@@ -189,7 +189,7 @@ TEST_F(Asn1DecoderTest, OidGet_LengthZero_Failure) {
TEST_F(Asn1DecoderTest, OidGet_TooSmall_Failure) {
uint8_t data[] = { 0x06, 0x01, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
EXPECT_FALSE(asn1_oid_get(ctx, &oid, &length));
asn1_context_free(ctx);
@@ -198,7 +198,7 @@ TEST_F(Asn1DecoderTest, OidGet_TooSmall_Failure) {
TEST_F(Asn1DecoderTest, OidGet_Success) {
uint8_t data[] = { 0x06, 0x01, 0x99, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* oid;
+ const uint8_t* oid;
size_t length;
ASSERT_TRUE(asn1_oid_get(ctx, &oid, &length));
EXPECT_EQ(1U, length);
@@ -209,7 +209,7 @@ TEST_F(Asn1DecoderTest, OidGet_Success) {
TEST_F(Asn1DecoderTest, OctetStringGet_LengthZero_Failure) {
uint8_t data[] = { 0x04, 0x00, 0x55, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* string;
+ const uint8_t* string;
size_t length;
ASSERT_FALSE(asn1_octet_string_get(ctx, &string, &length));
asn1_context_free(ctx);
@@ -218,7 +218,7 @@ TEST_F(Asn1DecoderTest, OctetStringGet_LengthZero_Failure) {
TEST_F(Asn1DecoderTest, OctetStringGet_TooSmall_Failure) {
uint8_t data[] = { 0x04, 0x01, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* string;
+ const uint8_t* string;
size_t length;
ASSERT_FALSE(asn1_octet_string_get(ctx, &string, &length));
asn1_context_free(ctx);
@@ -227,7 +227,7 @@ TEST_F(Asn1DecoderTest, OctetStringGet_TooSmall_Failure) {
TEST_F(Asn1DecoderTest, OctetStringGet_Success) {
uint8_t data[] = { 0x04, 0x01, 0xAA, };
asn1_context_t* ctx = asn1_context_new(data, sizeof(data));
- uint8_t* string;
+ const uint8_t* string;
size_t length;
ASSERT_TRUE(asn1_octet_string_get(ctx, &string, &length));
EXPECT_EQ(1U, length);
diff --git a/verifier.cpp b/verifier.cpp
index 6daeac94e..7f165860e 100644
--- a/verifier.cpp
+++ b/verifier.cpp
@@ -24,6 +24,7 @@
#include <algorithm>
#include <functional>
#include <memory>
+#include <vector>
#include <android-base/logging.h>
#include <openssl/bn.h>
@@ -60,51 +61,53 @@ static constexpr size_t MiB = 1024 * 1024;
* SEQUENCE (SignatureAlgorithmIdentifier)
* OCTET STRING (SignatureValue)
*/
-static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_der,
- size_t* sig_der_length) {
- asn1_context_t* ctx = asn1_context_new(pkcs7_der, pkcs7_der_len);
- if (ctx == NULL) {
- return false;
- }
+static bool read_pkcs7(const uint8_t* pkcs7_der, size_t pkcs7_der_len,
+ std::vector<uint8_t>* sig_der) {
+ CHECK(sig_der != nullptr);
+ sig_der->clear();
+
+ asn1_context_t* ctx = asn1_context_new(pkcs7_der, pkcs7_der_len);
+ if (ctx == NULL) {
+ return false;
+ }
- asn1_context_t* pkcs7_seq = asn1_sequence_get(ctx);
- if (pkcs7_seq != NULL && asn1_sequence_next(pkcs7_seq)) {
- asn1_context_t *signed_data_app = asn1_constructed_get(pkcs7_seq);
- if (signed_data_app != NULL) {
- asn1_context_t* signed_data_seq = asn1_sequence_get(signed_data_app);
- if (signed_data_seq != NULL
- && asn1_sequence_next(signed_data_seq)
- && asn1_sequence_next(signed_data_seq)
- && asn1_sequence_next(signed_data_seq)
- && asn1_constructed_skip_all(signed_data_seq)) {
- asn1_context_t *sig_set = asn1_set_get(signed_data_seq);
- if (sig_set != NULL) {
- asn1_context_t* sig_seq = asn1_sequence_get(sig_set);
- if (sig_seq != NULL
- && asn1_sequence_next(sig_seq)
- && asn1_sequence_next(sig_seq)
- && asn1_sequence_next(sig_seq)
- && asn1_sequence_next(sig_seq)) {
- uint8_t* sig_der_ptr;
- if (asn1_octet_string_get(sig_seq, &sig_der_ptr, sig_der_length)) {
- *sig_der = (uint8_t*) malloc(*sig_der_length);
- if (*sig_der != NULL) {
- memcpy(*sig_der, sig_der_ptr, *sig_der_length);
- }
- }
- asn1_context_free(sig_seq);
- }
- asn1_context_free(sig_set);
- }
- asn1_context_free(signed_data_seq);
+ asn1_context_t* pkcs7_seq = asn1_sequence_get(ctx);
+ if (pkcs7_seq != NULL && asn1_sequence_next(pkcs7_seq)) {
+ asn1_context_t *signed_data_app = asn1_constructed_get(pkcs7_seq);
+ if (signed_data_app != NULL) {
+ asn1_context_t* signed_data_seq = asn1_sequence_get(signed_data_app);
+ if (signed_data_seq != NULL
+ && asn1_sequence_next(signed_data_seq)
+ && asn1_sequence_next(signed_data_seq)
+ && asn1_sequence_next(signed_data_seq)
+ && asn1_constructed_skip_all(signed_data_seq)) {
+ asn1_context_t *sig_set = asn1_set_get(signed_data_seq);
+ if (sig_set != NULL) {
+ asn1_context_t* sig_seq = asn1_sequence_get(sig_set);
+ if (sig_seq != NULL
+ && asn1_sequence_next(sig_seq)
+ && asn1_sequence_next(sig_seq)
+ && asn1_sequence_next(sig_seq)
+ && asn1_sequence_next(sig_seq)) {
+ const uint8_t* sig_der_ptr;
+ size_t sig_der_length;
+ if (asn1_octet_string_get(sig_seq, &sig_der_ptr, &sig_der_length)) {
+ sig_der->resize(sig_der_length);
+ std::copy(sig_der_ptr, sig_der_ptr + sig_der_length, sig_der->begin());
}
- asn1_context_free(signed_data_app);
+ asn1_context_free(sig_seq);
+ }
+ asn1_context_free(sig_set);
}
- asn1_context_free(pkcs7_seq);
+ asn1_context_free(signed_data_seq);
+ }
+ asn1_context_free(signed_data_app);
}
- asn1_context_free(ctx);
+ asn1_context_free(pkcs7_seq);
+ }
+ asn1_context_free(ctx);
- return *sig_der != NULL;
+ return !sig_der->empty();
}
/*
@@ -115,7 +118,7 @@ static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_d
* Returns VERIFY_SUCCESS or VERIFY_FAILURE (if any error is encountered or no key matches the
* signature).
*/
-int verify_file(unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
+int verify_file(const unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
const std::function<void(float)>& set_progress) {
if (set_progress) {
set_progress(0.0);
@@ -136,7 +139,7 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
return VERIFY_FAILURE;
}
- unsigned char* footer = addr + length - FOOTER_SIZE;
+ const unsigned char* footer = addr + length - FOOTER_SIZE;
if (footer[2] != 0xff || footer[3] != 0xff) {
LOG(ERROR) << "footer is wrong";
@@ -174,7 +177,7 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
// (2 bytes) and the comment data.
size_t signed_len = length - eocd_size + EOCD_HEADER_SIZE - 2;
- unsigned char* eocd = addr + length - eocd_size;
+ const unsigned char* eocd = addr + length - eocd_size;
// If this is really is the EOCD record, it will begin with the magic number $50 $4b $05 $06.
if (eocd[0] != 0x50 || eocd[1] != 0x4b || eocd[2] != 0x05 || eocd[3] != 0x06) {
@@ -183,7 +186,7 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
}
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 (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 the real one, libziparchive will
// find the later (wrong) one, which could be exploitable. Fail the verification if this
// sequence occurs anywhere after the real one.
@@ -232,16 +235,14 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
uint8_t sha256[SHA256_DIGEST_LENGTH];
SHA256_Final(sha256, &sha256_ctx);
- uint8_t* sig_der = nullptr;
- size_t sig_der_length = 0;
-
- uint8_t* signature = eocd + eocd_size - signature_start;
+ const uint8_t* signature = eocd + eocd_size - signature_start;
size_t signature_size = signature_start - FOOTER_SIZE;
LOG(INFO) << "signature (offset: " << std::hex << (length - signature_start) << ", length: "
<< signature_size << "): " << print_hex(signature, signature_size);
- if (!read_pkcs7(signature, signature_size, &sig_der, &sig_der_length)) {
+ std::vector<uint8_t> sig_der;
+ if (!read_pkcs7(signature, signature_size, &sig_der)) {
LOG(ERROR) << "Could not find signature DER block";
return VERIFY_FAILURE;
}
@@ -268,22 +269,21 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
// 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::KEY_TYPE_RSA) {
- if (!RSA_verify(hash_nid, hash, key.hash_len, sig_der, sig_der_length, key.rsa.get())) {
+ if (!RSA_verify(hash_nid, hash, key.hash_len, sig_der.data(), sig_der.size(),
+ key.rsa.get())) {
LOG(INFO) << "failed to verify against RSA key " << i;
continue;
}
LOG(INFO) << "whole-file signature verified against RSA key " << i;
- free(sig_der);
return VERIFY_SUCCESS;
} 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())) {
+ if (!ECDSA_verify(0, hash, key.hash_len, sig_der.data(), sig_der.size(), key.ec.get())) {
LOG(INFO) << "failed to verify against EC key " << i;
continue;
}
LOG(INFO) << "whole-file signature verified against EC key " << i;
- free(sig_der);
return VERIFY_SUCCESS;
} else {
LOG(INFO) << "Unknown key type " << key.key_type;
@@ -297,7 +297,6 @@ int verify_file(unsigned char* addr, size_t length, const std::vector<Certificat
if (need_sha256) {
LOG(INFO) << "SHA-256 digest: " << print_hex(sha256, SHA256_DIGEST_LENGTH);
}
- free(sig_der);
LOG(ERROR) << "failed to verify whole-file signature";
return VERIFY_FAILURE;
}
diff --git a/verifier.h b/verifier.h
index 067dab554..6bee74947 100644
--- a/verifier.h
+++ b/verifier.h
@@ -65,7 +65,7 @@ struct Certificate {
* given keys. It optionally accepts a callback function for posting the progress to. Returns one
* of the constants of VERIFY_SUCCESS and VERIFY_FAILURE.
*/
-int verify_file(unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
+int verify_file(const unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
const std::function<void(float)>& set_progress = nullptr);
bool load_keys(const char* filename, std::vector<Certificate>& certs);