AU: Verify source rootfs/kernel hashes before applying delta.
New style full updates will not send the old rootfs hash so no check takes
place.
BUG=7562
TEST=unit tests, gmerged on device and tested with good/bad source partition
Change-Id: I65b28bf57110e4d87472d4aea59121878cde24b0
Review URL: http://codereview.chromium.org/3712003
diff --git a/action_processor.h b/action_processor.h
index fc42a24..9bc4ef2 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -37,6 +37,7 @@
kActionCodeDownloadSizeMismatchError = 11,
kActionCodeDownloadPayloadVerificationError = 12,
kActionCodeDownloadAppliedUpdateVerificationError = 13,
+ kActionCodeDownloadWriteError = 14,
};
class AbstractAction;
diff --git a/delta_diff_generator.cc b/delta_diff_generator.cc
index 9659ad2..f024481 100644
--- a/delta_diff_generator.cc
+++ b/delta_diff_generator.cc
@@ -611,6 +611,7 @@
TEST_AND_RETURN_FALSE(hasher.Finalize());
const vector<char>& hash = hasher.raw_hash();
info->set_hash(hash.data(), hash.size());
+ LOG(INFO) << "hash: " << hasher.hash();
return true;
}
@@ -619,12 +620,8 @@
const string& old_rootfs,
const string& new_rootfs,
DeltaArchiveManifest* manifest) {
- if (!old_kernel.empty()) {
- TEST_AND_RETURN_FALSE(
- InitializePartitionInfo(true,
- old_kernel,
- manifest->mutable_old_kernel_info()));
- }
+ // TODO(petkov): Generate the old kernel info when we stop generating full
+ // updates for the kernel partition.
TEST_AND_RETURN_FALSE(
InitializePartitionInfo(true,
new_kernel,
diff --git a/delta_performer.cc b/delta_performer.cc
index d111015..5bb5d0a 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -578,6 +578,28 @@
return true;
}
+bool DeltaPerformer::VerifySourcePartitions() {
+ LOG(INFO) << "Verifying source partitions.";
+ CHECK(manifest_valid_);
+ if (manifest_.has_old_kernel_info()) {
+ const PartitionInfo& info = manifest_.old_kernel_info();
+ TEST_AND_RETURN_FALSE(current_kernel_hash_ != NULL &&
+ current_kernel_hash_->size() == info.hash().size() &&
+ memcmp(current_kernel_hash_->data(),
+ info.hash().data(),
+ current_kernel_hash_->size()) == 0);
+ }
+ if (manifest_.has_old_rootfs_info()) {
+ const PartitionInfo& info = manifest_.old_rootfs_info();
+ TEST_AND_RETURN_FALSE(current_rootfs_hash_ != NULL &&
+ current_rootfs_hash_->size() == info.hash().size() &&
+ memcmp(current_rootfs_hash_->data(),
+ info.hash().data(),
+ current_rootfs_hash_->size()) == 0);
+ }
+ return true;
+}
+
void DeltaPerformer::DiscardBufferHeadBytes(size_t count) {
hash_calculator_.Update(&buffer_[0], count);
buffer_.erase(buffer_.begin(), buffer_.begin() + count);
@@ -661,6 +683,7 @@
next_operation == kUpdateStateOperationInvalid ||
next_operation <= 0) {
// Initiating a new update, no more state needs to be initialized.
+ TEST_AND_RETURN_FALSE(VerifySourcePartitions());
return true;
}
next_operation_num_ = next_operation;
diff --git a/delta_performer.h b/delta_performer.h
index 7394f94..0e3cb93 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -34,7 +34,9 @@
next_operation_num_(0),
buffer_offset_(0),
last_updated_buffer_offset_(kuint64max),
- block_size_(0) {}
+ block_size_(0),
+ current_kernel_hash_(NULL),
+ current_rootfs_hash_(NULL) {}
// Opens the kernel. Should be called before or after Open(), but before
// Write(). The kernel file will be close()d when Close() is called.
@@ -94,6 +96,14 @@
// success, false otherwise.
static bool ResetUpdateProgress(PrefsInterface* prefs, bool quick);
+ void set_current_kernel_hash(const std::vector<char>* hash) {
+ current_kernel_hash_ = hash;
+ }
+
+ void set_current_rootfs_hash(const std::vector<char>* hash) {
+ current_rootfs_hash_ = hash;
+ }
+
private:
friend class DeltaPerformerTest;
FRIEND_TEST(DeltaPerformerTest, IsIdempotentOperationTest);
@@ -101,6 +111,12 @@
static bool IsIdempotentOperation(
const DeltaArchiveManifest_InstallOperation& op);
+ // Verifies that the expected source partition hashes (if present) match the
+ // hashes for the current partitions. Returns true if there're no expected
+ // hashes in the payload (e.g., if it's a new-style full update) or if the
+ // hashes match; returns false otherwise.
+ bool VerifySourcePartitions();
+
// Returns true if enough of the delta file has been passed via Write()
// to be able to perform a given install operation.
bool CanPerformInstallOperation(
@@ -181,6 +197,11 @@
// Signatures message blob extracted directly from the payload.
std::vector<char> signatures_message_data_;
+ // Hashes for the current partitions to be used for source partition
+ // verification.
+ const std::vector<char>* current_kernel_hash_;
+ const std::vector<char>* current_rootfs_hash_;
+
DISALLOW_COPY_AND_ASSIGN(DeltaPerformer);
};
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index e0db6e7..4f6661e 100755
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -258,12 +258,14 @@
EXPECT_EQ(expected_sig_data_length, manifest.signatures_size());
EXPECT_FALSE(signature.data().empty());
- EXPECT_EQ(old_kernel_data.size(), manifest.old_kernel_info().size());
+ // TODO(petkov): Add a test once the generator start sending old kernel
+ // info.
+ EXPECT_FALSE(manifest.has_old_kernel_info());
+
EXPECT_EQ(new_kernel_data.size(), manifest.new_kernel_info().size());
EXPECT_EQ(image_size, manifest.old_rootfs_info().size());
EXPECT_EQ(image_size, manifest.new_rootfs_info().size());
- EXPECT_FALSE(manifest.old_kernel_info().hash().empty());
EXPECT_FALSE(manifest.new_kernel_info().hash().empty());
EXPECT_FALSE(manifest.old_rootfs_info().hash().empty());
EXPECT_FALSE(manifest.new_rootfs_info().hash().empty());
@@ -289,6 +291,11 @@
EXPECT_EQ(0, performer.Open(a_img.c_str(), 0, 0));
EXPECT_TRUE(performer.OpenKernel(old_kernel.c_str()));
+ vector<char> rootfs_hash;
+ CHECK_EQ(image_size,
+ OmahaHashCalculator::RawHashOfFile(a_img, image_size, &rootfs_hash));
+ performer.set_current_rootfs_hash(&rootfs_hash);
+
// Write at some number of bytes per operation. Arbitrarily chose 5.
const size_t kBytesPerWrite = 5;
for (size_t i = 0; i < delta.size(); i += kBytesPerWrite) {
diff --git a/download_action.cc b/download_action.cc
index 2920d16..b6f7f81 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -64,6 +64,10 @@
writer_ = decompressing_file_writer_.get();
} else {
delta_performer_.reset(new DeltaPerformer(prefs_));
+ delta_performer_->set_current_kernel_hash(
+ &install_plan_.current_kernel_hash);
+ delta_performer_->set_current_rootfs_hash(
+ &install_plan_.current_rootfs_hash);
writer_ = delta_performer_.get();
}
}
@@ -93,9 +97,10 @@
}
void DownloadAction::TerminateProcessing() {
- CHECK(writer_);
- CHECK_EQ(writer_->Close(), 0);
- writer_ = NULL;
+ if (writer_) {
+ LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
+ writer_ = NULL;
+ }
http_fetcher_->TerminateTransfer();
if (delegate_) {
delegate_->SetDownloadStatus(false); // Set to inactive.
@@ -108,8 +113,12 @@
bytes_received_ += length;
if (delegate_)
delegate_->BytesReceived(bytes_received_, install_plan_.size);
- int rc = writer_->Write(bytes, length);
- TEST_AND_RETURN(rc >= 0);
+ if (writer_ && writer_->Write(bytes, length) < 0) {
+ LOG(ERROR) << "Write error -- terminating processing.";
+ TerminateProcessing();
+ processor_->ActionComplete(this, kActionCodeDownloadWriteError);
+ return;
+ }
omaha_hash_calculator_.Update(bytes, length);
}
@@ -131,7 +140,7 @@
void DownloadAction::TransferComplete(HttpFetcher *fetcher, bool successful) {
if (writer_) {
- CHECK_EQ(writer_->Close(), 0) << errno;
+ LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
writer_ = NULL;
}
if (delegate_) {
diff --git a/filesystem_copier_action.cc b/filesystem_copier_action.cc
index 5628ec3..1452b4d 100755
--- a/filesystem_copier_action.cc
+++ b/filesystem_copier_action.cc
@@ -79,6 +79,8 @@
return;
}
+ DetermineFilesystemSize(src_fd);
+
src_stream_ = g_unix_input_stream_new(src_fd, TRUE);
dst_stream_ = g_unix_output_stream_new(dst_fd, TRUE);
@@ -89,7 +91,7 @@
g_input_stream_read_async(src_stream_,
&buffer_[0],
- buffer_.size(),
+ GetBytesToRead(),
G_PRIORITY_DEFAULT,
canceller_,
&FilesystemCopierAction::StaticAsyncReadyCallback,
@@ -137,9 +139,27 @@
if (bytes_read == 0) {
// We're done!
+ if (!hasher_.Finalize()) {
+ LOG(ERROR) << "Unable to finalize the hash.";
+ Cleanup(false, was_cancelled);
+ return;
+ }
+ LOG(INFO) << "hash: " << hasher_.hash();
+ if (copying_kernel_install_path_) {
+ install_plan_.current_kernel_hash = hasher_.raw_hash();
+ } else {
+ install_plan_.current_rootfs_hash = hasher_.raw_hash();
+ }
Cleanup(true, was_cancelled);
return;
}
+ if (!hasher_.Update(buffer_.data(), bytes_read)) {
+ LOG(ERROR) << "Unable to update the hash.";
+ Cleanup(false, was_cancelled);
+ return;
+ }
+ filesystem_size_ -= bytes_read;
+
// Kick off a write
read_in_flight_ = false;
buffer_valid_size_ = bytes_read;
@@ -175,11 +195,26 @@
g_input_stream_read_async(
src_stream_,
&buffer_[0],
- buffer_.size(),
+ GetBytesToRead(),
G_PRIORITY_DEFAULT,
canceller_,
&FilesystemCopierAction::StaticAsyncReadyCallback,
this);
}
+void FilesystemCopierAction::DetermineFilesystemSize(int fd) {
+ filesystem_size_ = kint64max;
+ int block_count = 0, block_size = 0;
+ if (!copying_kernel_install_path_ &&
+ utils::GetFilesystemSizeFromFD(fd, &block_count, &block_size)) {
+ filesystem_size_ = static_cast<int64_t>(block_count) * block_size;
+ LOG(INFO) << "Filesystem size: " << filesystem_size_ << " bytes ("
+ << block_count << "x" << block_size << ").";
+ }
+}
+
+int64_t FilesystemCopierAction::GetBytesToRead() {
+ return std::min(static_cast<int64_t>(buffer_.size()), filesystem_size_);
+}
+
} // namespace chromeos_update_engine
diff --git a/filesystem_copier_action.h b/filesystem_copier_action.h
index 786c2ab..6720a47 100644
--- a/filesystem_copier_action.h
+++ b/filesystem_copier_action.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -7,12 +7,17 @@
#include <sys/stat.h>
#include <sys/types.h>
+
#include <string>
#include <vector>
+
#include <gio/gio.h>
#include <glib.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
+
#include "update_engine/action.h"
#include "update_engine/install_plan.h"
+#include "update_engine/omaha_hash_calculator.h"
// This action will only do real work if it's a delta update. It will
// copy the root partition to install partition, and then terminate.
@@ -38,7 +43,8 @@
dst_stream_(NULL),
canceller_(NULL),
read_in_flight_(false),
- buffer_valid_size_(0) {}
+ buffer_valid_size_(0),
+ filesystem_size_(kint64max) {}
typedef ActionTraits<FilesystemCopierAction>::InputObjectType
InputObjectType;
typedef ActionTraits<FilesystemCopierAction>::OutputObjectType
@@ -56,6 +62,9 @@
std::string Type() const { return StaticType(); }
private:
+ friend class FilesystemCopierActionTest;
+ FRIEND_TEST(FilesystemCopierActionTest, RunAsRootDetermineFilesystemSizeTest);
+
// Callback from glib when the copy operation is done.
void AsyncReadyCallback(GObject *source_object, GAsyncResult *res);
static void StaticAsyncReadyCallback(GObject *source_object,
@@ -64,16 +73,25 @@
reinterpret_cast<FilesystemCopierAction*>(user_data)->AsyncReadyCallback(
source_object, res);
}
-
+
// Cleans up all the variables we use for async operations and tells
// the ActionProcessor we're done w/ success as passed in.
// was_cancelled should be true if TerminateProcessing() was called.
void Cleanup(bool success, bool was_cancelled);
-
+
+ // Determine, if possible, the source file system size to avoid copying the
+ // whole partition. Currently this supports only the root file system assuming
+ // it's ext3-compatible.
+ void DetermineFilesystemSize(int fd);
+
+ // Returns the number of bytes to read based on the size of the buffer and the
+ // filesystem size.
+ int64_t GetBytesToRead();
+
// If true, this action is copying to the kernel_install_path from
// the install plan, otherwise it's copying just to the install_path.
const bool copying_kernel_install_path_;
-
+
// The path to copy from. If empty (the default), the source is from the
// passed in InstallPlan.
std::string copy_source_;
@@ -82,14 +100,14 @@
// source/destination partitions.
GInputStream* src_stream_;
GOutputStream* dst_stream_;
-
+
// If non-NULL, the cancellable object for the in-flight async call.
GCancellable* canceller_;
-
+
// True if we're waiting on a read to complete; false if we're
// waiting on a write.
bool read_in_flight_;
-
+
// The buffer for storing data we read/write.
std::vector<char> buffer_;
@@ -98,7 +116,15 @@
// The install plan we're passed in via the input pipe.
InstallPlan install_plan_;
-
+
+ // Calculates the hash of the copied data.
+ OmahaHashCalculator hasher_;
+
+ // Copies and hashes this many bytes from the head of the input stream. This
+ // field is initialized when the action is started and decremented as more
+ // bytes get copied.
+ int64_t filesystem_size_;
+
DISALLOW_COPY_AND_ASSIGN(FilesystemCopierAction);
};
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index d9305ff..cd77bef 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -2,12 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include <glib.h>
+#include <fcntl.h>
+
#include <set>
#include <string>
#include <vector>
+
+#include <base/eintr_wrapper.h>
+#include <base/string_util.h>
+#include <glib.h>
#include <gtest/gtest.h>
-#include "base/string_util.h"
+
#include "update_engine/filesystem_copier_action.h"
#include "update_engine/filesystem_iterator.h"
#include "update_engine/omaha_hash_calculator.h"
@@ -314,4 +319,31 @@
DoTest(false, true, false);
}
+TEST_F(FilesystemCopierActionTest, RunAsRootDetermineFilesystemSizeTest) {
+ string img;
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/img.XXXXXX", &img, NULL));
+ ScopedPathUnlinker img_unlinker(img);
+ CreateExtImageAtPath(img, NULL);
+ // Extend the "partition" holding the file system from 10MiB to 20MiB.
+ EXPECT_EQ(0, System(StringPrintf(
+ "dd if=/dev/zero of=%s seek=20971519 bs=1 count=1",
+ img.c_str())));
+ EXPECT_EQ(20 * 1024 * 1024, utils::FileSize(img));
+
+ for (int i = 0; i < 2; ++i) {
+ bool is_kernel = i == 1;
+ FilesystemCopierAction action(is_kernel);
+ EXPECT_EQ(kint64max, action.filesystem_size_);
+ {
+ int fd = HANDLE_EINTR(open(img.c_str(), O_RDONLY));
+ EXPECT_TRUE(fd > 0);
+ ScopedFdCloser fd_closer(&fd);
+ action.DetermineFilesystemSize(fd);
+ }
+ EXPECT_EQ(is_kernel ? kint64max : 10 * 1024 * 1024,
+ action.filesystem_size_);
+ }
+}
+
+
} // namespace chromeos_update_engine
diff --git a/install_plan.h b/install_plan.h
index 8dc42aa..9911d32 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -6,6 +6,8 @@
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_INSTALL_PLAN_H__
#include <string>
+#include <vector>
+
#include "base/logging.h"
// InstallPlan is a simple struct that contains relevant info for many
@@ -37,6 +39,8 @@
std::string download_hash; // hash of the data at the url
std::string install_path; // path to install device
std::string kernel_install_path; // path to kernel install device
+ std::vector<char> current_kernel_hash; // computed by FileSystemCopierAction
+ std::vector<char> current_rootfs_hash; // computed by FileSystemCopierAction
bool operator==(const InstallPlan& that) const {
return (is_full_update == that.is_full_update) &&
diff --git a/omaha_hash_calculator.cc b/omaha_hash_calculator.cc
index fdc70f0..ac4edf4 100644
--- a/omaha_hash_calculator.cc
+++ b/omaha_hash_calculator.cc
@@ -113,6 +113,20 @@
return true;
}
+off_t OmahaHashCalculator::RawHashOfFile(const std::string& name, off_t length,
+ std::vector<char>* out_hash) {
+ OmahaHashCalculator calc;
+ off_t res = calc.UpdateFile(name, length);
+ if (res < 0) {
+ return res;
+ }
+ if (!calc.Finalize()) {
+ return -1;
+ }
+ *out_hash = calc.raw_hash();
+ return res;
+}
+
string OmahaHashCalculator::OmahaHashOfBytes(
const void* data, size_t length) {
OmahaHashCalculator calc;
diff --git a/omaha_hash_calculator.h b/omaha_hash_calculator.h
index 5daba78..dc9fe4e 100644
--- a/omaha_hash_calculator.h
+++ b/omaha_hash_calculator.h
@@ -62,6 +62,8 @@
static bool RawHashOfData(const std::vector<char>& data,
std::vector<char>* out_hash);
+ static off_t RawHashOfFile(const std::string& name, off_t length,
+ std::vector<char>* out_hash);
// Used by tests
static std::string OmahaHashOfBytes(const void* data, size_t length);
diff --git a/omaha_hash_calculator_unittest.cc b/omaha_hash_calculator_unittest.cc
index 5ba3ac1..28dc030 100644
--- a/omaha_hash_calculator_unittest.cc
+++ b/omaha_hash_calculator_unittest.cc
@@ -115,6 +115,25 @@
EXPECT_EQ("qqlAJmTxpB9A67xSyZk+tmrrNmYClY/fqig7ceZNsSM=", calc.hash());
}
+TEST(OmahaHashCalculatorTest, RawHashOfFileSimpleTest) {
+ string data_path;
+ ASSERT_TRUE(
+ utils::MakeTempFile("/tmp/data.XXXXXX", &data_path, NULL));
+ ScopedPathUnlinker data_path_unlinker(data_path);
+ ASSERT_TRUE(utils::WriteFile(data_path.c_str(), "hi", 2));
+
+ static const int kLengths[] = { -1, 2, 10 };
+ for (size_t i = 0; i < arraysize(kLengths); i++) {
+ vector<char> exp_raw_hash(kExpectedRawHash,
+ kExpectedRawHash + arraysize(kExpectedRawHash));
+ vector<char> raw_hash;
+ EXPECT_EQ(2, OmahaHashCalculator::RawHashOfFile(data_path,
+ kLengths[i],
+ &raw_hash));
+ EXPECT_TRUE(exp_raw_hash == raw_hash);
+ }
+}
+
TEST(OmahaHashCalculatorTest, UpdateFileNonexistentTest) {
OmahaHashCalculator calc;
EXPECT_EQ(-1, calc.UpdateFile("/some/non-existent/file", -1));