Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 76fdb241 authored by Tao Bao's avatar Tao Bao
Browse files

verify_file: Add constness to a few addresses. 

We should not touch any data while verifying packages (or parsing the
in-memory ASN.1 structures).

Test: mmma bootable/recovery
Test: recovery_component_test passes.
Test: recovery_unit_test passes.
Change-Id: Ie990662c6451ec066a1807b3081c9296afbdb0bf
parent 110102f3

asn1_decoder.cpp

+6 −6
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ 


typedef struct asn1_context {

  size_t length;

    uint8_t* p;

  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;

    }

asn1_decoder.h

+3 −3
Original line number Diff line number Diff line
@@ -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_ */

install.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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);

tests/unit/asn1_decoder_test.cpp

+14 −14
Original line number Diff line number Diff line
@@ -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);

verifier.cpp

+53 −54
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <algorithm>

#include <functional>

#include <memory>

#include <vector>



#include <android-base/logging.h>

#include <openssl/bn.h>
@@ -60,8 +61,11 @@ 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) {

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;
@@ -85,12 +89,11 @@ static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_d 
              && 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);

                            }

            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(sig_seq);

          }
@@ -104,7 +107,7 @@ static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_d 
  }

  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";
@@ -168,7 +171,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) {
@@ -226,16 +229,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;

  }
@@ -262,22 +263,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;
@@ -291,7 +291,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;

}