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

Commit cbe93e65 authored by Tianjie Xu's avatar Tianjie Xu
Browse files

Remove the load_keys function 

This function is used to parse the result of dumpKeys. It's no longer
needed as we are now parsing the public keys from the zipfile.

Bug: 116655889
Test: unit tests pass
Change-Id: I817906e451664058c644f4329ff499bbe4587ebb
parent b5110de1

tests/component/verifier_test.cpp

+9 −75
Original line number Diff line number Diff line
@@ -238,8 +238,9 @@ class VerifierTest : public testing::TestWithParam<std::vector<std::string>> { 
    }



    for (auto it = ++args.cbegin(); it != args.cend(); ++it) {

      std::string public_key_file = from_testdata_base("testkey_" + *it + ".txt");

      ASSERT_TRUE(load_keys(public_key_file.c_str(), certs));

      std::string public_key_file = from_testdata_base("testkey_" + *it + ".x509.pem");

      certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);

      LoadKeyFromFile(public_key_file, &certs.back());

    }

  }
@@ -253,70 +254,10 @@ class VerifierSuccessTest : public VerifierTest { 
class VerifierFailureTest : public VerifierTest {

};



TEST(VerifierTest, load_keys_multiple_keys) {

  std::string testkey_v4;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v4.txt"), &testkey_v4));



  std::string testkey_v3;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));



  std::string keys = testkey_v4 + "," + testkey_v3 + "," + testkey_v4;

  TemporaryFile key_file1;

  ASSERT_TRUE(android::base::WriteStringToFile(keys, key_file1.path));

  std::vector<Certificate> certs;

  ASSERT_TRUE(load_keys(key_file1.path, certs));

  ASSERT_EQ(3U, certs.size());

}



TEST(VerifierTest, load_keys_invalid_keys) {

  std::vector<Certificate> certs;

  ASSERT_FALSE(load_keys("/doesntexist", certs));



  // Empty file.

  TemporaryFile key_file1;

  ASSERT_FALSE(load_keys(key_file1.path, certs));



  // Invalid contents.

  ASSERT_TRUE(android::base::WriteStringToFile("invalid", key_file1.path));

  ASSERT_FALSE(load_keys(key_file1.path, certs));



  std::string testkey_v4;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v4.txt"), &testkey_v4));



  // Invalid key version: "v4 ..." => "v6 ...".

  std::string invalid_key2(testkey_v4);

  invalid_key2[1] = '6';

  TemporaryFile key_file2;

  ASSERT_TRUE(android::base::WriteStringToFile(invalid_key2, key_file2.path));

  ASSERT_FALSE(load_keys(key_file2.path, certs));



  // Invalid key content: inserted extra bytes ",2209831334".

  std::string invalid_key3(testkey_v4);

  invalid_key3.insert(invalid_key2.size() - 2, ",2209831334");

  TemporaryFile key_file3;

  ASSERT_TRUE(android::base::WriteStringToFile(invalid_key3, key_file3.path));

  ASSERT_FALSE(load_keys(key_file3.path, certs));



  // Invalid key: the last key must not end with an extra ','.

  std::string invalid_key4 = testkey_v4 + ",";

  TemporaryFile key_file4;

  ASSERT_TRUE(android::base::WriteStringToFile(invalid_key4, key_file4.path));

  ASSERT_FALSE(load_keys(key_file4.path, certs));



  // Invalid key separator.

  std::string invalid_key5 = testkey_v4 + ";" + testkey_v4;

  TemporaryFile key_file5;

  ASSERT_TRUE(android::base::WriteStringToFile(invalid_key5, key_file5.path));

  ASSERT_FALSE(load_keys(key_file5.path, certs));

}



TEST(VerifierTest, BadPackage_AlteredFooter) {

  std::string testkey_v3;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));

  TemporaryFile key_file1;

  ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path));

  std::vector<Certificate> certs;

  ASSERT_TRUE(load_keys(key_file1.path, certs));

  certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);

  LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back());



  std::string package;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package));
@@ -330,12 +271,9 @@ TEST(VerifierTest, BadPackage_AlteredFooter) { 
}



TEST(VerifierTest, BadPackage_AlteredContent) {

  std::string testkey_v3;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));

  TemporaryFile key_file1;

  ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path));

  std::vector<Certificate> certs;

  ASSERT_TRUE(load_keys(key_file1.path, certs));

  certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);

  LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back());



  std::string package;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package));
@@ -356,13 +294,9 @@ TEST(VerifierTest, BadPackage_AlteredContent) { 
}



