If a public key is present, disallow old style full payloads.
This is necessary to ensure that if a public key is present the payload is
signed and the signature passes verification.
BUG=chromium-os:11760
TEST=unit tests, tested on device
Change-Id: I6af61ead0e918c0b971dbcfeabcab3be03e6eb97
Review URL: http://codereview.chromium.org/6574009
diff --git a/action_processor.h b/action_processor.h
index 124fe00..ef06181 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -40,6 +40,7 @@
kActionCodeDownloadWriteError = 14,
kActionCodeNewRootfsVerificationError = 15,
kActionCodeNewKernelVerificationError = 16,
+ kActionCodeSignedDeltaPayloadExpectedError = 17,
kActionCodeOmahaRequestEmptyResponseError = 200,
kActionCodeOmahaRequestXMLParseError = 201,
kActionCodeOmahaRequestNoUpdateCheckNode = 202,
diff --git a/delta_performer.cc b/delta_performer.cc
index 117e8f5..1a98102 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -33,12 +33,13 @@
namespace chromeos_update_engine {
+const char DeltaPerformer::kUpdatePayloadPublicKeyPath[] =
+ "/usr/share/update_engine/update-payload-key.pub.pem";
+
namespace {
const int kDeltaVersionLength = 8;
const int kDeltaProtobufLengthLength = 8;
-const char kUpdatePayloadPublicKeyPath[] =
- "/usr/share/update_engine/update-payload-key.pub.pem";
const int kUpdateStateOperationInvalid = -1;
const int kMaxResumedUpdateFailures = 10;
diff --git a/delta_performer.h b/delta_performer.h
index c693333..7cc1d9b 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -31,6 +31,8 @@
kMetadataParseInsufficientData,
};
+ static const char kUpdatePayloadPublicKeyPath[];
+
DeltaPerformer(PrefsInterface* prefs)
: prefs_(prefs),
fd_(-1),
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index fdbd80a..74e149f 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -19,6 +19,11 @@
const char OmahaResponseHandlerAction::kDeadlineFile[] =
"/tmp/update-check-response-deadline";
+OmahaResponseHandlerAction::OmahaResponseHandlerAction(PrefsInterface* prefs)
+ : prefs_(prefs),
+ got_no_update_response_(false),
+ key_path_(DeltaPerformer::kUpdatePayloadPublicKeyPath) {}
+
void OmahaResponseHandlerAction::PerformAction() {
CHECK(HasInputObject());
ScopedActionCompleter completer(processor_, this);
@@ -49,6 +54,11 @@
utils::BootKernelDevice(install_plan_.install_path);
install_plan_.is_full_update = !response.is_delta;
+ if (!response.is_delta && utils::FileExists(key_path_.c_str())) {
+ // Can't sign old style full payloads but signature is required so bail out.
+ completer.set_code(kActionCodeSignedDeltaPayloadExpectedError);
+ return;
+ }
TEST_AND_RETURN(HasOutputPipe());
if (HasOutputPipe())
diff --git a/omaha_response_handler_action.h b/omaha_response_handler_action.h
index 25c57e0..1785138 100644
--- a/omaha_response_handler_action.h
+++ b/omaha_response_handler_action.h
@@ -29,9 +29,7 @@
public:
static const char kDeadlineFile[];
- OmahaResponseHandlerAction(PrefsInterface* prefs)
- : prefs_(prefs),
- got_no_update_response_(false) {}
+ OmahaResponseHandlerAction(PrefsInterface* prefs);
typedef ActionTraits<OmahaResponseHandlerAction>::InputObjectType
InputObjectType;
typedef ActionTraits<OmahaResponseHandlerAction>::OutputObjectType
@@ -53,6 +51,7 @@
// Debugging/logging
static std::string StaticType() { return "OmahaResponseHandlerAction"; }
std::string Type() const { return StaticType(); }
+ void set_key_path(const std::string& path) { key_path_ = path; }
private:
// Assumes you want to install on the "other" device, where the other
@@ -74,6 +73,9 @@
// True only if we got a response and the response said no updates
bool got_no_update_response_;
+ // Public key path to use for payload verification.
+ std::string key_path_;
+
DISALLOW_COPY_AND_ASSIGN(OmahaResponseHandlerAction);
};
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 7f1ee15..4a60315 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -23,6 +23,7 @@
// If out is non-NULL, it's set w/ the response from the action.
bool DoTest(const OmahaResponse& in,
const string& boot_dev,
+ bool test_key,
InstallPlan* out);
};
@@ -58,6 +59,7 @@
bool OmahaResponseHandlerActionTest::DoTest(const OmahaResponse& in,
const string& boot_dev,
+ bool test_key,
InstallPlan* out) {
ActionProcessor processor;
OmahaResponseHandlerActionProcessorDelegate delegate;
@@ -71,6 +73,9 @@
.WillOnce(Return(true));
}
OmahaResponseHandlerAction response_handler_action(&prefs);
+ if (test_key) {
+ response_handler_action.set_key_path("/dev/null");
+ }
response_handler_action.set_boot_device(boot_dev);
BondActions(&feeder_action, &response_handler_action);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -84,7 +89,9 @@
if (out)
*out = collector_action.object();
EXPECT_TRUE(delegate.code_set_);
- return delegate.code_ == kActionCodeSuccess;
+ ActionExitCode expected_code = test_key ?
+ kActionCodeSignedDeltaPayloadExpectedError : kActionCodeSuccess;
+ return delegate.code_ == expected_code;
}
TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
@@ -103,7 +110,7 @@
in.is_delta = false;
in.deadline = "20101020";
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", false, &install_plan));
EXPECT_TRUE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
@@ -131,7 +138,7 @@
in.prompt = true;
in.is_delta = true;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", false, &install_plan));
EXPECT_FALSE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
@@ -154,7 +161,7 @@
in.is_delta = false;
in.deadline = "some-deadline";
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", false, &install_plan));
EXPECT_TRUE(install_plan.is_full_update);
EXPECT_EQ(in.codebase, install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.download_hash);
@@ -167,11 +174,20 @@
}
}
+TEST_F(OmahaResponseHandlerActionTest, PublicKeyOldStyleTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.codebase = "http://foo/the_update_a.b.c.d.tgz";
+ in.is_delta = false;
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", true, &install_plan));
+}
+
TEST_F(OmahaResponseHandlerActionTest, NoUpdatesTest) {
OmahaResponse in;
in.update_exists = false;
InstallPlan install_plan;
- EXPECT_FALSE(DoTest(in, "/dev/sda1", &install_plan));
+ EXPECT_FALSE(DoTest(in, "/dev/sda1", false, &install_plan));
EXPECT_FALSE(install_plan.is_full_update);
EXPECT_EQ("", install_plan.download_url);
EXPECT_EQ("", install_plan.download_hash);
diff --git a/update_attempter.cc b/update_attempter.cc
index 3ea1c18..949434e 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -456,12 +456,12 @@
return;
}
- // For now assume that Omaha response action failure means that
- // there's no update so don't send an event. Also, double check that
- // the failure has not occurred while sending an error event -- in
- // which case don't schedule another. This shouldn't really happen
- // but just in case...
- if (action->Type() == OmahaResponseHandlerAction::StaticType() ||
+ // For now assume that a generic Omaha response action failure means that
+ // there's no update so don't send an event. Also, double check that the
+ // failure has not occurred while sending an error event -- in which case
+ // don't schedule another. This shouldn't really happen but just in case...
+ if ((action->Type() == OmahaResponseHandlerAction::StaticType() &&
+ code == kActionCodeError) ||
status_ == UPDATE_STATUS_REPORTING_ERROR_EVENT) {
return;
}