update_engine: drop manual padding

this padding was initially required for using sha256+rsa2048. It's not
needed anymore as we can rely on the RSA_PKCS1_PADDING

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

Author: tormath1

src/update_engine/payload_processor.cc

@@ -361,7 +361,6 @@ ActionExitCode PayloadProcessor::VerifyPayload() {
   TEST_AND_RETURN_VAL(kActionCodeDownloadPayloadPubKeyVerificationError,
                       signed_hasher.Finalize());
   vector<char> hash_data = signed_hasher.raw_hash();
-  PayloadSigner::PadRSA2048SHA256Hash(&hash_data);
   TEST_AND_RETURN_VAL(kActionCodeDownloadPayloadPubKeyVerificationError,
                       !hash_data.empty());
   if (hash_data != signed_hash_data) {

src/update_engine/payload_processor_unittest.cc

@@ -204,7 +204,6 @@ static void SignGeneratedShellPayload(SignatureTest signature_test,
   // Pad the hash
   vector<char> hash;
   ASSERT_TRUE(utils::ReadFile(hash_file, &hash));
-  ASSERT_TRUE(PayloadSigner::PadRSA2048SHA256Hash(&hash));
   ASSERT_TRUE(WriteFileVector(hash_file, hash));
 
   string sig_file;

src/update_engine/payload_signer.cc

@@ -24,59 +24,6 @@ const uint32_t kSignatureMessageCurrentVersion = 2;
 
 namespace {
 
-// The following is a standard PKCS1-v1_5 padding for SHA256 signatures, as
-// defined in RFC3447. It is prepended to the actual signature (32 bytes) to
-// form a sequence of 256 bytes (2048 bits) that is amenable to RSA signing. The
-// padded hash will look as follows:
-//
-//    0x00 0x01 0xff ... 0xff 0x00  ASN1HEADER  SHA256HASH
-//   |--------------205-----------||----19----||----32----|
-//
-// where ASN1HEADER is the ASN.1 description of the signed data. The complete 51
-// bytes of actual data (i.e. the ASN.1 header complete with the hash) are
-// packed as follows:
-//
-//  SEQUENCE(2+49) {
-//   SEQUENCE(2+13) {
-//    OBJECT(2+9) id-sha256
-//    NULL(2+0)
-//   }
-//   OCTET STRING(2+32) <actual signature bytes...>
-//  }
-const unsigned char kRSA2048SHA256Padding[] = {
-  // PKCS1-v1_5 padding
-  0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-  0xff, 0xff, 0xff, 0xff, 0x00,
-  // ASN.1 header
-  0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86,
-  0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05,
-  0x00, 0x04, 0x20,
-};
-
 // Given raw |signatures|, packs them into a protobuf and serializes it into a
 // binary blob. Returns true on success, false otherwise.
 bool ConvertSignatureToProtobufBlob(const vector<vector<char> >& signatures,
@@ -179,7 +126,6 @@ bool PayloadSigner::SignHash(const vector<char>& hash,
   // We expect unpadded SHA256 hash coming in
   TEST_AND_RETURN_FALSE(hash.size() == 32);
   vector<char> padded_hash(hash);
-  PadRSA2048SHA256Hash(&padded_hash);
   TEST_AND_RETURN_FALSE(utils::WriteFile(hash_path.c_str(),
                                          padded_hash.data(),
                                          padded_hash.size()));
@@ -309,7 +255,7 @@ bool PayloadSigner::GetRawHashFromSignature(
   EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(pkey, NULL);
   if (!ctx
       || EVP_PKEY_verify_recover_init(ctx) <= 0
-      || EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_NO_PADDING) <= 0
+      || EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) <= 0
       || EVP_PKEY_CTX_set_signature_md(ctx, EVP_sha256()) <= 0) {
     LOG(ERROR) << "Couldn't initialise EVP_PKEY_CTX";
     EVP_PKEY_free(pkey);
@@ -359,7 +305,6 @@ bool PayloadSigner::VerifySignedPayload(const std::string& payload_path,
   vector<char> hash;
   TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(
       payload.data(), metadata_size + manifest.signatures_offset(), &hash));
-  PadRSA2048SHA256Hash(&hash);
   TEST_AND_RETURN_FALSE(hash == signed_hash);
   return true;
 }
@@ -432,14 +377,4 @@ bool PayloadSigner::AddSignatureToPayload(
   return true;
 }
 
-bool PayloadSigner::PadRSA2048SHA256Hash(std::vector<char>* hash) {
-  TEST_AND_RETURN_FALSE(hash->size() == 32);
-  hash->insert(hash->begin(),
-               reinterpret_cast<const char*>(kRSA2048SHA256Padding),
-               reinterpret_cast<const char*>(kRSA2048SHA256Padding +
-                                             sizeof(kRSA2048SHA256Padding)));
-  TEST_AND_RETURN_FALSE(hash->size() == 256);
-  return true;
-}
-
 }  // namespace chromeos_update_engine

src/update_engine/payload_signer.h

@@ -113,13 +113,6 @@ class PayloadSigner {
                                   const std::string& public_key_path,
                                   uint32_t client_key_check_version);
 
-  // Pads a SHA256 hash so that it may be encrypted/signed with RSA2048
-  // using the PKCS#1 v1.5 scheme.
-  // hash should be a pointer to vector of exactly 256 bits. The vector
-  // will be modified in place and will result in having a length of
-  // 2048 bits. Returns true on success, false otherwise.
-  static bool PadRSA2048SHA256Hash(std::vector<char>* hash);
-
   // Reads the payload from the given |payload_path| into the |out_payload|
   // vector. It also parses the manifest protobuf in the payload and returns it
   // in |out_manifest| along with the size of the entire metadata in

src/update_engine/payload_signer_unittest.cc

@@ -130,7 +130,6 @@ TEST(PayloadSignerTest, VerifySignatureTest) {
   vector<char> padded_hash_data(reinterpret_cast<const char *>(kDataHash),
                                 reinterpret_cast<const char *>(kDataHash +
                                                          sizeof(kDataHash)));
-  PayloadSigner::PadRSA2048SHA256Hash(&padded_hash_data);
   ASSERT_EQ(padded_hash_data.size(), hash_data.size());
   for (size_t i = 0; i < padded_hash_data.size(); i++) {
     EXPECT_EQ(padded_hash_data[i], hash_data[i]);