AU: detect failure when using public key verification
This makes public key verification non-fatal. A future CL will use
this knowledge to report statistics.
Change-Id: I9440155dd71621662e5c0f4011b3001bbc65e6d7
BUG=chromium-os:13341
TEST=unittests
Review URL: http://codereview.chromium.org/6778029
diff --git a/delta_performer.cc b/delta_performer.cc
index 54a69fb..b5d1cff 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -569,10 +569,22 @@
return true;
}
+#define TEST_SET_TRUE_RET_TRUE(_ptr, _condition) \
+ do { \
+ if (!(_condition)) { \
+ LOG(ERROR) << "Non fatal public key verification: " << #_condition; \
+ if (_ptr) { \
+ *(_ptr) = true; \
+ } \
+ return true; \
+ } \
+ } while(0)
+
bool DeltaPerformer::VerifyPayload(
const string& public_key_path,
const std::string& update_check_response_hash,
- const uint64_t update_check_response_size) {
+ const uint64_t update_check_response_size,
+ bool* signature_failed) {
string key_path = public_key_path;
if (key_path.empty()) {
key_path = kUpdatePayloadPublicKeyPath;
@@ -593,21 +605,35 @@
LOG(WARNING) << "Not verifying signed delta payload -- missing public key.";
return true;
}
- TEST_AND_RETURN_FALSE(!signatures_message_data_.empty());
+ TEST_SET_TRUE_RET_TRUE(signature_failed, !signatures_message_data_.empty());
vector<char> signed_hash_data;
- TEST_AND_RETURN_FALSE(PayloadSigner::VerifySignature(signatures_message_data_,
- key_path,
- &signed_hash_data));
+ TEST_SET_TRUE_RET_TRUE(signature_failed, PayloadSigner::VerifySignature(
+ signatures_message_data_,
+ key_path,
+ &signed_hash_data));
OmahaHashCalculator signed_hasher;
- TEST_AND_RETURN_FALSE(signed_hasher.SetContext(signed_hash_context_));
- TEST_AND_RETURN_FALSE(signed_hasher.Finalize());
+ TEST_SET_TRUE_RET_TRUE(signature_failed,
+ signed_hasher.SetContext(signed_hash_context_));
+ TEST_SET_TRUE_RET_TRUE(signature_failed,
+ signed_hasher.Finalize());
vector<char> hash_data = signed_hasher.raw_hash();
PayloadSigner::PadRSA2048SHA256Hash(&hash_data);
- TEST_AND_RETURN_FALSE(!hash_data.empty());
- TEST_AND_RETURN_FALSE(hash_data == signed_hash_data);
+ TEST_SET_TRUE_RET_TRUE(signature_failed, !hash_data.empty());
+ if (hash_data != signed_hash_data) {
+ LOG(ERROR) << "Public key verificaion failed. This is non-fatal. "
+ "Attached Signature:";
+ utils::HexDumpVector(signed_hash_data);
+ LOG(ERROR) << "Computed Signature:";
+ utils::HexDumpVector(hash_data);
+ if (signature_failed) {
+ *signature_failed = true;
+ }
+ }
return true;
}
+#undef TEST_SET_TRUE_RET_TRUE
+
bool DeltaPerformer::GetNewPartitionInfo(uint64_t* kernel_size,
vector<char>* kernel_hash,
uint64_t* rootfs_size,
diff --git a/delta_performer.h b/delta_performer.h
index 7cc1d9b..a80e180 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -67,9 +67,12 @@
// is unavailable; it returns false if the public key is available but the
// delta payload doesn't include a signature. If |public_key_path| is an empty
// string, uses the default public key path.
+ // If the signature check fails, *signature_failed will be set to true (if
+ // it's non-NULL); this will not cause the method to fail.
bool VerifyPayload(const std::string& public_key_path,
const std::string& update_check_response_hash,
- const uint64_t update_check_response_size);
+ const uint64_t update_check_response_size,
+ bool* signature_failed);
// Reads from the update manifest the expected sizes and hashes of the target
// kernel and rootfs partitions. These values can be used for applied update
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 0c2d9b9..9c926d5 100755
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -450,18 +450,21 @@
strlen(new_data_string)));
EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
- bool expect_verify_success =
- signature_test != kSignatureNone &&
- signature_test != kSignatureGeneratedShellBadKey;
- EXPECT_EQ(expect_verify_success,
- performer.VerifyPayload(
- kUnittestPublicKeyPath,
- OmahaHashCalculator::OmahaHashOfData(delta),
- delta.size()));
+ const bool expect_public_verify_failure =
+ signature_test == kSignatureNone ||
+ signature_test == kSignatureGeneratedShellBadKey;
+ bool public_verify_failure = false;
+ EXPECT_TRUE(performer.VerifyPayload(
+ kUnittestPublicKeyPath,
+ OmahaHashCalculator::OmahaHashOfData(delta),
+ delta.size(),
+ &public_verify_failure));
+ EXPECT_EQ(expect_public_verify_failure, public_verify_failure);
EXPECT_TRUE(performer.VerifyPayload(
"/public/key/does/not/exists",
OmahaHashCalculator::OmahaHashOfData(delta),
- delta.size()));
+ delta.size(),
+ NULL));
uint64_t new_kernel_size;
vector<char> new_kernel_hash;
diff --git a/download_action.cc b/download_action.cc
index d6ae97d..9577047 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -163,7 +163,8 @@
if (!install_plan_.is_full_update) {
if (!delta_performer_->VerifyPayload("",
install_plan_.download_hash,
- install_plan_.size)) {
+ install_plan_.size,
+ NULL)) {
LOG(ERROR) << "Download of " << install_plan_.download_url
<< " failed due to payload verification error.";
code = kActionCodeDownloadPayloadVerificationError;