Store raw payload hash blob in install plan.
am: 2703ef4466
Change-Id: I1387743ff791993ff6cfcb21d044daf5871b2c87
diff --git a/common/hash_calculator.cc b/common/hash_calculator.cc
index de6e0f9..ebfdb6e 100644
--- a/common/hash_calculator.cc
+++ b/common/hash_calculator.cc
@@ -20,7 +20,6 @@
#include <base/logging.h>
#include <base/posix/eintr_wrapper.h>
-#include <brillo/data_encoding.h>
#include "update_engine/common/utils.h"
@@ -37,7 +36,7 @@
// Mostly just passes the data through to OpenSSL's SHA256_Update()
bool HashCalculator::Update(const void* data, size_t length) {
TEST_AND_RETURN_FALSE(valid_);
- TEST_AND_RETURN_FALSE(hash_.empty());
+ TEST_AND_RETURN_FALSE(raw_hash_.empty());
static_assert(sizeof(size_t) <= sizeof(unsigned long), // NOLINT(runtime/int)
"length param may be truncated in SHA256_Update");
TEST_AND_RETURN_FALSE(SHA256_Update(&ctx_, data, length) == 1);
@@ -73,16 +72,11 @@
}
// Call Finalize() when all data has been passed in. This mostly just
-// calls OpenSSL's SHA256_Final() and then base64 encodes the hash.
+// calls OpenSSL's SHA256_Final().
bool HashCalculator::Finalize() {
- TEST_AND_RETURN_FALSE(hash_.empty());
TEST_AND_RETURN_FALSE(raw_hash_.empty());
raw_hash_.resize(SHA256_DIGEST_LENGTH);
TEST_AND_RETURN_FALSE(SHA256_Final(raw_hash_.data(), &ctx_) == 1);
-
- // Convert raw_hash_ to base64 encoding and store it in hash_.
- hash_ = brillo::data_encoding::Base64Encode(raw_hash_.data(),
- raw_hash_.size());
return true;
}
@@ -115,21 +109,6 @@
return res;
}
-string HashCalculator::HashOfBytes(const void* data, size_t length) {
- HashCalculator calc;
- calc.Update(data, length);
- calc.Finalize();
- return calc.hash();
-}
-
-string HashCalculator::HashOfString(const string& str) {
- return HashOfBytes(str.data(), str.size());
-}
-
-string HashCalculator::HashOfData(const brillo::Blob& data) {
- return HashOfBytes(data.data(), data.size());
-}
-
string HashCalculator::GetContext() const {
return string(reinterpret_cast<const char*>(&ctx_), sizeof(ctx_));
}
diff --git a/common/hash_calculator.h b/common/hash_calculator.h
index f749585..06d2cfb 100644
--- a/common/hash_calculator.h
+++ b/common/hash_calculator.h
@@ -27,11 +27,11 @@
#include <base/macros.h>
#include <brillo/secure_blob.h>
-// Omaha uses base64 encoded SHA-256 as the hash. This class provides a simple
-// wrapper around OpenSSL providing such a formatted hash of data passed in.
+// This class provides a simple wrapper around OpenSSL providing a hash of data
+// passed in.
// The methods of this class must be called in a very specific order: First the
// ctor (of course), then 0 or more calls to Update(), then Finalize(), then 0
-// or more calls to hash().
+// or more calls to raw_hash().
namespace chromeos_update_engine {
@@ -50,17 +50,10 @@
off_t UpdateFile(const std::string& name, off_t length);
// Call Finalize() when all data has been passed in. This method tells
- // OpenSSl that no more data will come in and base64 encodes the resulting
- // hash.
+ // OpenSSL that no more data will come in.
// Returns true on success.
bool Finalize();
- // Gets the hash. Finalize() must have been called.
- const std::string& hash() const {
- DCHECK(!hash_.empty()) << "Call Finalize() first";
- return hash_;
- }
-
const brillo::Blob& raw_hash() const {
DCHECK(!raw_hash_.empty()) << "Call Finalize() first";
return raw_hash_;
@@ -83,15 +76,9 @@
static off_t RawHashOfFile(const std::string& name, off_t length,
brillo::Blob* out_hash);
- // Used by tests
- static std::string HashOfBytes(const void* data, size_t length);
- static std::string HashOfString(const std::string& str);
- static std::string HashOfData(const brillo::Blob& data);
-
private:
- // If non-empty, the final base64 encoded hash and the raw hash. Will only be
- // set to non-empty when Finalize is called.
- std::string hash_;
+ // If non-empty, the final raw hash. Will only be set to non-empty when
+ // Finalize is called.
brillo::Blob raw_hash_;
// Init success
diff --git a/common/hash_calculator_unittest.cc b/common/hash_calculator_unittest.cc
index 436e6a7..233237b 100644
--- a/common/hash_calculator_unittest.cc
+++ b/common/hash_calculator_unittest.cc
@@ -22,6 +22,7 @@
#include <string>
#include <vector>
+#include <brillo/data_encoding.h>
#include <brillo/secure_blob.h>
#include <gtest/gtest.h>
@@ -33,9 +34,8 @@
namespace chromeos_update_engine {
// Generated by running this on a linux shell:
-// $ echo -n hi | openssl dgst -sha256 -binary | openssl base64
-static const char kExpectedHash[] =
- "j0NDRmSPa5bfid2pAcUXaxCm2Dlh3TwayItZstwyeqQ=";
+// $ echo -n hi | openssl dgst -sha256 -binary |
+// hexdump -v -e '" " 12/1 "0x%02x, " "\n"'
static const uint8_t kExpectedRawHash[] = {
0x8f, 0x43, 0x43, 0x46, 0x64, 0x8f, 0x6b, 0x96,
0xdf, 0x89, 0xdd, 0xa9, 0x01, 0xc5, 0x17, 0x6b,
@@ -52,7 +52,6 @@
HashCalculator calc;
calc.Update("hi", 2);
calc.Finalize();
- EXPECT_EQ(kExpectedHash, calc.hash());
brillo::Blob raw_hash(std::begin(kExpectedRawHash),
std::end(kExpectedRawHash));
EXPECT_TRUE(raw_hash == calc.raw_hash());
@@ -63,7 +62,6 @@
calc.Update("h", 1);
calc.Update("i", 1);
calc.Finalize();
- EXPECT_EQ(kExpectedHash, calc.hash());
brillo::Blob raw_hash(std::begin(kExpectedRawHash),
std::end(kExpectedRawHash));
EXPECT_TRUE(raw_hash == calc.raw_hash());
@@ -78,7 +76,6 @@
calc_next.SetContext(calc_context);
calc_next.Update("i", 1);
calc_next.Finalize();
- EXPECT_EQ(kExpectedHash, calc_next.hash());
brillo::Blob raw_hash(std::begin(kExpectedRawHash),
std::end(kExpectedRawHash));
EXPECT_TRUE(raw_hash == calc_next.raw_hash());
@@ -106,7 +103,8 @@
// echo -n $C
// let C=C+1
// done | openssl dgst -sha256 -binary | openssl base64
- EXPECT_EQ("NZf8k6SPBkYMvhaX8YgzuMgbkLP1XZ+neM8K5wcSsf8=", calc.hash());
+ EXPECT_EQ("NZf8k6SPBkYMvhaX8YgzuMgbkLP1XZ+neM8K5wcSsf8=",
+ brillo::data_encoding::Base64Encode(calc.raw_hash()));
}
TEST_F(HashCalculatorTest, UpdateFileSimpleTest) {
@@ -121,7 +119,6 @@
HashCalculator calc;
EXPECT_EQ(2, calc.UpdateFile(data_path, kLengths[i]));
EXPECT_TRUE(calc.Finalize());
- EXPECT_EQ(kExpectedHash, calc.hash());
brillo::Blob raw_hash(std::begin(kExpectedRawHash),
std::end(kExpectedRawHash));
EXPECT_TRUE(raw_hash == calc.raw_hash());
@@ -132,7 +129,8 @@
EXPECT_EQ(1, calc.UpdateFile(data_path, 1));
EXPECT_TRUE(calc.Finalize());
// echo -n h | openssl dgst -sha256 -binary | openssl base64
- EXPECT_EQ("qqlAJmTxpB9A67xSyZk+tmrrNmYClY/fqig7ceZNsSM=", calc.hash());
+ EXPECT_EQ("qqlAJmTxpB9A67xSyZk+tmrrNmYClY/fqig7ceZNsSM=",
+ brillo::data_encoding::Base64Encode(calc.raw_hash()));
}
TEST_F(HashCalculatorTest, RawHashOfFileSimpleTest) {
diff --git a/common/utils.cc b/common/utils.cc
index ea748c1..f528660 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -944,8 +944,16 @@
return str;
}
-string CalculateP2PFileId(const string& payload_hash, size_t payload_size) {
- string encoded_hash = brillo::data_encoding::Base64Encode(payload_hash);
+// The P2P file id should be the same for devices running new version and old
+// version so that they can share it with each other. The hash in the response
+// was base64 encoded, but now that we switched to use "hash_sha256" field which
+// is hex encoded, we have to convert them back to base64 for P2P. However, the
+// base64 encoded hash was base64 encoded here again historically for some
+// reason, so we keep the same behavior here.
+string CalculateP2PFileId(const brillo::Blob& payload_hash,
+ size_t payload_size) {
+ string encoded_hash = brillo::data_encoding::Base64Encode(
+ brillo::data_encoding::Base64Encode(payload_hash));
return base::StringPrintf("cros_update_size_%" PRIuS "_hash_%s",
payload_size,
encoded_hash.c_str());
diff --git a/common/utils.h b/common/utils.h
index 8cccc24..eaf2640 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -53,7 +53,7 @@
std::string StringVectorToString(const std::vector<std::string> &vec_str);
// Calculates the p2p file id from payload hash and size
-std::string CalculateP2PFileId(const std::string& payload_hash,
+std::string CalculateP2PFileId(const brillo::Blob& payload_hash,
size_t payload_size);
// Parse the firmware version from one line of output from the
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index b06de09..94e615d 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -70,7 +70,6 @@
static const char* kTagMoreInfo = "MoreInfo";
// Deprecated: "NeedsAdmin"
static const char* kTagPrompt = "Prompt";
-static const char* kTagSha256 = "sha256";
static const char* kTagDisableP2PForDownloading = "DisableP2PForDownloading";
static const char* kTagDisableP2PForSharing = "DisableP2PForSharing";
static const char* kTagPublicKeyRsa = "PublicKeyRsa";
@@ -360,6 +359,7 @@
vector<string> url_codebase;
string package_name;
string package_size;
+ string package_hash;
string manifest_version;
map<string, string> action_postinstall_attrs;
};
@@ -420,6 +420,7 @@
// Only look at the first <package>.
data->package_name = attrs["name"];
data->package_size = attrs["size"];
+ data->package_hash = attrs["hash_sha256"];
} else if (data->current_path == "/response/app/updatecheck/manifest") {
// Get the version.
data->manifest_version = attrs[kTagVersion];
@@ -870,6 +871,13 @@
LOG(INFO) << "Payload size = " << output_object->size << " bytes";
+ output_object->hash = parser_data->package_hash;
+ if (output_object->hash.empty()) {
+ LOG(ERROR) << "Omaha Response has empty hash_sha256 value";
+ completer->set_code(ErrorCode::kOmahaResponseInvalid);
+ return false;
+ }
+
return true;
}
@@ -893,13 +901,6 @@
return false;
}
- output_object->hash = attrs[kTagSha256];
- if (output_object->hash.empty()) {
- LOG(ERROR) << "Omaha Response has empty sha256 value";
- completer->set_code(ErrorCode::kOmahaResponseInvalid);
- return false;
- }
-
// Get the optional properties one by one.
output_object->more_info_url = attrs[kTagMoreInfo];
output_object->metadata_size = ParseInt(attrs[kTagMetadataSize]);
@@ -1133,10 +1134,13 @@
next_data_offset + next_data_length;
}
- string file_id = utils::CalculateP2PFileId(response.hash, response.size);
+ brillo::Blob raw_hash;
+ if (!base::HexStringToBytes(response.hash, &raw_hash))
+ return;
+ string file_id = utils::CalculateP2PFileId(raw_hash, response.size);
if (system_state_->p2p_manager()) {
- LOG(INFO) << "Checking if payload is available via p2p, file_id="
- << file_id << " minimum_size=" << minimum_size;
+ LOG(INFO) << "Checking if payload is available via p2p, file_id=" << file_id
+ << " minimum_size=" << minimum_size;
system_state_->p2p_manager()->LookupUrlForFile(
file_id,
minimum_size,
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 1c1d25c..74e5361 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -89,34 +89,48 @@
}
string GetUpdateResponse() const {
- return
- "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
- "protocol=\"3.0\">"
- "<daystart elapsed_seconds=\"100\"" +
- (elapsed_days.empty() ? "" : (" elapsed_days=\"" + elapsed_days + "\""))
- + "/>"
- "<app appid=\"" + app_id + "\" " +
- (include_cohorts ? "cohort=\"" + cohort + "\" cohorthint=\"" +
- cohorthint + "\" cohortname=\"" + cohortname + "\" " : "") +
- " status=\"ok\">"
- "<ping status=\"ok\"/><updatecheck status=\"ok\">"
- "<urls><url codebase=\"" + codebase + "\"/></urls>"
- "<manifest version=\"" + version + "\">"
- "<packages><package hash=\"not-used\" name=\"" + filename + "\" "
- "size=\"" + base::Int64ToString(size) + "\"/></packages>"
- "<actions><action event=\"postinstall\" "
- "ChromeOSVersion=\"" + version + "\" "
- "MoreInfo=\"" + more_info_url + "\" Prompt=\"" + prompt + "\" "
- "IsDelta=\"true\" "
- "IsDeltaPayload=\"true\" "
- "MaxDaysToScatter=\"" + max_days_to_scatter + "\" "
- "sha256=\"" + hash + "\" "
- "needsadmin=\"" + needsadmin + "\" " +
- (deadline.empty() ? "" : ("deadline=\"" + deadline + "\" ")) +
- (disable_p2p_for_downloading ?
- "DisableP2PForDownloading=\"true\" " : "") +
- (disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
- "/></actions></manifest></updatecheck></app></response>";
+ return "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
+ "protocol=\"3.0\">"
+ "<daystart elapsed_seconds=\"100\"" +
+ (elapsed_days.empty() ? ""
+ : (" elapsed_days=\"" + elapsed_days + "\"")) +
+ "/>"
+ "<app appid=\"" +
+ app_id + "\" " +
+ (include_cohorts
+ ? "cohort=\"" + cohort + "\" cohorthint=\"" + cohorthint +
+ "\" cohortname=\"" + cohortname + "\" "
+ : "") +
+ " status=\"ok\">"
+ "<ping status=\"ok\"/><updatecheck status=\"ok\">"
+ "<urls><url codebase=\"" +
+ codebase +
+ "\"/></urls>"
+ "<manifest version=\"" +
+ version +
+ "\">"
+ "<packages><package hash=\"not-used\" name=\"" +
+ filename + "\" size=\"" + base::Int64ToString(size) +
+ "\" hash_sha256=\"" + hash +
+ "\"/></packages>"
+ "<actions><action event=\"postinstall\" "
+ "ChromeOSVersion=\"" +
+ version + "\" MoreInfo=\"" + more_info_url + "\" Prompt=\"" +
+ prompt +
+ "\" "
+ "IsDelta=\"true\" "
+ "IsDeltaPayload=\"true\" "
+ "MaxDaysToScatter=\"" +
+ max_days_to_scatter +
+ "\" "
+ "sha256=\"not-used\" "
+ "needsadmin=\"" +
+ needsadmin + "\" " +
+ (deadline.empty() ? "" : ("deadline=\"" + deadline + "\" ")) +
+ (disable_p2p_for_downloading ? "DisableP2PForDownloading=\"true\" "
+ : "") +
+ (disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
+ "/></actions></manifest></updatecheck></app></response>";
}
// Return the payload URL, which is split in two fields in the XML response.
@@ -130,7 +144,7 @@
string prompt = "true";
string codebase = "http://code/base/";
string filename = "file.signed";
- string hash = "HASH1234=";
+ string hash = "4841534831323334";
string needsadmin = "false";
int64_t size = 123;
string deadline = "";
@@ -1041,13 +1055,13 @@
"<urls><url codebase=\"http://missing/field/test/\"/></urls>"
"<manifest version=\"10.2.3.4\">"
"<packages><package hash=\"not-used\" name=\"f\" "
- "size=\"587\"/></packages>"
+ "size=\"587\" hash_sha256=\"lkq34j5345\"/></packages>"
"<actions><action event=\"postinstall\" "
"ChromeOSVersion=\"10.2.3.4\" "
"Prompt=\"false\" "
"IsDelta=\"true\" "
"IsDeltaPayload=\"false\" "
- "sha256=\"lkq34j5345\" "
+ "sha256=\"not-used\" "
"needsadmin=\"true\" "
"/></actions></manifest></updatecheck></app></response>";
LOG(INFO) << "Input Response = " << input_response;
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 33380d7..1bfd353 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -19,6 +19,7 @@
#include <string>
#include <base/logging.h>
+#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
#include <policy/device_policy.h>
@@ -86,7 +87,12 @@
// Fill up the other properties based on the response.
install_plan_.payload_size = response.size;
- install_plan_.payload_hash = response.hash;
+ if (!base::HexStringToBytes(response.hash, &install_plan_.payload_hash)) {
+ LOG(ERROR) << "Failed to convert payload hash from hex string to bytes: "
+ << response.hash;
+ completer.set_code(ErrorCode::kOmahaResponseInvalid);
+ return;
+ }
install_plan_.metadata_size = response.metadata_size;
install_plan_.metadata_signature = response.metadata_signature;
install_plan_.public_key_rsa = response.public_key_rsa;
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 60b139b..473eaf8 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -59,6 +59,8 @@
InstallPlan* out);
FakeSystemState fake_system_state_;
+ // "Hash+"
+ const brillo::Blob expected_hash_ = {0x48, 0x61, 0x73, 0x68, 0x2b};
};
class OmahaResponseHandlerActionProcessorDelegate
@@ -90,6 +92,7 @@
"very_long_name_and_no_slashes-very_long_name_and_no_slashes"
"-the_update_a.b.c.d_DELTA_.tgz";
const char* const kBadVersion = "don't update me";
+const char* const kPayloadHashHex = "486173682b";
} // namespace
bool OmahaResponseHandlerActionTest::DoTest(
@@ -148,14 +151,14 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.more_info_url = "http://more/info";
- in.hash = "HASH+";
+ in.hash = kPayloadHashHex;
in.size = 12;
in.prompt = false;
in.deadline = "20101020";
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_EQ(1U, install_plan.target_slot);
string deadline;
EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline));
@@ -173,7 +176,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
in.prompt = true;
InstallPlan install_plan;
@@ -181,7 +184,7 @@
fake_system_state_.fake_boot_control()->SetCurrentSlot(1);
EXPECT_TRUE(DoTest(in, test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_EQ(0U, install_plan.target_slot);
string deadline;
EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline) &&
@@ -194,7 +197,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back(kLongName);
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
in.prompt = true;
in.deadline = "some-deadline";
@@ -202,7 +205,7 @@
fake_system_state_.fake_boot_control()->SetCurrentSlot(0);
EXPECT_TRUE(DoTest(in, test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_EQ(1U, install_plan.target_slot);
string deadline;
EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline));
@@ -225,7 +228,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("http://test.should/need/hash.checks.signed");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
// Hash checks are always skipped for non-official update URLs.
EXPECT_CALL(*(fake_system_state_.mock_request_params()),
@@ -234,7 +237,7 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
EXPECT_EQ(in.version, install_plan.version);
}
@@ -245,7 +248,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("http://url.normally/needs/hash.checks.signed");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
EXPECT_CALL(*(fake_system_state_.mock_request_params()),
IsUpdateUrlOfficial())
@@ -253,7 +256,7 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_FALSE(install_plan.hash_checks_mandatory);
EXPECT_EQ(in.version, install_plan.version);
}
@@ -266,7 +269,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("http://url.normally/needs/hash.checks.signed");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
EXPECT_CALL(*(fake_system_state_.mock_request_params()),
IsUpdateUrlOfficial())
@@ -275,7 +278,7 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_FALSE(install_plan.hash_checks_mandatory);
EXPECT_EQ(in.version, install_plan.version);
}
@@ -286,7 +289,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("https://test.should.not/need/hash.checks.signed");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
EXPECT_CALL(*(fake_system_state_.mock_request_params()),
IsUpdateUrlOfficial())
@@ -294,7 +297,7 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_FALSE(install_plan.hash_checks_mandatory);
EXPECT_EQ(in.version, install_plan.version);
}
@@ -306,7 +309,7 @@
in.payload_urls.push_back("http://test.should.still/need/hash.checks");
in.payload_urls.push_back("https://test.should.still/need/hash.checks");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
EXPECT_CALL(*(fake_system_state_.mock_request_params()),
IsUpdateUrlOfficial())
@@ -314,7 +317,7 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
EXPECT_EQ(in.version, install_plan.version);
}
@@ -325,7 +328,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("https://MoreStableChannelTest");
in.more_info_url = "http://more/info";
- in.hash = "HASHjk";
+ in.hash = kPayloadHashHex;
in.size = 15;
// Create a uniquely named test directory.
@@ -360,7 +363,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("https://LessStableChannelTest");
in.more_info_url = "http://more/info";
- in.hash = "HASHjk";
+ in.hash = kPayloadHashHex;
in.size = 15;
// Create a uniquely named test directory.
@@ -395,7 +398,7 @@
in.version = "a.b.c.d";
in.payload_urls.push_back("https://would.not/cause/hash/checks");
in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
+ in.hash = kPayloadHashHex;
in.size = 12;
OmahaRequestParams params(&fake_system_state_);
@@ -416,8 +419,8 @@
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "", &install_plan));
- EXPECT_EQ(in.hash, install_plan.payload_hash);
- EXPECT_EQ(install_plan.download_url, p2p_url);
+ EXPECT_EQ(expected_hash_, install_plan.payload_hash);
+ EXPECT_EQ(p2p_url, install_plan.download_url);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
}
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index e442441..21299d7 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -1361,14 +1361,13 @@
LOG(INFO) << "Verifying metadata hash signature using public key: "
<< path_to_public_key.value();
- HashCalculator metadata_hasher;
- metadata_hasher.Update(payload.data(), metadata_size_);
- if (!metadata_hasher.Finalize()) {
+ brillo::Blob calculated_metadata_hash;
+ if (!HashCalculator::RawHashOfBytes(
+ payload.data(), metadata_size_, &calculated_metadata_hash)) {
LOG(ERROR) << "Unable to compute actual hash of manifest";
return ErrorCode::kDownloadMetadataSignatureVerificationError;
}
- brillo::Blob calculated_metadata_hash = metadata_hasher.raw_hash();
PayloadVerifier::PadRSA2048SHA256Hash(&calculated_metadata_hash);
if (calculated_metadata_hash.empty()) {
LOG(ERROR) << "Computed actual hash of metadata is empty.";
@@ -1515,15 +1514,14 @@
(operation.data_sha256_hash().data() +
operation.data_sha256_hash().size()));
- HashCalculator operation_hasher;
- operation_hasher.Update(buffer_.data(), operation.data_length());
- if (!operation_hasher.Finalize()) {
+ brillo::Blob calculated_op_hash;
+ if (!HashCalculator::RawHashOfBytes(
+ buffer_.data(), operation.data_length(), &calculated_op_hash)) {
LOG(ERROR) << "Unable to compute actual hash of operation "
<< next_operation_num_;
return ErrorCode::kDownloadOperationHashVerificationError;
}
- brillo::Blob calculated_op_hash = operation_hasher.raw_hash();
if (calculated_op_hash != expected_op_hash) {
LOG(ERROR) << "Hash verification failed for operation "
<< next_operation_num_ << ". Expected hash = ";
@@ -1546,7 +1544,7 @@
} while (0);
ErrorCode DeltaPerformer::VerifyPayload(
- const string& update_check_response_hash,
+ const brillo::Blob& update_check_response_hash,
const uint64_t update_check_response_size) {
// See if we should use the public RSA key in the Omaha response.
@@ -1568,11 +1566,11 @@
buffer_offset_);
// Verifies the payload hash.
- const string& payload_hash_data = payload_hash_calculator_.hash();
TEST_AND_RETURN_VAL(ErrorCode::kDownloadPayloadVerificationError,
- !payload_hash_data.empty());
- TEST_AND_RETURN_VAL(ErrorCode::kPayloadHashMismatchError,
- payload_hash_data == update_check_response_hash);
+ !payload_hash_calculator_.raw_hash().empty());
+ TEST_AND_RETURN_VAL(
+ ErrorCode::kPayloadHashMismatchError,
+ payload_hash_calculator_.raw_hash() == update_check_response_hash);
// Verifies the signed payload hash.
if (!utils::FileExists(path_to_public_key.value().c_str())) {
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 71d7178..7fe2cd2 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -113,13 +113,13 @@
bool IsManifestValid();
// Verifies the downloaded payload against the signed hash included in the
- // payload, against the update check hash (which is in base64 format) and
- // size using the public key and returns ErrorCode::kSuccess on success, an
- // error code on failure. This method should be called after closing the
- // stream. Note this method skips the signed hash check if the public key is
- // unavailable; it returns ErrorCode::kSignedDeltaPayloadExpectedError if the
- // public key is available but the delta payload doesn't include a signature.
- ErrorCode VerifyPayload(const std::string& update_check_response_hash,
+ // payload, against the update check hash and size using the public key and
+ // returns ErrorCode::kSuccess on success, an error code on failure.
+ // This method should be called after closing the stream. Note this method
+ // skips the signed hash check if the public key is unavailable; it returns
+ // ErrorCode::kSignedDeltaPayloadExpectedError if the public key is available
+ // but the delta payload doesn't include a signature.
+ ErrorCode VerifyPayload(const brillo::Blob& update_check_response_hash,
const uint64_t update_check_response_size);
// Converts an ordered collection of Extent objects which contain data of
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index afbb8dc..e87a907 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -854,11 +854,11 @@
int expected_times = (expected_result == ErrorCode::kSuccess) ? 1 : 0;
EXPECT_CALL(state->mock_delegate_, DownloadComplete()).Times(expected_times);
- LOG(INFO) << "Verifying payload for expected result "
- << expected_result;
- EXPECT_EQ(expected_result, performer->VerifyPayload(
- HashCalculator::HashOfData(state->delta),
- state->delta.size()));
+ LOG(INFO) << "Verifying payload for expected result " << expected_result;
+ brillo::Blob expected_hash;
+ HashCalculator::RawHashOfData(state->delta, &expected_hash);
+ EXPECT_EQ(expected_result,
+ performer->VerifyPayload(expected_hash, state->delta.size()));
LOG(INFO) << "Verified payload.";
if (expected_result != ErrorCode::kSuccess) {
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 084848e..65ae1ab 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -23,7 +23,6 @@
#include <vector>
#include <base/files/file_path.h>
-#include <base/strings/stringprintf.h>
#include "update_engine/common/action_pipe.h"
#include "update_engine/common/boot_control_interface.h"
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 5e9ef5c..4392b74 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -139,13 +139,13 @@
0, writer.Open(output_temp_file.path().c_str(), O_WRONLY | O_CREAT, 0));
writer.set_fail_write(fail_write);
- // We pull off the first byte from data and seek past it.
- string hash = HashCalculator::HashOfBytes(&data[1], data.size() - 1);
uint64_t size = data.size();
InstallPlan install_plan;
install_plan.payload_type = InstallPayloadType::kDelta;
install_plan.payload_size = size;
- install_plan.payload_hash = hash;
+ // We pull off the first byte from data and seek past it.
+ EXPECT_TRUE(HashCalculator::RawHashOfBytes(
+ &data[1], data.size() - 1, &install_plan.payload_hash));
install_plan.source_slot = 0;
install_plan.target_slot = 1;
// We mark both slots as bootable. Only the target slot should be unbootable
@@ -371,7 +371,7 @@
// takes ownership of passed in HttpFetcher
InstallPlan install_plan;
install_plan.payload_size = 1;
- install_plan.payload_hash = HashCalculator::HashOfString("x");
+ EXPECT_TRUE(HashCalculator::RawHashOfData({'x'}, &install_plan.payload_hash));
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
MockPrefs prefs;
@@ -456,7 +456,7 @@
0, writer.Open(output_temp_file.path().c_str(), O_WRONLY | O_CREAT, 0));
InstallPlan install_plan;
install_plan.payload_size = data_.length();
- install_plan.payload_hash = "1234hash";
+ install_plan.payload_hash = {'1', '2', '3', '4', 'h', 'a', 's', 'h'};
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
MockPrefs prefs;
@@ -569,7 +569,8 @@
// Prepare the file with existing data before starting to write to
// it via DownloadAction.
- string file_id = utils::CalculateP2PFileId("1234hash", data_.length());
+ string file_id = utils::CalculateP2PFileId(
+ {'1', '2', '3', '4', 'h', 'a', 's', 'h'}, data_.length());
ASSERT_TRUE(p2p_manager_->FileShare(file_id, data_.length()));
string existing_data;
for (unsigned int i = 0; i < 1000; i++)
@@ -606,7 +607,8 @@
// Prepare the file with all existing data before starting to write
// to it via DownloadAction.
- string file_id = utils::CalculateP2PFileId("1234hash", data_.length());
+ string file_id = utils::CalculateP2PFileId(
+ {'1', '2', '3', '4', 'h', 'a', 's', 'h'}, data_.length());
ASSERT_TRUE(p2p_manager_->FileShare(file_id, data_.length()));
string existing_data;
for (unsigned int i = 0; i < 1000; i++)
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 5156f96..4f30582 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -34,16 +34,13 @@
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_consumer/payload_constants.h"
+using brillo::data_encoding::Base64Encode;
using std::string;
namespace chromeos_update_engine {
namespace {
const off_t kReadFileBufferSize = 128 * 1024;
-
-string StringForHashBytes(const brillo::Blob& hash) {
- return brillo::data_encoding::Base64Encode(hash.data(), hash.size());
-}
} // namespace
void FilesystemVerifierAction::PerformAction() {
@@ -199,7 +196,8 @@
}
InstallPlan::Partition& partition =
install_plan_.partitions[partition_index_];
- LOG(INFO) << "Hash of " << partition.name << ": " << hasher_->hash();
+ LOG(INFO) << "Hash of " << partition.name << ": "
+ << Base64Encode(hasher_->raw_hash());
switch (verifier_step_) {
case VerifierStep::kVerifyTargetHash:
@@ -231,9 +229,9 @@
" means that the delta I've been given doesn't match my"
" existing system. The "
<< partition.name << " partition I have has hash: "
- << StringForHashBytes(hasher_->raw_hash())
+ << Base64Encode(hasher_->raw_hash())
<< " but the update expected me to have "
- << StringForHashBytes(partition.source_hash) << " .";
+ << Base64Encode(partition.source_hash) << " .";
LOG(INFO) << "To get the checksum of the " << partition.name
<< " partition run this command: dd if="
<< partition.source_path
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index b04da74..fff0ac2 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -18,6 +18,7 @@
#include <base/format_macros.h>
#include <base/logging.h>
+#include <base/strings/string_number_conversions.h>
#include <base/strings/stringprintf.h>
#include "update_engine/common/utils.h"
@@ -68,19 +69,17 @@
utils::ToString(partition.run_postinstall).c_str());
}
- LOG(INFO) << "InstallPlan: "
- << (is_resume ? "resume" : "new_update")
+ LOG(INFO) << "InstallPlan: " << (is_resume ? "resume" : "new_update")
<< ", payload type: " << InstallPayloadTypeToString(payload_type)
<< ", source_slot: " << BootControlInterface::SlotName(source_slot)
<< ", target_slot: " << BootControlInterface::SlotName(target_slot)
- << ", url: " << download_url
- << ", payload size: " << payload_size
- << ", payload hash: " << payload_hash
+ << ", url: " << download_url << ", payload size: " << payload_size
+ << ", payload hash: "
+ << base::HexEncode(payload_hash.data(), payload_hash.size())
<< ", metadata size: " << metadata_size
- << ", metadata signature: " << metadata_signature
- << partitions_str
- << ", hash_checks_mandatory: " << utils::ToString(
- hash_checks_mandatory)
+ << ", metadata signature: " << metadata_signature << partitions_str
+ << ", hash_checks_mandatory: "
+ << utils::ToString(hash_checks_mandatory)
<< ", powerwash_required: " << utils::ToString(powerwash_required);
}
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 3f0005c..0e25cc3 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -57,7 +57,7 @@
std::string version; // version we are installing.
uint64_t payload_size{0}; // size of the payload
- std::string payload_hash; // SHA256 hash of the payload
+ brillo::Blob payload_hash; // SHA256 hash of the payload
uint64_t metadata_size{0}; // size of the metadata
std::string metadata_signature; // signature of the metadata
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 045d52f..e928912 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -31,6 +31,7 @@
#include <base/format_macros.h>
#include <base/strings/stringprintf.h>
#include <base/threading/simple_thread.h>
+#include <brillo/data_encoding.h>
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/subprocess.h"
@@ -845,7 +846,8 @@
TEST_AND_RETURN_FALSE(hasher.Finalize());
const brillo::Blob& hash = hasher.raw_hash();
info->set_hash(hash.data(), hash.size());
- LOG(INFO) << part.path << ": size=" << part.size << " hash=" << hasher.hash();
+ LOG(INFO) << part.path << ": size=" << part.size
+ << " hash=" << brillo::data_encoding::Base64Encode(hash);
return true;
}
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index ddf6f0a..ddb2833 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -312,10 +312,8 @@
bool PayloadFile::AddOperationHash(InstallOperation* op,
const brillo::Blob& buf) {
- HashCalculator hasher;
- TEST_AND_RETURN_FALSE(hasher.Update(buf.data(), buf.size()));
- TEST_AND_RETURN_FALSE(hasher.Finalize());
- const brillo::Blob& hash = hasher.raw_hash();
+ brillo::Blob hash;
+ TEST_AND_RETURN_FALSE(HashCalculator::RawHashOfData(buf, &hash));
op->set_data_sha256_hash(hash.data(), hash.size());
return true;
}
diff --git a/update_attempter.cc b/update_attempter.cc
index 8e25819..ba103bf 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -32,6 +32,7 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include <brillo/bind_lambda.h>
+#include <brillo/data_encoding.h>
#include <brillo/errors/error_codes.h>
#include <brillo/make_unique_ptr.h>
#include <brillo/message_loops/message_loop.h>
@@ -939,7 +940,8 @@
// Generate an unique payload identifier.
const string target_version_uid =
- install_plan.payload_hash + ":" + install_plan.metadata_signature;
+ brillo::data_encoding::Base64Encode(install_plan.payload_hash) + ":" +
+ install_plan.metadata_signature;
// Expect to reboot into the new version to send the proper metric during
// next boot.
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index dcdd989..6b055af 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -24,6 +24,7 @@
#include <base/logging.h>
#include <base/strings/string_number_conversions.h>
#include <brillo/bind_lambda.h>
+#include <brillo/data_encoding.h>
#include <brillo/message_loops/message_loop.h>
#include <brillo/strings/string_utils.h>
@@ -151,7 +152,11 @@
install_plan_.payload_size = 0;
}
}
- install_plan_.payload_hash = headers[kPayloadPropertyFileHash];
+ if (!brillo::data_encoding::Base64Decode(headers[kPayloadPropertyFileHash],
+ &install_plan_.payload_hash)) {
+ LOG(WARNING) << "Unable to decode base64 file hash: "
+ << headers[kPayloadPropertyFileHash];
+ }
if (!base::StringToUint64(headers[kPayloadPropertyMetadataSize],
&install_plan_.metadata_size)) {
install_plan_.metadata_size = 0;