Add EC key signing support am: 7bbe015a1b
am: 42f5bd481f
Change-Id: Idd5d8ae86fe062c7f08a41bcaa53402fd653ea02
diff --git a/Android.bp b/Android.bp
index 1be0d63..e5e592c 100644
--- a/Android.bp
+++ b/Android.bp
@@ -598,16 +598,19 @@
name: "ue_unittest_keys",
cmd: "openssl rsa -in $(location unittest_key.pem) -pubout -out $(location unittest_key.pub.pem) &&" +
"openssl rsa -in $(location unittest_key2.pem) -pubout -out $(location unittest_key2.pub.pem) &&" +
- "openssl rsa -in $(location unittest_key_RSA4096.pem) -pubout -out $(location unittest_key_RSA4096.pub.pem)",
+ "openssl rsa -in $(location unittest_key_RSA4096.pem) -pubout -out $(location unittest_key_RSA4096.pub.pem) &&" +
+ "openssl pkey -in $(location unittest_key_EC.pem) -pubout -out $(location unittest_key_EC.pub.pem)",
srcs: [
"unittest_key.pem",
"unittest_key2.pem",
"unittest_key_RSA4096.pem",
+ "unittest_key_EC.pem",
],
out: [
"unittest_key.pub.pem",
"unittest_key2.pub.pem",
"unittest_key_RSA4096.pub.pem",
+ "unittest_key_EC.pub.pem",
],
}
@@ -659,6 +662,7 @@
"unittest_key.pem",
"unittest_key2.pem",
"unittest_key_RSA4096.pem",
+ "unittest_key_EC.pem",
"update_engine.conf",
],
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 38494f2..28c11b6 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -60,6 +60,8 @@
extern const char* kUnittestPublicKeyPath;
extern const char* kUnittestPrivateKey2Path;
extern const char* kUnittestPublicKey2Path;
+extern const char* kUnittestPrivateKeyECPath;
+extern const char* kUnittestPublicKeyECPath;
static const uint32_t kDefaultKernelSize = 4096; // Something small for a test
// clang-format off
@@ -107,6 +109,7 @@
kSignatureGeneratedPlaceholder, // Insert placeholder signatures, then real.
kSignatureGeneratedPlaceholderMismatch, // Insert a wrong sized placeholder.
kSignatureGeneratedShell, // Sign the generated payload through shell cmds.
+ kSignatureGeneratedShellECKey, // Sign with a EC key through shell cmds.
kSignatureGeneratedShellBadKey, // Sign with a bad key through shell cmds.
kSignatureGeneratedShellRotateCl1, // Rotate key, test client v1
kSignatureGeneratedShellRotateCl2, // Rotate key, test client v2
@@ -164,53 +167,127 @@
return true;
}
-static size_t GetSignatureSize(const string& private_key_path) {
- const brillo::Blob data(1, 'x');
- brillo::Blob hash;
- EXPECT_TRUE(HashCalculator::RawHashOfData(data, &hash));
- brillo::Blob signature;
- EXPECT_TRUE(PayloadSigner::SignHash(hash, private_key_path, &signature));
- return signature.size();
-}
-
static bool InsertSignaturePlaceholder(size_t signature_size,
const string& payload_path,
uint64_t* out_metadata_size) {
vector<brillo::Blob> signatures;
signatures.push_back(brillo::Blob(signature_size, 0));
- return PayloadSigner::AddSignatureToPayload(
- payload_path, signatures, {}, payload_path, out_metadata_size);
+ return PayloadSigner::AddSignatureToPayload(payload_path,
+ {signature_size},
+ signatures,
+ {},
+ payload_path,
+ out_metadata_size);
}
static void SignGeneratedPayload(const string& payload_path,
uint64_t* out_metadata_size) {
string private_key_path = GetBuildArtifactsPath(kUnittestPrivateKeyPath);
- size_t signature_size = GetSignatureSize(private_key_path);
+ size_t signature_size;
+ ASSERT_TRUE(PayloadSigner::GetMaximumSignatureSize(private_key_path,
+ &signature_size));
brillo::Blob hash;
ASSERT_TRUE(PayloadSigner::HashPayloadForSigning(
payload_path, {signature_size}, &hash, nullptr));
brillo::Blob signature;
ASSERT_TRUE(PayloadSigner::SignHash(hash, private_key_path, &signature));
- ASSERT_TRUE(PayloadSigner::AddSignatureToPayload(
- payload_path, {signature}, {}, payload_path, out_metadata_size));
+ ASSERT_TRUE(PayloadSigner::AddSignatureToPayload(payload_path,
+ {signature_size},
+ {signature},
+ {},
+ payload_path,
+ out_metadata_size));
EXPECT_TRUE(PayloadSigner::VerifySignedPayload(
payload_path, GetBuildArtifactsPath(kUnittestPublicKeyPath)));
}
+static void SignGeneratedShellPayloadWithKeys(
+ const string& payload_path,
+ const vector<string>& private_key_paths,
+ const string& public_key_path,
+ bool verification_success) {
+ vector<string> signature_size_strings;
+ for (const auto& key_path : private_key_paths) {
+ size_t signature_size;
+ ASSERT_TRUE(
+ PayloadSigner::GetMaximumSignatureSize(key_path, &signature_size));
+ signature_size_strings.push_back(base::StringPrintf("%zu", signature_size));
+ }
+ string signature_size_string = base::JoinString(signature_size_strings, ":");
+
+ test_utils::ScopedTempFile hash_file("hash.XXXXXX");
+ string delta_generator_path = GetBuildArtifactsPath("delta_generator");
+ ASSERT_EQ(0,
+ System(base::StringPrintf(
+ "%s -in_file=%s -signature_size=%s -out_hash_file=%s",
+ delta_generator_path.c_str(),
+ payload_path.c_str(),
+ signature_size_string.c_str(),
+ hash_file.path().c_str())));
+
+ // Sign the hash with all private keys.
+ vector<test_utils::ScopedTempFile> sig_files;
+ vector<string> sig_file_paths;
+ for (const auto& key_path : private_key_paths) {
+ brillo::Blob hash, signature;
+ ASSERT_TRUE(utils::ReadFile(hash_file.path(), &hash));
+ ASSERT_TRUE(PayloadSigner::SignHash(hash, key_path, &signature));
+
+ test_utils::ScopedTempFile sig_file("signature.XXXXXX");
+ ASSERT_TRUE(test_utils::WriteFileVector(sig_file.path(), signature));
+ sig_file_paths.push_back(sig_file.path());
+ sig_files.push_back(std::move(sig_file));
+ }
+ string sig_files_string = base::JoinString(sig_file_paths, ":");
+
+ // Add the signature to the payload.
+ ASSERT_EQ(0,
+ System(base::StringPrintf("%s --signature_size=%s -in_file=%s "
+ "-payload_signature_file=%s -out_file=%s",
+ delta_generator_path.c_str(),
+ signature_size_string.c_str(),
+ payload_path.c_str(),
+ sig_files_string.c_str(),
+ payload_path.c_str())));
+
+ int verify_result = System(base::StringPrintf("%s -in_file=%s -public_key=%s",
+ delta_generator_path.c_str(),
+ payload_path.c_str(),
+ public_key_path.c_str()));
+
+ if (verification_success) {
+ ASSERT_EQ(0, verify_result);
+ } else {
+ ASSERT_NE(0, verify_result);
+ }
+}
+
static void SignGeneratedShellPayload(SignatureTest signature_test,
const string& payload_path) {
- string private_key_path = GetBuildArtifactsPath(kUnittestPrivateKeyPath);
+ vector<SignatureTest> supported_test = {
+ kSignatureGeneratedShell,
+ kSignatureGeneratedShellBadKey,
+ kSignatureGeneratedShellECKey,
+ kSignatureGeneratedShellRotateCl1,
+ kSignatureGeneratedShellRotateCl2,
+ };
+ ASSERT_TRUE(std::find(supported_test.begin(),
+ supported_test.end(),
+ signature_test) != supported_test.end());
+
+ string private_key_path;
if (signature_test == kSignatureGeneratedShellBadKey) {
ASSERT_TRUE(utils::MakeTempFile("key.XXXXXX", &private_key_path, nullptr));
+ } else if (signature_test == kSignatureGeneratedShellECKey) {
+ private_key_path = GetBuildArtifactsPath(kUnittestPrivateKeyECPath);
} else {
- ASSERT_TRUE(signature_test == kSignatureGeneratedShell ||
- signature_test == kSignatureGeneratedShellRotateCl1 ||
- signature_test == kSignatureGeneratedShellRotateCl2);
+ private_key_path = GetBuildArtifactsPath(kUnittestPrivateKeyPath);
}
ScopedPathUnlinker key_unlinker(private_key_path);
key_unlinker.set_should_remove(signature_test ==
kSignatureGeneratedShellBadKey);
+
// Generates a new private key that will not match the public key.
if (signature_test == kSignatureGeneratedShellBadKey) {
LOG(INFO) << "Generating a mismatched private key.";
@@ -229,64 +306,26 @@
fclose(fprikey);
RSA_free(rsa);
}
- size_t signature_size = GetSignatureSize(private_key_path);
- test_utils::ScopedTempFile hash_file("hash.XXXXXX");
- string signature_size_string;
- if (signature_test == kSignatureGeneratedShellRotateCl1 ||
- signature_test == kSignatureGeneratedShellRotateCl2)
- signature_size_string =
- base::StringPrintf("%zu:%zu", signature_size, signature_size);
- else
- signature_size_string = base::StringPrintf("%zu", signature_size);
- string delta_generator_path = GetBuildArtifactsPath("delta_generator");
- ASSERT_EQ(0,
- System(base::StringPrintf(
- "%s -in_file=%s -signature_size=%s -out_hash_file=%s",
- delta_generator_path.c_str(),
- payload_path.c_str(),
- signature_size_string.c_str(),
- hash_file.path().c_str())));
- // Sign the hash
- brillo::Blob hash, signature;
- ASSERT_TRUE(utils::ReadFile(hash_file.path(), &hash));
- ASSERT_TRUE(PayloadSigner::SignHash(hash, private_key_path, &signature));
-
- test_utils::ScopedTempFile sig_file("signature.XXXXXX");
- ASSERT_TRUE(test_utils::WriteFileVector(sig_file.path(), signature));
- string sig_files = sig_file.path();
-
- test_utils::ScopedTempFile sig_file2("signature.XXXXXX");
+ vector<string> private_key_paths = {private_key_path};
if (signature_test == kSignatureGeneratedShellRotateCl1 ||
signature_test == kSignatureGeneratedShellRotateCl2) {
- ASSERT_TRUE(PayloadSigner::SignHash(
- hash, GetBuildArtifactsPath(kUnittestPrivateKey2Path), &signature));
- ASSERT_TRUE(test_utils::WriteFileVector(sig_file2.path(), signature));
- // Append second sig file to first path
- sig_files += ":" + sig_file2.path();
+ private_key_paths.push_back(
+ GetBuildArtifactsPath(kUnittestPrivateKey2Path));
}
- ASSERT_EQ(0,
- System(base::StringPrintf(
- "%s -in_file=%s -payload_signature_file=%s -out_file=%s",
- delta_generator_path.c_str(),
- payload_path.c_str(),
- sig_files.c_str(),
- payload_path.c_str())));
- int verify_result = System(base::StringPrintf(
- "%s -in_file=%s -public_key=%s -public_key_version=%d",
- delta_generator_path.c_str(),
- payload_path.c_str(),
- (signature_test == kSignatureGeneratedShellRotateCl2
- ? GetBuildArtifactsPath(kUnittestPublicKey2Path)
- : GetBuildArtifactsPath(kUnittestPublicKeyPath))
- .c_str(),
- signature_test == kSignatureGeneratedShellRotateCl2 ? 2 : 1));
- if (signature_test == kSignatureGeneratedShellBadKey) {
- ASSERT_NE(0, verify_result);
+ std::string public_key;
+ if (signature_test == kSignatureGeneratedShellRotateCl2) {
+ public_key = GetBuildArtifactsPath(kUnittestPublicKey2Path);
+ } else if (signature_test == kSignatureGeneratedShellECKey) {
+ public_key = GetBuildArtifactsPath(kUnittestPublicKeyECPath);
} else {
- ASSERT_EQ(0, verify_result);
+ public_key = GetBuildArtifactsPath(kUnittestPublicKeyPath);
}
+
+ bool verification_success = signature_test != kSignatureGeneratedShellBadKey;
+ SignGeneratedShellPayloadWithKeys(
+ payload_path, private_key_paths, public_key, verification_success);
}
static void GenerateDeltaFile(bool full_kernel,
@@ -531,8 +570,9 @@
if (signature_test == kSignatureGeneratedPlaceholder ||
signature_test == kSignatureGeneratedPlaceholderMismatch) {
- size_t signature_size =
- GetSignatureSize(GetBuildArtifactsPath(kUnittestPrivateKeyPath));
+ size_t signature_size;
+ ASSERT_TRUE(PayloadSigner::GetMaximumSignatureSize(
+ GetBuildArtifactsPath(kUnittestPrivateKeyPath), &signature_size));
LOG(INFO) << "Inserting placeholder signature.";
ASSERT_TRUE(InsertSignaturePlaceholder(
signature_size, state->delta_path, &state->metadata_size));
@@ -555,6 +595,7 @@
LOG(INFO) << "Signing payload.";
SignGeneratedPayload(state->delta_path, &state->metadata_size);
} else if (signature_test == kSignatureGeneratedShell ||
+ signature_test == kSignatureGeneratedShellECKey ||
signature_test == kSignatureGeneratedShellBadKey ||
signature_test == kSignatureGeneratedShellRotateCl1 ||
signature_test == kSignatureGeneratedShellRotateCl2) {
@@ -597,14 +638,15 @@
else
EXPECT_EQ(1, sigs_message.signatures_size());
const Signatures::Signature& signature = sigs_message.signatures(0);
- EXPECT_EQ(1U, signature.version());
- uint64_t expected_sig_data_length = 0;
vector<string> key_paths{GetBuildArtifactsPath(kUnittestPrivateKeyPath)};
- if (signature_test == kSignatureGeneratedShellRotateCl1 ||
- signature_test == kSignatureGeneratedShellRotateCl2) {
+ if (signature_test == kSignatureGeneratedShellECKey) {
+ key_paths = {GetBuildArtifactsPath(kUnittestPrivateKeyECPath)};
+ } else if (signature_test == kSignatureGeneratedShellRotateCl1 ||
+ signature_test == kSignatureGeneratedShellRotateCl2) {
key_paths.push_back(GetBuildArtifactsPath(kUnittestPrivateKey2Path));
}
+ uint64_t expected_sig_data_length = 0;
EXPECT_TRUE(PayloadSigner::SignatureBlobLength(
key_paths, &expected_sig_data_length));
EXPECT_EQ(expected_sig_data_length, manifest.signatures_size());
@@ -717,7 +759,9 @@
ASSERT_TRUE(PayloadSigner::GetMetadataSignature(
state->delta.data(),
state->metadata_size,
- GetBuildArtifactsPath(kUnittestPrivateKeyPath),
+ (signature_test == kSignatureGeneratedShellECKey)
+ ? GetBuildArtifactsPath(kUnittestPrivateKeyECPath)
+ : GetBuildArtifactsPath(kUnittestPrivateKeyPath),
&install_plan->payloads[0].metadata_signature));
EXPECT_FALSE(install_plan->payloads[0].metadata_signature.empty());
@@ -728,7 +772,9 @@
install_plan,
&install_plan->payloads[0],
false /* interactive */);
- string public_key_path = GetBuildArtifactsPath(kUnittestPublicKeyPath);
+ string public_key_path = signature_test == kSignatureGeneratedShellECKey
+ ? GetBuildArtifactsPath(kUnittestPublicKeyECPath)
+ : GetBuildArtifactsPath(kUnittestPublicKeyPath);
EXPECT_TRUE(utils::FileExists(public_key_path.c_str()));
(*performer)->set_public_key_path(public_key_path);
@@ -1060,6 +1106,17 @@
}
TEST(DeltaPerformerIntegrationTest,
+ RunAsRootSmallImageSignGeneratedShellECKeyTest) {
+ DoSmallImageTest(false,
+ false,
+ false,
+ -1,
+ kSignatureGeneratedShellECKey,
+ false,
+ kInPlaceMinorPayloadVersion);
+}
+
+TEST(DeltaPerformerIntegrationTest,
RunAsRootSmallImageSignGeneratedShellBadKeyTest) {
DoSmallImageTest(false,
false,
diff --git a/payload_consumer/payload_verifier.cc b/payload_consumer/payload_verifier.cc
index b2c5be4..02eeb76 100644
--- a/payload_consumer/payload_verifier.cc
+++ b/payload_consumer/payload_verifier.cc
@@ -88,7 +88,19 @@
// Tries every signature in the signature blob.
for (int i = 0; i < signatures.signatures_size(); i++) {
const Signatures::Signature& signature = signatures.signatures(i);
- brillo::Blob sig_data(signature.data().begin(), signature.data().end());
+ brillo::Blob sig_data;
+ if (signature.has_unpadded_signature_size()) {
+ TEST_AND_RETURN_FALSE(signature.unpadded_signature_size() <=
+ signature.data().size());
+ LOG(INFO) << "Truncating the signature to its unpadded size: "
+ << signature.unpadded_signature_size() << ".";
+ sig_data.assign(
+ signature.data().begin(),
+ signature.data().begin() + signature.unpadded_signature_size());
+ } else {
+ sig_data.assign(signature.data().begin(), signature.data().end());
+ }
+
brillo::Blob sig_hash_data;
if (VerifyRawSignature(sig_data, sha256_hash_data, &sig_hash_data)) {
LOG(INFO) << "Verified correct signature " << i + 1 << " out of "
@@ -102,7 +114,7 @@
LOG(ERROR) << "None of the " << signatures.signatures_size()
<< " signatures is correct. Expected hash before padding:";
utils::HexDumpVector(sha256_hash_data);
- LOG(ERROR) << "But found decrypted hashes:";
+ LOG(ERROR) << "But found RSA decrypted hashes:";
for (const auto& sig_hash_data : tested_hashes) {
utils::HexDumpVector(sig_hash_data);
}
@@ -116,20 +128,35 @@
TEST_AND_RETURN_FALSE(public_key_ != nullptr);
int key_type = EVP_PKEY_id(public_key_.get());
- TEST_AND_RETURN_FALSE(key_type == EVP_PKEY_RSA);
- brillo::Blob sig_hash_data;
- TEST_AND_RETURN_FALSE(
- GetRawHashFromSignature(sig_data, public_key_.get(), &sig_hash_data));
+ if (key_type == EVP_PKEY_RSA) {
+ brillo::Blob sig_hash_data;
+ TEST_AND_RETURN_FALSE(
+ GetRawHashFromSignature(sig_data, public_key_.get(), &sig_hash_data));
- if (decrypted_sig_data != nullptr) {
- *decrypted_sig_data = sig_hash_data;
+ if (decrypted_sig_data != nullptr) {
+ *decrypted_sig_data = sig_hash_data;
+ }
+
+ brillo::Blob padded_hash_data = sha256_hash_data;
+ TEST_AND_RETURN_FALSE(
+ PadRSASHA256Hash(&padded_hash_data, sig_hash_data.size()));
+
+ return padded_hash_data == sig_hash_data;
}
- brillo::Blob padded_hash_data = sha256_hash_data;
- TEST_AND_RETURN_FALSE(
- PadRSASHA256Hash(&padded_hash_data, sig_hash_data.size()));
+ if (key_type == EVP_PKEY_EC) {
+ EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(public_key_.get());
+ TEST_AND_RETURN_FALSE(ec_key != nullptr);
+ return ECDSA_verify(0,
+ sha256_hash_data.data(),
+ sha256_hash_data.size(),
+ sig_data.data(),
+ sig_data.size(),
+ ec_key) == 1;
+ }
- return padded_hash_data == sig_hash_data;
+ LOG(ERROR) << "Unsupported key type " << key_type;
+ return false;
}
bool PayloadVerifier::GetRawHashFromSignature(
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index bef09bb..1323534 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -63,9 +63,6 @@
bool parsing_successful = base::StringToSizeT(str, &size);
LOG_IF(FATAL, !parsing_successful) << "Invalid signature size: " << str;
- LOG_IF(FATAL, size != 256 && size != 512)
- << "Only signature sizes of 256 or 512 bytes are supported.";
-
signature_sizes->push_back(size);
}
}
@@ -138,6 +135,7 @@
void SignPayload(const string& in_file,
const string& out_file,
+ const vector<size_t>& signature_sizes,
const string& payload_signature_file,
const string& metadata_signature_file,
const string& out_metadata_size_file) {
@@ -151,6 +149,7 @@
SignatureFileFlagToBlobs(metadata_signature_file, &metadata_signatures);
uint64_t final_metadata_size;
CHECK(PayloadSigner::AddSignatureToPayload(in_file,
+ signature_sizes,
payload_signatures,
metadata_signatures,
out_file,
@@ -461,6 +460,7 @@
if (!FLAGS_payload_signature_file.empty()) {
SignPayload(FLAGS_in_file,
FLAGS_out_file,
+ signature_sizes,
FLAGS_payload_signature_file,
FLAGS_metadata_signature_file,
FLAGS_out_metadata_size_file);
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 3c9ce95..72780b1 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -47,23 +47,29 @@
namespace chromeos_update_engine {
namespace {
-
-// The payload verifier will check all the signatures included in the payload
-// regardless of the version field. Old version of the verifier require the
-// version field to be included and be 1.
-const uint32_t kSignatureMessageLegacyVersion = 1;
-
// Given raw |signatures|, packs them into a protobuf and serializes it into a
// string. Returns true on success, false otherwise.
bool ConvertSignaturesToProtobuf(const vector<brillo::Blob>& signatures,
+ const vector<size_t>& padded_signature_sizes,
string* out_serialized_signature) {
+ TEST_AND_RETURN_FALSE(signatures.size() == padded_signature_sizes.size());
// Pack it into a protobuf
Signatures out_message;
- for (const brillo::Blob& signature : signatures) {
+ for (size_t i = 0; i < signatures.size(); i++) {
+ const auto& signature = signatures[i];
+ const auto& padded_signature_size = padded_signature_sizes[i];
+ TEST_AND_RETURN_FALSE(padded_signature_size >= signature.size());
Signatures::Signature* sig_message = out_message.add_signatures();
- // Set all the signatures with the same version number.
- sig_message->set_version(kSignatureMessageLegacyVersion);
- sig_message->set_data(signature.data(), signature.size());
+ // Skip assigning the same version number because we don't need to be
+ // compatible with old major version 1 client anymore.
+
+ // TODO(Xunchang) don't need to set the unpadded_signature_size field for
+ // RSA key signed signatures.
+ sig_message->set_unpadded_signature_size(signature.size());
+ brillo::Blob padded_signature = signature;
+ padded_signature.insert(
+ padded_signature.end(), padded_signature_size - signature.size(), 0);
+ sig_message->set_data(padded_signature.data(), padded_signature.size());
}
// Serialize protobuf
@@ -204,8 +210,35 @@
return true;
}
+std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)> CreatePrivateKeyFromPath(
+ const string& private_key_path) {
+ FILE* fprikey = fopen(private_key_path.c_str(), "rb");
+ if (!fprikey) {
+ PLOG(ERROR) << "Failed to read " << private_key_path;
+ return {nullptr, nullptr};
+ }
+
+ auto private_key = std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>(
+ PEM_read_PrivateKey(fprikey, nullptr, nullptr, nullptr), EVP_PKEY_free);
+ fclose(fprikey);
+ return private_key;
+}
+
} // namespace
+bool PayloadSigner::GetMaximumSignatureSize(const string& private_key_path,
+ size_t* signature_size) {
+ *signature_size = 0;
+ auto private_key = CreatePrivateKeyFromPath(private_key_path);
+ if (!private_key) {
+ LOG(ERROR) << "Failed to create private key from " << private_key_path;
+ return false;
+ }
+
+ *signature_size = EVP_PKEY_size(private_key.get());
+ return true;
+}
+
void PayloadSigner::AddSignatureToManifest(uint64_t signature_blob_offset,
uint64_t signature_blob_length,
bool add_dummy_op,
@@ -283,13 +316,11 @@
// openssl rsautl -raw -sign -inkey |private_key_path|
// -in |padded_hash| -out |out_signature|
- FILE* fprikey = fopen(private_key_path.c_str(), "rb");
- TEST_AND_RETURN_FALSE(fprikey != nullptr);
-
- std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)> private_key(
- PEM_read_PrivateKey(fprikey, nullptr, nullptr, nullptr), EVP_PKEY_free);
- fclose(fprikey);
- TEST_AND_RETURN_FALSE(private_key != nullptr);
+ auto private_key = CreatePrivateKeyFromPath(private_key_path);
+ if (!private_key) {
+ LOG(ERROR) << "Failed to create private key from " << private_key_path;
+ return false;
+ }
int key_type = EVP_PKEY_id(private_key.get());
brillo::Blob signature;
@@ -314,6 +345,28 @@
}
TEST_AND_RETURN_FALSE(static_cast<size_t>(signature_size) ==
signature.size());
+ } else if (key_type == EVP_PKEY_EC) {
+ EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(private_key.get());
+ TEST_AND_RETURN_FALSE(ec_key != nullptr);
+
+ signature.resize(ECDSA_size(ec_key));
+ unsigned int signature_size;
+ if (ECDSA_sign(0,
+ hash.data(),
+ hash.size(),
+ signature.data(),
+ &signature_size,
+ ec_key) != 1) {
+ LOG(ERROR) << "Signing hash failed: "
+ << ERR_error_string(ERR_get_error(), nullptr);
+ return false;
+ }
+
+ // NIST P-256
+ LOG(ERROR) << "signature max size " << signature.size() << " size "
+ << signature_size;
+ TEST_AND_RETURN_FALSE(signature.size() >= signature_size);
+ signature.resize(signature_size);
} else {
LOG(ERROR) << "key_type " << key_type << " isn't supported for signing";
return false;
@@ -326,13 +379,19 @@
const vector<string>& private_key_paths,
string* out_serialized_signature) {
vector<brillo::Blob> signatures;
+ vector<size_t> padded_signature_sizes;
for (const string& path : private_key_paths) {
brillo::Blob signature;
TEST_AND_RETURN_FALSE(SignHash(hash_data, path, &signature));
signatures.push_back(signature);
+
+ size_t padded_signature_size;
+ TEST_AND_RETURN_FALSE(
+ GetMaximumSignatureSize(path, &padded_signature_size));
+ padded_signature_sizes.push_back(padded_signature_size);
}
- TEST_AND_RETURN_FALSE(
- ConvertSignaturesToProtobuf(signatures, out_serialized_signature));
+ TEST_AND_RETURN_FALSE(ConvertSignaturesToProtobuf(
+ signatures, padded_signature_sizes, out_serialized_signature));
return true;
}
@@ -379,7 +438,8 @@
signatures.emplace_back(signature_size, 0);
}
string signature;
- TEST_AND_RETURN_FALSE(ConvertSignaturesToProtobuf(signatures, &signature));
+ TEST_AND_RETURN_FALSE(
+ ConvertSignaturesToProtobuf(signatures, signature_sizes, &signature));
brillo::Blob payload;
uint64_t metadata_size, signatures_offset;
@@ -403,6 +463,7 @@
bool PayloadSigner::AddSignatureToPayload(
const string& payload_path,
+ const vector<size_t>& padded_signature_sizes,
const vector<brillo::Blob>& payload_signatures,
const vector<brillo::Blob>& metadata_signatures,
const string& signed_payload_path,
@@ -411,11 +472,11 @@
// Loads the payload and adds the signature op to it.
string payload_signature, metadata_signature;
- TEST_AND_RETURN_FALSE(
- ConvertSignaturesToProtobuf(payload_signatures, &payload_signature));
+ TEST_AND_RETURN_FALSE(ConvertSignaturesToProtobuf(
+ payload_signatures, padded_signature_sizes, &payload_signature));
if (!metadata_signatures.empty()) {
- TEST_AND_RETURN_FALSE(
- ConvertSignaturesToProtobuf(metadata_signatures, &metadata_signature));
+ TEST_AND_RETURN_FALSE(ConvertSignaturesToProtobuf(
+ metadata_signatures, padded_signature_sizes, &metadata_signature));
}
brillo::Blob payload;
uint64_t signatures_offset;
diff --git a/payload_generator/payload_signer.h b/payload_generator/payload_signer.h
index 76e583b..bd1e32f 100644
--- a/payload_generator/payload_signer.h
+++ b/payload_generator/payload_signer.h
@@ -105,6 +105,7 @@
// otherwise.
static bool AddSignatureToPayload(
const std::string& payload_path,
+ const std::vector<size_t>& padded_signature_sizes,
const std::vector<brillo::Blob>& payload_signatures,
const std::vector<brillo::Blob>& metadata_signatures,
const std::string& signed_payload_path,
@@ -122,6 +123,13 @@
static bool ExtractPayloadProperties(const std::string& payload_path,
brillo::KeyValueStore* properties);
+ // This function calculates the maximum size, in bytes, of a signature signed
+ // by private_key_path. For an RSA key, this returns the number of bytes
+ // needed to represent the modulus. For an EC key, this returns the maximum
+ // size of a DER-encoded ECDSA signature.
+ static bool GetMaximumSignatureSize(const std::string& private_key_path,
+ size_t* signature_size);
+
private:
// This should never be constructed
DISALLOW_IMPLICIT_CONSTRUCTORS(PayloadSigner);
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 457b34c..bf7100b 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -46,6 +46,8 @@
const char* kUnittestPublicKey2Path = "unittest_key2.pub.pem";
const char* kUnittestPrivateKeyRSA4096Path = "unittest_key_RSA4096.pem";
const char* kUnittestPublicKeyRSA4096Path = "unittest_key_RSA4096.pub.pem";
+const char* kUnittestPrivateKeyECPath = "unittest_key_EC.pem";
+const char* kUnittestPublicKeyECPath = "unittest_key_EC.pub.pem";
// Some data and its corresponding hash and signature:
const char kDataToSign[] = "This is some data to sign.";
@@ -115,7 +117,6 @@
EXPECT_TRUE(signatures.ParseFromString(signature));
EXPECT_EQ(1, signatures.signatures_size());
const Signatures::Signature& sig = signatures.signatures(0);
- EXPECT_EQ(1U, sig.version());
const string& sig_data = sig.data();
ASSERT_EQ(arraysize(kDataSignature), sig_data.size());
for (size_t i = 0; i < arraysize(kDataSignature); i++) {
@@ -128,12 +129,14 @@
SignSampleData(&signature,
{GetBuildArtifactsPath(kUnittestPrivateKeyPath),
GetBuildArtifactsPath(kUnittestPrivateKey2Path),
- GetBuildArtifactsPath(kUnittestPrivateKeyRSA4096Path)});
+ GetBuildArtifactsPath(kUnittestPrivateKeyRSA4096Path),
+ GetBuildArtifactsPath(kUnittestPrivateKeyECPath)});
// Either public key should pass the verification.
for (const auto& path : {kUnittestPublicKeyPath,
kUnittestPublicKey2Path,
- kUnittestPublicKeyRSA4096Path}) {
+ kUnittestPublicKeyRSA4096Path,
+ kUnittestPublicKeyECPath}) {
string public_key;
EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(path), &public_key));
auto payload_verifier = PayloadVerifier::CreateInstance(public_key);
diff --git a/unittest_key_EC.pem b/unittest_key_EC.pem
new file mode 100644
index 0000000..9e65a68
--- /dev/null
+++ b/unittest_key_EC.pem
@@ -0,0 +1,5 @@
+-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGaguGj8Yb1KkqKHd
+ISblUsjtOCbzAuVpX81i02sm8FWhRANCAARBnuotwKOsuvjH6iwTDhOAi7Q5pLWz
+xDkZjg2pcfbfi9FFTvLYETas7B2W6fx9PUezUmHTFTDV2JZuMYYFdZOw
+-----END PRIVATE KEY-----
diff --git a/update_metadata.proto b/update_metadata.proto
index 1657a7e..9bc0d8a 100644
--- a/update_metadata.proto
+++ b/update_metadata.proto
@@ -126,8 +126,17 @@
message Signatures {
message Signature {
- optional uint32 version = 1;
+ optional uint32 version = 1 [deprecated = true];
optional bytes data = 2;
+
+ // The DER encoded signature size of EC keys is nondeterministic for
+ // different input of sha256 hash. However, we need the size of the
+ // serialized signatures protobuf string to be fixed before signing;
+ // because this size is part of the content to be signed. Therefore, we
+ // always pad the signature data to the maximum possible signature size of
+ // a given key. And the payload verifier will truncate the signature to
+ // its correct size based on the value of |unpadded_signature_size|.
+ optional fixed32 unpadded_signature_size = 3;
}
repeated Signature signatures = 1;
}