diff options
-rw-r--r-- | asn1_decoder.cpp | 12 | ||||
-rw-r--r-- | asn1_decoder.h | 6 | ||||
-rw-r--r-- | install.cpp | 2 | ||||
-rw-r--r-- | tests/unit/asn1_decoder_test.cpp | 28 | ||||
-rw-r--r-- | verifier.cpp | 107 | ||||
-rw-r--r-- | verifier.h | 2 |
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); |