Merge commit '7438410328547d8f55b6555ebdcfd9768978fcb9' into patch_branch
Change-Id: Idc4f0d63865479d4e3b85069270c6c21d52a747d
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index 66767fb..00ea128 100644
--- a/common/http_fetcher_unittest.cc
+++ b/common/http_fetcher_unittest.cc
@@ -19,6 +19,7 @@
#include <sys/socket.h>
#include <unistd.h>
+#include <algorithm>
#include <memory>
#include <string>
#include <utility>
@@ -218,6 +219,7 @@
virtual bool IsMock() const = 0;
virtual bool IsMulti() const = 0;
virtual bool IsHttpSupported() const = 0;
+ virtual bool IsFileFetcher() const = 0;
virtual void IgnoreServerAborting(HttpServer* server) const {}
@@ -251,6 +253,7 @@
bool IsMock() const override { return true; }
bool IsMulti() const override { return false; }
bool IsHttpSupported() const override { return true; }
+ bool IsFileFetcher() const override { return false; }
HttpServer* CreateServer() override {
return new NullHttpServer;
@@ -292,6 +295,7 @@
bool IsMock() const override { return false; }
bool IsMulti() const override { return false; }
bool IsHttpSupported() const override { return true; }
+ bool IsFileFetcher() const override { return false; }
void IgnoreServerAborting(HttpServer* server) const override {
// Nothing to do.
@@ -342,6 +346,17 @@
}
string BigUrl(in_port_t port) const override {
+ static string big_contents = []() {
+ string buf;
+ buf.reserve(kBigLength);
+ constexpr const char* kBigUrlContent = "abcdefghij";
+ for (size_t i = 0; i < kBigLength; i += strlen(kBigUrlContent)) {
+ buf.append(kBigUrlContent,
+ std::min(kBigLength - i, strlen(kBigUrlContent)));
+ }
+ return buf;
+ }();
+ test_utils::WriteFileString(temp_file_.path(), big_contents);
return "file://" + temp_file_.path();
}
string SmallUrl(in_port_t port) const override {
@@ -355,6 +370,7 @@
bool IsMock() const override { return false; }
bool IsMulti() const override { return false; }
bool IsHttpSupported() const override { return false; }
+ bool IsFileFetcher() const override { return true; }
void IgnoreServerAborting(HttpServer* server) const override {}
@@ -364,6 +380,31 @@
test_utils::ScopedTempFile temp_file_{"ue_file_fetcher.XXXXXX"};
};
+class MultiRangeHttpFetcherOverFileFetcherTest : public FileFetcherTest {
+ public:
+ // Necessary to unhide the definition in the base class.
+ using AnyHttpFetcherTest::NewLargeFetcher;
+ HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override {
+ MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher(new FileFetcher());
+ ret->ClearRanges();
+ // FileFetcher doesn't support range with unspecified length.
+ ret->AddRange(0, 1);
+ // Speed up test execution.
+ ret->set_idle_seconds(1);
+ ret->set_retry_seconds(1);
+ fake_hardware_.SetIsOfficialBuild(false);
+ return ret;
+ }
+
+ // Necessary to unhide the definition in the base class.
+ using AnyHttpFetcherTest::NewSmallFetcher;
+ HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
+ return NewLargeFetcher(proxy_resolver);
+ }
+
+ bool IsMulti() const override { return true; }
+};
+
//
// Infrastructure for type tests of HTTP fetcher.
// See: http://code.google.com/p/googletest/wiki/AdvancedGuide#Typed_Tests
@@ -401,7 +442,8 @@
typedef ::testing::Types<LibcurlHttpFetcherTest,
MockHttpFetcherTest,
MultiRangeHttpFetcherTest,
- FileFetcherTest>
+ FileFetcherTest,
+ MultiRangeHttpFetcherOverFileFetcherTest>
HttpFetcherTestTypes;
TYPED_TEST_CASE(HttpFetcherTest, HttpFetcherTestTypes);
@@ -1160,6 +1202,26 @@
vector<pair<off_t, off_t>> ranges;
ranges.push_back(make_pair(0, 25));
+ ranges.push_back(make_pair(99, 17));
+ MultiTest(this->test_.NewLargeFetcher(),
+ this->test_.fake_hardware(),
+ this->test_.BigUrl(server->GetPort()),
+ ranges,
+ "abcdefghijabcdefghijabcdejabcdefghijabcdef",
+ 25 + 17,
+ this->test_.IsFileFetcher() ? kHttpResponseOk
+ : kHttpResponsePartialContent);
+}
+
+TYPED_TEST(HttpFetcherTest, MultiHttpFetcherUnspecifiedEndTest) {
+ if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
+ return;
+
+ unique_ptr<HttpServer> server(this->test_.CreateServer());
+ ASSERT_TRUE(server->started_);
+
+ vector<pair<off_t, off_t>> ranges;
+ ranges.push_back(make_pair(0, 25));
ranges.push_back(make_pair(99, 0));
MultiTest(this->test_.NewLargeFetcher(),
this->test_.fake_hardware(),
@@ -1185,11 +1247,12 @@
ranges,
"abcdefghijabcdefghijabcd",
24,
- kHttpResponsePartialContent);
+ this->test_.IsFileFetcher() ? kHttpResponseOk
+ : kHttpResponsePartialContent);
}
TYPED_TEST(HttpFetcherTest, MultiHttpFetcherMultiEndTest) {
- if (!this->test_.IsMulti())
+ if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
return;
unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1235,7 +1298,7 @@
// (1) successful recovery: The offset fetch will fail twice but succeed with
// the third proxy.
TYPED_TEST(HttpFetcherTest, MultiHttpFetcherErrorIfOffsetRecoverableTest) {
- if (!this->test_.IsMulti())
+ if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
return;
unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1258,7 +1321,7 @@
// (2) unsuccessful recovery: The offset fetch will fail repeatedly. The
// fetcher will signal a (failed) completed transfer to the delegate.
TYPED_TEST(HttpFetcherTest, MultiHttpFetcherErrorIfOffsetUnrecoverableTest) {
- if (!this->test_.IsMulti())
+ if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
return;
unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1290,15 +1353,17 @@
size_t length) override {
LOG(INFO) << "ReceivedBytes, " << length << " bytes.";
EXPECT_EQ(fetcher, fetcher_.get());
+ bool should_terminate = false;
if (bytes_downloaded_ < terminate_trigger_bytes_ &&
bytes_downloaded_ + length >= terminate_trigger_bytes_) {
MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&HttpFetcher::TerminateTransfer,
base::Unretained(fetcher_.get())));
+ should_terminate = true;
}
bytes_downloaded_ += length;
- return true;
+ return !should_terminate;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
diff --git a/common/multi_range_http_fetcher.cc b/common/multi_range_http_fetcher.cc
index d39b7f9..230106d 100644
--- a/common/multi_range_http_fetcher.cc
+++ b/common/multi_range_http_fetcher.cc
@@ -99,10 +99,13 @@
range.length() - bytes_received_this_range_);
}
LOG_IF(WARNING, next_size <= 0) << "Asked to write length <= 0";
+ // bytes_received_this_range_ needs to be updated regardless of the delegate_
+ // result, because it will be used to determine a successful transfer in
+ // TransferEnded().
+ bytes_received_this_range_ += length;
if (delegate_ && !delegate_->ReceivedBytes(this, bytes, next_size))
return false;
- bytes_received_this_range_ += length;
if (range.HasLength() && bytes_received_this_range_ >= range.length()) {
// Terminates the current fetcher. Waits for its TransferTerminated
// callback before starting the next range so that we don't end up
diff --git a/common/test_utils.h b/common/test_utils.h
index ffe6f67..5be48cf 100644
--- a/common/test_utils.h
+++ b/common/test_utils.h
@@ -24,18 +24,15 @@
// Streams used for gtest's PrintTo() functions.
#include <iostream> // NOLINT(readability/streams)
#include <memory>
-#include <set>
#include <string>
#include <vector>
-#include <base/callback.h>
#include <base/files/file_path.h>
#include <base/files/scoped_temp_dir.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "update_engine/common/action.h"
-#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
#include "update_engine/update_metadata.pb.h"
@@ -76,22 +73,6 @@
return system(cmd.c_str());
}
-inline int Symlink(const std::string& oldpath, const std::string& newpath) {
- return symlink(oldpath.c_str(), newpath.c_str());
-}
-
-inline int Chmod(const std::string& path, mode_t mode) {
- return chmod(path.c_str(), mode);
-}
-
-inline int Mkdir(const std::string& path, mode_t mode) {
- return mkdir(path.c_str(), mode);
-}
-
-inline int Chdir(const std::string& path) {
- return chdir(path.c_str());
-}
-
// Reads a symlink from disk. Returns empty string on failure.
std::string Readlink(const std::string& path);
@@ -144,7 +125,7 @@
ADD_FAILURE();
}
- const std::string &dev() {
+ const std::string& dev() const {
EXPECT_TRUE(is_bound_);
return dev_;
}
diff --git a/common/utils.cc b/common/utils.cc
index 1a8fd53..c609013 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -83,11 +83,6 @@
// The path to the kernel's boot_id.
const char kBootIdPath[] = "/proc/sys/kernel/random/boot_id";
-// A pointer to a null-terminated string containing the root directory where all
-// the temporary files should be created. If null, the system default is used
-// instead.
-const char* root_temp_dir = nullptr;
-
// Return true if |disk_name| is an MTD or a UBI device. Note that this test is
// simply based on the name of the device.
bool IsMtdDeviceName(const string& disk_name) {
@@ -144,15 +139,11 @@
}
base::FilePath temp_dir;
- if (root_temp_dir) {
- temp_dir = base::FilePath(root_temp_dir);
- } else {
#ifdef __ANDROID__
- temp_dir = base::FilePath(constants::kNonVolatileDirectory).Append("tmp");
+ temp_dir = base::FilePath(constants::kNonVolatileDirectory).Append("tmp");
#else
- TEST_AND_RETURN_FALSE(base::GetTempDir(&temp_dir));
+ TEST_AND_RETURN_FALSE(base::GetTempDir(&temp_dir));
#endif // __ANDROID__
- }
if (!base::PathExists(temp_dir))
TEST_AND_RETURN_FALSE(base::CreateDirectory(temp_dir));
*template_path = temp_dir.Append(path);
@@ -163,10 +154,6 @@
namespace utils {
-void SetRootTempDir(const char* new_root_temp_dir) {
- root_temp_dir = new_root_temp_dir;
-}
-
string ParseECVersion(string input_line) {
base::TrimWhitespaceASCII(input_line, base::TRIM_ALL, &input_line);
@@ -927,12 +914,6 @@
return base_code;
}
-Time TimeFromStructTimespec(struct timespec *ts) {
- int64_t us = static_cast<int64_t>(ts->tv_sec) * Time::kMicrosecondsPerSecond +
- static_cast<int64_t>(ts->tv_nsec) / Time::kNanosecondsPerMicrosecond;
- return Time::UnixEpoch() + TimeDelta::FromMicroseconds(us);
-}
-
string StringVectorToString(const vector<string> &vec_str) {
string str = "[";
for (vector<string>::const_iterator i = vec_str.begin();
@@ -962,48 +943,6 @@
encoded_hash.c_str());
}
-bool DecodeAndStoreBase64String(const string& base64_encoded,
- base::FilePath *out_path) {
- brillo::Blob contents;
-
- out_path->clear();
-
- if (base64_encoded.size() == 0) {
- LOG(ERROR) << "Can't decode empty string.";
- return false;
- }
-
- if (!brillo::data_encoding::Base64Decode(base64_encoded, &contents) ||
- contents.size() == 0) {
- LOG(ERROR) << "Error decoding base64.";
- return false;
- }
-
- FILE *file = base::CreateAndOpenTemporaryFile(out_path);
- if (file == nullptr) {
- LOG(ERROR) << "Error creating temporary file.";
- return false;
- }
-
- if (fwrite(contents.data(), 1, contents.size(), file) != contents.size()) {
- PLOG(ERROR) << "Error writing to temporary file.";
- if (fclose(file) != 0)
- PLOG(ERROR) << "Error closing temporary file.";
- if (unlink(out_path->value().c_str()) != 0)
- PLOG(ERROR) << "Error unlinking temporary file.";
- out_path->clear();
- return false;
- }
-
- if (fclose(file) != 0) {
- PLOG(ERROR) << "Error closing temporary file.";
- out_path->clear();
- return false;
- }
-
- return true;
-}
-
bool ConvertToOmahaInstallDate(Time time, int *out_num_days) {
time_t unix_time = time.ToTimeT();
// Output of: date +"%s" --date="Jan 1, 2007 0:00 PST".
diff --git a/common/utils.h b/common/utils.h
index e55a6e5..4b83cc4 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -44,11 +44,6 @@
namespace utils {
-// Converts a struct timespec representing a number of seconds since
-// the Unix epoch to a base::Time. Sub-microsecond time is rounded
-// down.
-base::Time TimeFromStructTimespec(struct timespec *ts);
-
// Formats |vec_str| as a string of the form ["<elem1>", "<elem2>"].
// Does no escaping, only use this for presentation in error messages.
std::string StringVectorToString(const std::vector<std::string> &vec_str);
@@ -132,13 +127,6 @@
// only returns true if "/dev/ubi%d_0" becomes available in |timeout| seconds.
bool TryAttachingUbiVolume(int volume_num, int timeout);
-// Setup the directory |new_root_temp_dir| to be used as the root directory for
-// temporary files instead of the system's default. If the directory doesn't
-// exists, it will be created when first used.
-// NOTE: The memory pointed by |new_root_temp_dir| must be available until this
-// function is called again with a different value.
-void SetRootTempDir(const char* new_root_temp_dir);
-
// If |base_filename_template| is neither absolute (starts with "/") nor
// explicitly relative to the current working directory (starts with "./" or
// "../"), then it is prepended the system's temporary directory. On success,
@@ -275,14 +263,6 @@
// it'll return the same value again.
ErrorCode GetBaseErrorCode(ErrorCode code);
-// Decodes the data in |base64_encoded| and stores it in a temporary
-// file. Returns false if the given data is empty, not well-formed
-// base64 or if an error occurred. If true is returned, the decoded
-// data is stored in the file returned in |out_path|. The file should
-// be deleted when no longer needed.
-bool DecodeAndStoreBase64String(const std::string& base64_encoded,
- base::FilePath *out_path);
-
// Converts |time| to an Omaha InstallDate which is defined as "the
// number of PST8PDT calendar weeks since Jan 1st 2007 0:00 PST, times
// seven" with PST8PDT defined as "Pacific Time" (e.g. UTC-07:00 if
diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index 3405b68..b30b2d2 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -287,47 +287,6 @@
"-1s");
}
-TEST(UtilsTest, TimeFromStructTimespecTest) {
- struct timespec ts;
-
- // Unix epoch (Thursday 00:00:00 UTC on Jan 1, 1970)
- ts = (struct timespec) {.tv_sec = 0, .tv_nsec = 0};
- EXPECT_EQ(base::Time::UnixEpoch(), utils::TimeFromStructTimespec(&ts));
-
- // 42 ms after the Unix billennium (Sunday 01:46:40 UTC on September 9, 2001)
- ts = (struct timespec) {.tv_sec = 1000 * 1000 * 1000,
- .tv_nsec = 42 * 1000 * 1000};
- base::Time::Exploded exploded = (base::Time::Exploded) {
- .year = 2001, .month = 9, .day_of_week = 0, .day_of_month = 9,
- .hour = 1, .minute = 46, .second = 40, .millisecond = 42};
- base::Time time;
- EXPECT_TRUE(base::Time::FromUTCExploded(exploded, &time));
- EXPECT_EQ(time, utils::TimeFromStructTimespec(&ts));
-}
-
-TEST(UtilsTest, DecodeAndStoreBase64String) {
- base::FilePath path;
-
- // Ensure we return false on empty strings or invalid base64.
- EXPECT_FALSE(utils::DecodeAndStoreBase64String("", &path));
- EXPECT_FALSE(utils::DecodeAndStoreBase64String("not valid base64", &path));
-
- // Pass known base64 and check that it matches. This string was generated
- // the following way:
- //
- // $ echo "Update Engine" | base64
- // VXBkYXRlIEVuZ2luZQo=
- EXPECT_TRUE(utils::DecodeAndStoreBase64String("VXBkYXRlIEVuZ2luZQo=",
- &path));
- ScopedPathUnlinker unlinker(path.value());
- string expected_contents = "Update Engine\n";
- string contents;
- EXPECT_TRUE(utils::ReadFile(path.value(), &contents));
- EXPECT_EQ(contents, expected_contents);
- EXPECT_EQ(static_cast<off_t>(expected_contents.size()),
- utils::FileSize(path.value()));
-}
-
TEST(UtilsTest, ConvertToOmahaInstallDate) {
// The Omaha Epoch starts at Jan 1, 2007 0:00 PST which is a
// Monday. In Unix time, this point in time is easily obtained via
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index 02fd17b..736a87a 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -43,6 +43,7 @@
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/prefs.h"
+#include "update_engine/common/subprocess.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/fake_p2p_manager_configuration.h"
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 7dcb5f7..2834e61 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -261,7 +261,7 @@
// Update chunk index, log as needed: if forced by called, or we completed a
// progress chunk, or a timeout has expired.
- base::Time curr_time = base::Time::Now();
+ base::TimeTicks curr_time = base::TimeTicks::Now();
unsigned curr_progress_chunk =
overall_progress_ * kProgressLogMaxChunks / 100;
if (force_log || curr_progress_chunk > last_progress_chunk_ ||
@@ -525,19 +525,17 @@
<< "Trusting metadata size in payload = " << metadata_size_;
}
- // See if we should use the public RSA key in the Omaha response.
- base::FilePath path_to_public_key(public_key_path_);
- base::FilePath tmp_key;
- if (GetPublicKeyFromResponse(&tmp_key))
- path_to_public_key = tmp_key;
- ScopedPathUnlinker tmp_key_remover(tmp_key.value());
- if (tmp_key.empty())
- tmp_key_remover.set_should_remove(false);
+ string public_key;
+ if (!GetPublicKey(&public_key)) {
+ LOG(ERROR) << "Failed to get public key.";
+ *error = ErrorCode::kDownloadMetadataSignatureVerificationError;
+ return MetadataParseResult::kError;
+ }
// We have the full metadata in |payload|. Verify its integrity
// and authenticity based on the information we have in Omaha response.
*error = payload_metadata_.ValidateMetadataSignature(
- payload, payload_->metadata_signature, path_to_public_key);
+ payload, payload_->metadata_signature, public_key);
if (*error != ErrorCode::kSuccess) {
if (install_plan_->hash_checks_mandatory) {
// The autoupdate_CatchBadSignatures test checks for this string
@@ -760,7 +758,7 @@
next_operation_num_++;
UpdateOverallProgress(false, "Completed ");
- CheckpointUpdateProgress();
+ CheckpointUpdateProgress(false);
}
// In major version 2, we don't add dummy operation to the payload.
@@ -789,7 +787,9 @@
// Since we extracted the SignatureMessage we need to advance the
// checkpoint, otherwise we would reload the signature and try to extract
// it again.
- CheckpointUpdateProgress();
+ // This is the last checkpoint for an update, force this checkpoint to be
+ // saved.
+ CheckpointUpdateProgress(true);
}
return true;
@@ -1596,15 +1596,21 @@
return true;
}
-bool DeltaPerformer::GetPublicKeyFromResponse(base::FilePath *out_tmp_key) {
- if (hardware_->IsOfficialBuild() ||
- utils::FileExists(public_key_path_.c_str()) ||
- install_plan_->public_key_rsa.empty())
- return false;
+bool DeltaPerformer::GetPublicKey(string* out_public_key) {
+ out_public_key->clear();
- if (!utils::DecodeAndStoreBase64String(install_plan_->public_key_rsa,
- out_tmp_key))
- return false;
+ if (utils::FileExists(public_key_path_.c_str())) {
+ LOG(INFO) << "Verifying using public key: " << public_key_path_;
+ return utils::ReadFile(public_key_path_, out_public_key);
+ }
+
+ // If this is an official build then we are not allowed to use public key from
+ // Omaha response.
+ if (!hardware_->IsOfficialBuild() && !install_plan_->public_key_rsa.empty()) {
+ LOG(INFO) << "Verifying using public key from Omaha response.";
+ return brillo::data_encoding::Base64Decode(install_plan_->public_key_rsa,
+ out_public_key);
+ }
return true;
}
@@ -1771,24 +1777,21 @@
ErrorCode DeltaPerformer::VerifyPayload(
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.
- base::FilePath path_to_public_key(public_key_path_);
- base::FilePath tmp_key;
- if (GetPublicKeyFromResponse(&tmp_key))
- path_to_public_key = tmp_key;
- ScopedPathUnlinker tmp_key_remover(tmp_key.value());
- if (tmp_key.empty())
- tmp_key_remover.set_should_remove(false);
-
- LOG(INFO) << "Verifying payload using public key: "
- << path_to_public_key.value();
+ string public_key;
+ if (!GetPublicKey(&public_key)) {
+ LOG(ERROR) << "Failed to get public key.";
+ return ErrorCode::kDownloadPayloadPubKeyVerificationError;
+ }
// Verifies the download size.
- TEST_AND_RETURN_VAL(ErrorCode::kPayloadSizeMismatchError,
- update_check_response_size ==
- metadata_size_ + metadata_signature_size_ +
- buffer_offset_);
+ if (update_check_response_size !=
+ metadata_size_ + metadata_signature_size_ + buffer_offset_) {
+ LOG(ERROR) << "update_check_response_size (" << update_check_response_size
+ << ") doesn't match metadata_size (" << metadata_size_
+ << ") + metadata_signature_size (" << metadata_signature_size_
+ << ") + buffer_offset (" << buffer_offset_ << ").";
+ return ErrorCode::kPayloadSizeMismatchError;
+ }
// Verifies the payload hash.
TEST_AND_RETURN_VAL(ErrorCode::kDownloadPayloadVerificationError,
@@ -1798,7 +1801,7 @@
payload_hash_calculator_.raw_hash() == update_check_response_hash);
// Verifies the signed payload hash.
- if (!utils::FileExists(path_to_public_key.value().c_str())) {
+ if (public_key.empty()) {
LOG(WARNING) << "Not verifying signed delta payload -- missing public key.";
return ErrorCode::kSuccess;
}
@@ -1811,7 +1814,7 @@
!hash_data.empty());
if (!PayloadVerifier::VerifySignature(
- signatures_message_data_, path_to_public_key.value(), hash_data)) {
+ signatures_message_data_, public_key, hash_data)) {
// The autoupdate_CatchBadSignatures test checks for this string
// in log-files. Keep in sync.
LOG(ERROR) << "Public key verification failed, thus update failed.";
@@ -1901,9 +1904,9 @@
return true;
}
-bool DeltaPerformer::CheckpointUpdateProgress() {
- base::Time curr_time = base::Time::Now();
- if (curr_time > update_checkpoint_time_) {
+bool DeltaPerformer::CheckpointUpdateProgress(bool force) {
+ base::TimeTicks curr_time = base::TimeTicks::Now();
+ if (force || curr_time > update_checkpoint_time_) {
update_checkpoint_time_ = curr_time + update_checkpoint_wait_;
} else {
return false;
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 8597a37..402e4be 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -257,20 +257,20 @@
// Checkpoints the update progress into persistent storage to allow this
// update attempt to be resumed after reboot.
- bool CheckpointUpdateProgress();
+ // If |force| is false, checkpoint may be throttled.
+ bool CheckpointUpdateProgress(bool force);
// Primes the required update state. Returns true if the update state was
// successfully initialized to a saved resume state or if the update is a new
// update. Returns false otherwise.
bool PrimeUpdateState();
- // If the Omaha response contains a public RSA key and we're allowed
- // to use it (e.g. if we're in developer mode), extract the key from
- // the response and store it in a temporary file and return true. In
- // the affirmative the path to the temporary file is stored in
- // |out_tmp_key| and it is the responsibility of the caller to clean
- // it up.
- bool GetPublicKeyFromResponse(base::FilePath *out_tmp_key);
+ // Get the public key to be used to verify metadata signature or payload
+ // signature. Always use |public_key_path_| if exists, otherwise if the Omaha
+ // response contains a public RSA key and we're allowed to use it (e.g. if
+ // we're in developer mode), decode the key from the response and store it in
+ // |out_public_key|. Returns false on failures.
+ bool GetPublicKey(std::string* out_public_key);
// After install_plan_ is filled with partition names and sizes, initialize
// metadata of partitions and map necessary devices before opening devices.
@@ -399,13 +399,13 @@
// and the actual point in time for the next forced log to be emitted.
const base::TimeDelta forced_progress_log_wait_{
base::TimeDelta::FromSeconds(kProgressLogTimeoutSeconds)};
- base::Time forced_progress_log_time_;
+ base::TimeTicks forced_progress_log_time_;
// The frequency that we should write an update checkpoint (constant), and
// the point in time at which the next checkpoint should be written.
const base::TimeDelta update_checkpoint_wait_{
base::TimeDelta::FromSeconds(kCheckpointFrequencySeconds)};
- base::Time update_checkpoint_time_;
+ base::TimeTicks update_checkpoint_time_;
DISALLOW_COPY_AND_ASSIGN(DeltaPerformer);
};
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 3cddee4..4cf9756 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -942,8 +942,6 @@
}
TEST_F(DeltaPerformerTest, UsePublicKeyFromResponse) {
- base::FilePath key_path;
-
// The result of the GetPublicKeyResponse() method is based on three things
//
// 1. Whether it's an official build; and
@@ -960,55 +958,65 @@
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
string non_existing_file = temp_dir.GetPath().Append("non-existing").value();
string existing_file = temp_dir.GetPath().Append("existing").value();
- EXPECT_EQ(0, System(base::StringPrintf("touch %s", existing_file.c_str())));
+ constexpr char kExistingKey[] = "Existing";
+ ASSERT_TRUE(test_utils::WriteFileString(existing_file, kExistingKey));
- // Non-official build, non-existing public-key, key in response -> true
+ // Non-official build, non-existing public-key, key in response ->
+ // kResponseKey
fake_hardware_.SetIsOfficialBuild(false);
performer_.public_key_path_ = non_existing_file;
- // This is the result of 'echo "Test" | base64' and is not meant to be a
- // valid public key, but it is valid base-64.
- constexpr char kBase64TestKey[] = "VGVzdAo=";
- install_plan_.public_key_rsa = kBase64TestKey;
- EXPECT_TRUE(performer_.GetPublicKeyFromResponse(&key_path));
- EXPECT_FALSE(key_path.empty());
- EXPECT_EQ(unlink(key_path.value().c_str()), 0);
- // Same with official build -> false
+ // This is the result of 'echo -n "Response" | base64' and is not meant to be
+ // a valid public key, but it is valid base-64.
+ constexpr char kResponseKey[] = "Response";
+ constexpr char kBase64ResponseKey[] = "UmVzcG9uc2U=";
+ install_plan_.public_key_rsa = kBase64ResponseKey;
+ string public_key;
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_EQ(public_key, kResponseKey);
+ // Same with official build -> no key
fake_hardware_.SetIsOfficialBuild(true);
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_TRUE(public_key.empty());
- // Non-official build, existing public-key, key in response -> false
+ // Non-official build, existing public-key, key in response -> kExistingKey
fake_hardware_.SetIsOfficialBuild(false);
performer_.public_key_path_ = existing_file;
- install_plan_.public_key_rsa = kBase64TestKey;
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
- // Same with official build -> false
+ install_plan_.public_key_rsa = kBase64ResponseKey;
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_EQ(public_key, kExistingKey);
+ // Same with official build -> kExistingKey
fake_hardware_.SetIsOfficialBuild(true);
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_EQ(public_key, kExistingKey);
- // Non-official build, non-existing public-key, no key in response -> false
+ // Non-official build, non-existing public-key, no key in response -> no key
fake_hardware_.SetIsOfficialBuild(false);
performer_.public_key_path_ = non_existing_file;
install_plan_.public_key_rsa = "";
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
- // Same with official build -> false
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_TRUE(public_key.empty());
+ // Same with official build -> no key
fake_hardware_.SetIsOfficialBuild(true);
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_TRUE(public_key.empty());
- // Non-official build, existing public-key, no key in response -> false
+ // Non-official build, existing public-key, no key in response -> kExistingKey
fake_hardware_.SetIsOfficialBuild(false);
performer_.public_key_path_ = existing_file;
install_plan_.public_key_rsa = "";
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
- // Same with official build -> false
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_EQ(public_key, kExistingKey);
+ // Same with official build -> kExistingKey
fake_hardware_.SetIsOfficialBuild(true);
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
+ EXPECT_TRUE(performer_.GetPublicKey(&public_key));
+ EXPECT_EQ(public_key, kExistingKey);
// Non-official build, non-existing public-key, key in response
// but invalid base64 -> false
fake_hardware_.SetIsOfficialBuild(false);
performer_.public_key_path_ = non_existing_file;
install_plan_.public_key_rsa = "not-valid-base64";
- EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
+ EXPECT_FALSE(performer_.GetPublicKey(&public_key));
}
TEST_F(DeltaPerformerTest, ConfVersionsMatch) {
diff --git a/payload_consumer/payload_metadata.cc b/payload_consumer/payload_metadata.cc
index 3079feb..b631c87 100644
--- a/payload_consumer/payload_metadata.cc
+++ b/payload_consumer/payload_metadata.cc
@@ -155,8 +155,8 @@
ErrorCode PayloadMetadata::ValidateMetadataSignature(
const brillo::Blob& payload,
- std::string metadata_signature,
- base::FilePath path_to_public_key) const {
+ const std::string& metadata_signature,
+ const std::string& pem_public_key) const {
if (payload.size() < metadata_size_ + metadata_signature_size_)
return ErrorCode::kDownloadMetadataSignatureError;
@@ -182,9 +182,6 @@
return ErrorCode::kDownloadMetadataSignatureMissingError;
}
- LOG(INFO) << "Verifying metadata hash signature using public key: "
- << path_to_public_key.value();
-
brillo::Blob calculated_metadata_hash;
if (!HashCalculator::RawHashOfBytes(
payload.data(), metadata_size_, &calculated_metadata_hash)) {
@@ -200,9 +197,8 @@
if (!metadata_signature_blob.empty()) {
brillo::Blob expected_metadata_hash;
- if (!PayloadVerifier::GetRawHashFromSignature(metadata_signature_blob,
- path_to_public_key.value(),
- &expected_metadata_hash)) {
+ if (!PayloadVerifier::GetRawHashFromSignature(
+ metadata_signature_blob, pem_public_key, &expected_metadata_hash)) {
LOG(ERROR) << "Unable to compute expected hash from metadata signature";
return ErrorCode::kDownloadMetadataSignatureError;
}
@@ -215,7 +211,7 @@
}
} else {
if (!PayloadVerifier::VerifySignature(metadata_signature_protobuf_blob,
- path_to_public_key.value(),
+ pem_public_key,
calculated_metadata_hash)) {
LOG(ERROR) << "Manifest hash verification failed.";
return ErrorCode::kDownloadMetadataSignatureMismatch;
diff --git a/payload_consumer/payload_metadata.h b/payload_consumer/payload_metadata.h
index 8748f6f..1b4c5c8 100644
--- a/payload_consumer/payload_metadata.h
+++ b/payload_consumer/payload_metadata.h
@@ -22,7 +22,7 @@
#include <string>
#include <vector>
-#include <base/files/file_path.h>
+#include <base/macros.h>
#include <brillo/secure_blob.h>
#include "update_engine/common/error_code.h"
@@ -66,8 +66,8 @@
// to the payload server doesn't exploit any vulnerability in the code that
// parses the protocol buffer.
ErrorCode ValidateMetadataSignature(const brillo::Blob& payload,
- std::string metadata_signature,
- base::FilePath path_to_public_key) const;
+ const std::string& metadata_signature,
+ const std::string& pem_public_key) const;
// Returns the major payload version. If the version was not yet parsed,
// returns zero.
diff --git a/payload_consumer/payload_verifier.cc b/payload_consumer/payload_verifier.cc
index ab5238c..f3d4626 100644
--- a/payload_consumer/payload_verifier.cc
+++ b/payload_consumer/payload_verifier.cc
@@ -16,6 +16,8 @@
#include "update_engine/payload_consumer/payload_verifier.h"
+#include <vector>
+
#include <base/logging.h>
#include <openssl/pem.h>
@@ -85,10 +87,8 @@
} // namespace
bool PayloadVerifier::VerifySignature(const brillo::Blob& signature_blob,
- const string& public_key_path,
+ const string& pem_public_key,
const brillo::Blob& hash_data) {
- TEST_AND_RETURN_FALSE(!public_key_path.empty());
-
Signatures signatures;
LOG(INFO) << "signature blob size = " << signature_blob.size();
TEST_AND_RETURN_FALSE(signatures.ParseFromArray(signature_blob.data(),
@@ -105,7 +105,7 @@
const Signatures_Signature& signature = signatures.signatures(i);
brillo::Blob sig_data(signature.data().begin(), signature.data().end());
brillo::Blob sig_hash_data;
- if (!GetRawHashFromSignature(sig_data, public_key_path, &sig_hash_data))
+ if (!GetRawHashFromSignature(sig_data, pem_public_key, &sig_hash_data))
continue;
if (hash_data == sig_hash_data) {
@@ -125,28 +125,19 @@
return false;
}
-
-bool PayloadVerifier::GetRawHashFromSignature(
- const brillo::Blob& sig_data,
- const string& public_key_path,
- brillo::Blob* out_hash_data) {
- TEST_AND_RETURN_FALSE(!public_key_path.empty());
-
+bool PayloadVerifier::GetRawHashFromSignature(const brillo::Blob& sig_data,
+ const string& pem_public_key,
+ brillo::Blob* out_hash_data) {
// The code below executes the equivalent of:
//
- // openssl rsautl -verify -pubin -inkey |public_key_path|
+ // openssl rsautl -verify -pubin -inkey <(echo |pem_public_key|)
// -in |sig_data| -out |out_hash_data|
- // Loads the public key.
- FILE* fpubkey = fopen(public_key_path.c_str(), "rb");
- if (!fpubkey) {
- LOG(ERROR) << "Unable to open public key file: " << public_key_path;
- return false;
- }
+ BIO* bp = BIO_new_mem_buf(pem_public_key.data(), pem_public_key.size());
+ char dummy_password[] = {' ', 0}; // Ensure no password is read from stdin.
+ RSA* rsa = PEM_read_bio_RSA_PUBKEY(bp, nullptr, nullptr, dummy_password);
+ BIO_free(bp);
- char dummy_password[] = { ' ', 0 }; // Ensure no password is read from stdin.
- RSA* rsa = PEM_read_RSA_PUBKEY(fpubkey, nullptr, nullptr, dummy_password);
- fclose(fpubkey);
TEST_AND_RETURN_FALSE(rsa != nullptr);
unsigned int keysize = RSA_size(rsa);
if (sig_data.size() > 2 * keysize) {
diff --git a/payload_consumer/payload_verifier.h b/payload_consumer/payload_verifier.h
index 8caef35..ec23ef2 100644
--- a/payload_consumer/payload_verifier.h
+++ b/payload_consumer/payload_verifier.h
@@ -32,19 +32,20 @@
class PayloadVerifier {
public:
// Interprets |signature_blob| as a protocol buffer containing the Signatures
- // message and decrypts each signature data using the |public_key_path|.
+ // message and decrypts each signature data using the |pem_public_key|.
+ // |pem_public_key| should be a PEM format RSA public key data.
// Returns whether *any* of the decrypted hashes matches the |hash_data|.
// In case of any error parsing the signatures or the public key, returns
// false.
static bool VerifySignature(const brillo::Blob& signature_blob,
- const std::string& public_key_path,
+ const std::string& pem_public_key,
const brillo::Blob& hash_data);
- // Decrypts sig_data with the given public_key_path and populates
- // out_hash_data with the decoded raw hash. Returns true if successful,
- // false otherwise.
+ // Decrypts |sig_data| with the given |pem_public_key| and populates
+ // |out_hash_data| with the decoded raw hash. |pem_public_key| should be a PEM
+ // format RSA public key data. Returns true if successful, false otherwise.
static bool GetRawHashFromSignature(const brillo::Blob& sig_data,
- const std::string& public_key_path,
+ const std::string& pem_public_key,
brillo::Blob* out_hash_data);
// Pads a SHA256 hash so that it may be encrypted/signed with RSA2048
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index 8381472..8d461d5 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -37,6 +37,7 @@
#include "update_engine/common/constants.h"
#include "update_engine/common/fake_boot_control.h"
#include "update_engine/common/fake_hardware.h"
+#include "update_engine/common/subprocess.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/mock_payload_state.h"
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index f93fb55..dc38c33 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -297,7 +297,7 @@
void FileDeltaProcessor::Run() {
TEST_AND_RETURN(blob_file_ != nullptr);
- base::Time start = base::Time::Now();
+ base::TimeTicks start = base::TimeTicks::Now();
if (!DeltaReadFile(&file_aops_,
old_part_,
@@ -326,8 +326,7 @@
}
LOG(INFO) << "Encoded file " << name_ << " (" << new_extents_blocks_
- << " blocks) in " << (base::Time::Now() - start).InSecondsF()
- << " seconds.";
+ << " blocks) in " << (base::TimeTicks::Now() - start);
}
bool FileDeltaProcessor::MergeOperation(vector<AnnotatedOperation>* aops) {
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 2c386fa..35a0c3f 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -256,17 +256,19 @@
&metadata_hash));
brillo::Blob signature_blob(payload.begin() + signatures_offset,
payload.end());
+ string public_key;
+ TEST_AND_RETURN_FALSE(utils::ReadFile(public_key_path, &public_key));
TEST_AND_RETURN_FALSE(PayloadVerifier::PadRSA2048SHA256Hash(&payload_hash));
TEST_AND_RETURN_FALSE(PayloadVerifier::VerifySignature(
- signature_blob, public_key_path, payload_hash));
+ signature_blob, public_key, payload_hash));
if (metadata_signature_size) {
- signature_blob.assign(payload.begin() + metadata_size,
- payload.begin() + metadata_size +
- metadata_signature_size);
+ signature_blob.assign(
+ payload.begin() + metadata_size,
+ payload.begin() + metadata_size + metadata_signature_size);
TEST_AND_RETURN_FALSE(
PayloadVerifier::PadRSA2048SHA256Hash(&metadata_hash));
TEST_AND_RETURN_FALSE(PayloadVerifier::VerifySignature(
- signature_blob, public_key_path, metadata_hash));
+ signature_blob, public_key, metadata_hash));
}
return true;
}
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 967e026..a6ef38d 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -153,14 +153,15 @@
GetBuildArtifactsPath(kUnittestPrivateKey2Path)});
// Either public key should pass the verification.
+ string public_key;
+ EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKeyPath),
+ &public_key));
EXPECT_TRUE(PayloadVerifier::VerifySignature(
- signature_blob,
- GetBuildArtifactsPath(kUnittestPublicKeyPath),
- padded_hash_data_));
+ signature_blob, public_key, padded_hash_data_));
+ EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKey2Path),
+ &public_key));
EXPECT_TRUE(PayloadVerifier::VerifySignature(
- signature_blob,
- GetBuildArtifactsPath(kUnittestPublicKey2Path),
- padded_hash_data_));
+ signature_blob, public_key, padded_hash_data_));
}
TEST_F(PayloadSignerTest, VerifySignatureTest) {
@@ -168,15 +169,16 @@
SignSampleData(&signature_blob,
{GetBuildArtifactsPath(kUnittestPrivateKeyPath)});
+ string public_key;
+ EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKeyPath),
+ &public_key));
EXPECT_TRUE(PayloadVerifier::VerifySignature(
- signature_blob,
- GetBuildArtifactsPath(kUnittestPublicKeyPath),
- padded_hash_data_));
+ signature_blob, public_key, padded_hash_data_));
// Passing the invalid key should fail the verification.
- EXPECT_FALSE(PayloadVerifier::VerifySignature(
- signature_blob,
- GetBuildArtifactsPath(kUnittestPublicKey2Path),
- padded_hash_data_));
+ EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKey2Path),
+ &public_key));
+ EXPECT_TRUE(PayloadVerifier::VerifySignature(
+ signature_blob, public_key, padded_hash_data_));
}
TEST_F(PayloadSignerTest, SkipMetadataSignatureTest) {
diff --git a/sideload_main.cc b/sideload_main.cc
index ddb312e..4bbb4b8 100644
--- a/sideload_main.cc
+++ b/sideload_main.cc
@@ -43,11 +43,6 @@
using update_engine::UpdateStatus;
using update_engine::UpdateEngineStatus;
-namespace {
-// The root directory used for temporary files in update_engine_sideload.
-const char kSideloadRootTempDir[] = "/tmp/update_engine_sideload";
-} // namespace
-
namespace chromeos_update_engine {
namespace {
@@ -208,10 +203,6 @@
// xz-embedded requires to initialize its CRC-32 table once on startup.
xz_crc32_init();
- // When called from recovery, /data is not accessible, so we need to use
- // /tmp for temporary files.
- chromeos_update_engine::utils::SetRootTempDir(kSideloadRootTempDir);
-
vector<string> headers = base::SplitString(
FLAGS_headers, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
diff --git a/update_attempter.cc b/update_attempter.cc
index 5efd257..5f32312 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -225,7 +225,7 @@
return;
}
- Time lsb_release_timestamp = utils::TimeFromStructTimespec(&sb.st_ctim);
+ Time lsb_release_timestamp = Time::FromTimeSpec(sb.st_ctim);
Time now = system_state_->clock()->GetWallclockTime();
TimeDelta age = now - lsb_release_timestamp;
if (age.InSeconds() < 0) {
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index a9033b7..a3974ab 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -396,8 +396,13 @@
"Failed to read metadata and signature from " + metadata_filename);
}
fd->Close();
- errorcode = payload_metadata.ValidateMetadataSignature(
- metadata, "", base::FilePath(constants::kUpdatePayloadPublicKeyPath));
+
+ string public_key;
+ if (!utils::ReadFile(constants::kUpdatePayloadPublicKeyPath, &public_key)) {
+ return LogAndSetError(error, FROM_HERE, "Failed to read public key.");
+ }
+ errorcode =
+ payload_metadata.ValidateMetadataSignature(metadata, "", public_key);
if (errorcode != ErrorCode::kSuccess) {
return LogAndSetError(error,
FROM_HERE,