TEST(VerifierTest, BadPackage_SignatureStartOutOfBounds) {

  std::string testkey_v3;

  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));



  TemporaryFile key_file;

  ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file.path));

  std::vector<Certificate> certs;

  ASSERT_TRUE(load_keys(key_file.path, certs));

  certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);

  LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back());



  // Signature start is 65535 (0xffff) while comment size is 0 (Bug: 31914369).

  std::string package = "\x50\x4b\x05\x06"s + std::string(12, '\0') + "\xff\xff\xff\xff\x00\x00"s;

verifier.cpp

+0 −249
Original line number Diff line number Diff line
@@ -308,144 +308,6 @@ int verify_file(const unsigned char* addr, size_t length, const std::vector<Cert 
  return VERIFY_FAILURE;

}



std::unique_ptr<RSA, RSADeleter> 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) {

        LOG(ERROR) << "key length (" << key_len_words << ") too large";

        return nullptr;

    }



    // Read the modulus.

    std::unique_ptr<uint32_t[]> 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<RSA, RSADeleter> 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) const {

    BN_free(bn);

  }

};



std::unique_ptr<EC_KEY, ECKEYDeleter> parse_ec_key(FILE* file) {

    uint32_t key_len_bytes = 0;

    if (fscanf(file, " %i", &key_len_bytes) != 1) {

        return nullptr;

    }



    std::unique_ptr<EC_GROUP, void (*)(EC_GROUP*)> 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<uint8_t[]> bytes(new uint8_t[key_len_bytes]);

    std::unique_ptr<BIGNUM, BNDeleter> 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<EC_KEY, ECKEYDeleter> 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;

}



static std::vector<Certificate> IterateZipEntriesAndSearchForKeys(const ZipArchiveHandle& handle) {

  void* cookie;

  ZipString suffix("x509.pem");
@@ -603,114 +465,3 @@ bool LoadCertificateFromBuffer(const std::vector<uint8_t>& pem_content, Certific 


  return true;

}
 


// 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:

//

//  "{64,0xc926ad21,{1795090719,...,-695002876},{-857949815,...,1175080310}}"

//

// For key versions newer than the original 2048-bit e=3 keys

// supported by Android, the string is preceded by a version

// identifier, eg:

//

//  "v2 {64,0xc926ad21,{1795090719,...,-695002876},{-857949815,...,1175080310}}"

//

// (Note that the braces and commas in this example are actual

// characters the parser expects to find in the file; the ellipses

// indicate more numbers omitted from this example.)

//

// The file may contain multiple keys in this format, separated by

// commas.  The last key must not be followed by a comma.

//

// A Certificate is a pair of an RSAPublicKey and a particular hash

// (we support SHA-1 and SHA-256; we store the hash length to signify

// which is being used).  The hash used is implied by the version number.

//

//       1: 2048-bit RSA key with e=3 and SHA-1 hash

//       2: 2048-bit RSA key with e=65537 and SHA-1 hash

//       3: 2048-bit RSA key with e=3 and SHA-256 hash

//       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 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, "re"), fclose);

  if (!f) {

    PLOG(ERROR) << "error opening " << filename;

    return false;

  }



  while (true) {

    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::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::KEY_TYPE_RSA;

          exponent = 65537;

          cert.hash_len = SHA_DIGEST_LENGTH;

          break;

        case 3:

          cert.key_type = Certificate::KEY_TYPE_RSA;

          exponent = 3;

          cert.hash_len = SHA256_DIGEST_LENGTH;

          break;

        case 4:

          cert.key_type = Certificate::KEY_TYPE_RSA;

          exponent = 65537;

          cert.hash_len = SHA256_DIGEST_LENGTH;

          break;

        case 5:

          cert.key_type = Certificate::KEY_TYPE_EC;

          cert.hash_len = SHA256_DIGEST_LENGTH;

          break;

        default:

          return false;

      }

    }



    if (cert.key_type == Certificate::KEY_TYPE_RSA) {

      cert.rsa = parse_rsa_key(f.get(), exponent);

      if (!cert.rsa) {

        return false;

      }



      LOG(INFO) << "read key e=" << exponent << " hash=" << cert.hash_len;

    } else if (cert.key_type == Certificate::KEY_TYPE_EC) {

      cert.ec = parse_ec_key(f.get());

      if (!cert.ec) {

        return false;

      }

    } else {

      LOG(ERROR) << "Unknown key type " << 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 {

      LOG(ERROR) << "unexpected character between keys";

      return false;

    }

  }

  return true;

}

verifier.h

+0 −2
Original line number Diff line number Diff line
@@ -70,8 +70,6 @@ struct Certificate { 
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);



// Checks that the RSA key has a modulus of 2048 bits long, and public exponent is 3 or 65537.

bool CheckRSAKey(const std::unique_ptr<RSA, RSADeleter>& rsa);