Add EC key signing support
The DER encoded signature size of ECDSA with P-256 NIST CURVE is
nondeterministic for different input of sha256 hash. For example,
the signature size can be 70, 71, 72 bytes with the maximum
possible size of 72 bytes. However, we need the size of the
serialized signatures protobuf string to be fixed before signing;
because the size is part of the content to be signed.
To achieve that, we can add padding to the signature; and update the
definition of the signature proto to include the unpadded signature
size.
message Signatures {
message Signature {
optional uint32 version = 1;
optional bytes data = 2;
optional fixed32 unpadded_signature_size = 3;
}
repeated Signature signatures = 1;
}
Therefore the payload verifier will read the unpadded signature
and use it to verify against the public keys. For RSA signatures, the
signature data already has the correct size. So the legacy update_engine
on the old devices will still be able to verify these signatures in new
proto format.
We also need to update the version in signature proto, and the minor
version of update_engine.
The EC key in the unittest is generated with the command:
openssl ecparam -name prime256v1 -genkey -noout -out prime256v1-key.pem
openssl pkey -in prime256v1-key.pem -out unittest_key_EC.pem
Bug: 141244025
Test: unit tests pass, sign a package with EC key and and install on sailfish
Change-Id: I0a16c9f2f2c7fe9ccc1070c87fbbd6b94bc1f542
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;
}