diff options
| -rw-r--r-- | common/utils.cc | 31 | ||||
| -rw-r--r-- | common/utils.h | 8 | ||||
| -rw-r--r-- | common/utils_unittest.cc | 43 | ||||
| -rw-r--r-- | omaha_request_action.cc | 28 | ||||
| -rw-r--r-- | omaha_request_action_unittest.cc | 74 | ||||
| -rw-r--r-- | omaha_response.h | 22 | ||||
| -rw-r--r-- | omaha_response_handler_action.cc | 18 | ||||
| -rw-r--r-- | omaha_response_handler_action_unittest.cc | 18 |
8 files changed, 209 insertions, 33 deletions
diff --git a/common/utils.cc b/common/utils.cc index b06954b0..1a8fd534 100644 --- a/common/utils.cc +++ b/common/utils.cc @@ -61,6 +61,7 @@ using base::Time; using base::TimeDelta; using std::min; +using std::numeric_limits; using std::pair; using std::string; using std::vector; @@ -1082,6 +1083,36 @@ int VersionPrefix(const std::string& version) { return value; } +void ParseRollbackKeyVersion(const string& raw_version, + uint16_t* high_version, + uint16_t* low_version) { + DCHECK(high_version); + DCHECK(low_version); + *high_version = numeric_limits<uint16_t>::max(); + *low_version = numeric_limits<uint16_t>::max(); + + vector<string> parts = base::SplitString( + raw_version, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); + if (parts.size() != 2) { + // The version string must have exactly one period. + return; + } + + int high; + int low; + if (!(base::StringToInt(parts[0], &high) && + base::StringToInt(parts[1], &low))) { + // Both parts of the version could not be parsed correctly. + return; + } + + if (high >= 0 && high < numeric_limits<uint16_t>::max() && low >= 0 && + low < numeric_limits<uint16_t>::max()) { + *high_version = static_cast<uint16_t>(high); + *low_version = static_cast<uint16_t>(low); + } +} + } // namespace utils } // namespace chromeos_update_engine diff --git a/common/utils.h b/common/utils.h index cbc5eb97..ecb97a34 100644 --- a/common/utils.h +++ b/common/utils.h @@ -21,6 +21,7 @@ #include <unistd.h> #include <algorithm> +#include <limits> #include <map> #include <memory> #include <set> @@ -322,6 +323,13 @@ bool GetBootId(std::string* boot_id); // first section of |version| is invalid (e.g. not a number). int VersionPrefix(const std::string& version); +// Parses a string in the form high.low, where high and low are 16 bit unsigned +// integers. If there is more than 1 dot, or if either of the two parts are +// not valid 16 bit unsigned numbers, then 0xffff is returned for both. +void ParseRollbackKeyVersion(const std::string& raw_version, + uint16_t* high_version, + uint16_t* low_version); + } // namespace utils diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc index 16fa4812..1590eeb4 100644 --- a/common/utils_unittest.cc +++ b/common/utils_unittest.cc @@ -22,6 +22,7 @@ #include <sys/stat.h> #include <sys/types.h> +#include <limits> #include <string> #include <vector> @@ -32,6 +33,7 @@ #include "update_engine/common/test_utils.h" +using std::numeric_limits; using std::string; using std::vector; @@ -450,6 +452,22 @@ static void VoidMacroTestHelper(bool* ret) { *ret = true; } +static void ExpectParseRollbackKeyVersion(const string& version, + uint16_t expected_high, + uint16_t expected_low) { + uint16_t actual_high; + uint16_t actual_low; + utils::ParseRollbackKeyVersion(version, &actual_high, &actual_low); + EXPECT_EQ(expected_high, actual_high); + EXPECT_EQ(expected_low, actual_low); +} + +static void ExpectInvalidParseRollbackKeyVersion(const string& version) { + ExpectParseRollbackKeyVersion(version, + numeric_limits<uint16_t>::max(), + numeric_limits<uint16_t>::max()); +} + TEST(UtilsTest, TestMacros) { bool void_test = false; VoidMacroTestHelper(&void_test); @@ -523,4 +541,29 @@ TEST(UtilsTest, VersionPrefix) { EXPECT_EQ(-1, utils::VersionPrefix("x.1")); } +TEST(UtilsTest, ParseDottedVersion) { + // Valid case. + ExpectParseRollbackKeyVersion("2.3", 2, 3); + ExpectParseRollbackKeyVersion("65535.65535", 65535, 65535); + + // Zero is technically allowed but never actually used. + ExpectParseRollbackKeyVersion("0.0", 0, 0); + + // Invalid cases. + ExpectInvalidParseRollbackKeyVersion(""); + ExpectInvalidParseRollbackKeyVersion("2"); + ExpectInvalidParseRollbackKeyVersion("2."); + ExpectInvalidParseRollbackKeyVersion(".2"); + ExpectInvalidParseRollbackKeyVersion("2.2."); + ExpectInvalidParseRollbackKeyVersion("2.2.3"); + ExpectInvalidParseRollbackKeyVersion(".2.2"); + ExpectInvalidParseRollbackKeyVersion("a.b"); + ExpectInvalidParseRollbackKeyVersion("1.b"); + ExpectInvalidParseRollbackKeyVersion("a.2"); + ExpectInvalidParseRollbackKeyVersion("65536.65536"); + ExpectInvalidParseRollbackKeyVersion("99999.99999"); + ExpectInvalidParseRollbackKeyVersion("99999.1"); + ExpectInvalidParseRollbackKeyVersion("1.99999"); +} + } // namespace chromeos_update_engine diff --git a/omaha_request_action.cc b/omaha_request_action.cc index ca6e3485..35020485 100644 --- a/omaha_request_action.cc +++ b/omaha_request_action.cc @@ -18,6 +18,7 @@ #include <inttypes.h> +#include <limits> #include <map> #include <sstream> #include <string> @@ -55,6 +56,7 @@ using base::Time; using base::TimeDelta; using chromeos_update_manager::kRollforwardInfinity; using std::map; +using std::numeric_limits; using std::string; using std::vector; @@ -938,6 +940,20 @@ bool ParsePackage(OmahaParserData::App* app, return true; } +// Parses the 2 key version strings kernel_version and firmware_version. If the +// field is not present, or cannot be parsed the values default to 0xffff. +void ParseRollbackVersions(OmahaParserData* parser_data, + OmahaResponse* output_object) { + utils::ParseRollbackKeyVersion( + parser_data->updatecheck_attrs[kFirmwareVersion], + &output_object->rollback_key_version.firmware_key, + &output_object->rollback_key_version.firmware); + utils::ParseRollbackKeyVersion( + parser_data->updatecheck_attrs[kKernelVersion], + &output_object->rollback_key_version.kernel_key, + &output_object->rollback_key_version.kernel); +} + } // namespace bool OmahaRequestAction::ParseResponse(OmahaParserData* parser_data, @@ -1004,14 +1020,10 @@ bool OmahaRequestAction::ParseResponse(OmahaParserData* parser_data, // Defaults to false if attribute is not present. output_object->is_rollback = ParseBool(parser_data->updatecheck_attrs[kRollback]); - // Defaults to 0 if attribute is not present. This is fine for these values, - // because it's the lowest possible value, and the rollback image won't be - // installed, if the values in the TPM are larger than these, and in case the - // values in the TPM are 0, all images will be able to boot up. - output_object->firmware_version = static_cast<uint32_t>( - ParseInt(parser_data->updatecheck_attrs[kFirmwareVersion])); - output_object->kernel_version = static_cast<uint32_t>( - ParseInt(parser_data->updatecheck_attrs[kKernelVersion])); + + // Parses the rollback versions of the current image. If the fields do not + // exist they default to 0xffff for the 4 key versions. + ParseRollbackVersions(parser_data, output_object); if (!ParseStatus(parser_data, output_object, completer)) return false; diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc index 7c9fe41e..21aecb22 100644 --- a/omaha_request_action_unittest.cc +++ b/omaha_request_action_unittest.cc @@ -84,6 +84,16 @@ const char kTestAppId2[] = "test-app2-id"; // This is a helper struct to allow unit tests build an update response with the // values they care about. struct FakeUpdateResponse { + string GetRollbackVersionAttributes() const { + return (rollback ? " _rollback=\"true\"" : "") + + (!rollback_firmware_version.empty() + ? " _firmware_version=\"" + rollback_firmware_version + "\"" + : "") + + (!rollback_kernel_version.empty() + ? " _kernel_version=\"" + rollback_kernel_version + "\"" + : ""); + } + string GetNoUpdateResponse() const { string entity_str; if (include_entity) @@ -121,8 +131,8 @@ struct FakeUpdateResponse { "\" cohortname=\"" + cohortname + "\" " : "") + " status=\"ok\">" - "<ping status=\"ok\"/><updatecheck status=\"ok\">" - "<urls><url codebase=\"" + + "<ping status=\"ok\"/><updatecheck status=\"ok\"" + + GetRollbackVersionAttributes() + ">" + "<urls><url codebase=\"" + codebase + "\"/></urls>" "<manifest version=\"" + @@ -220,6 +230,13 @@ struct FakeUpdateResponse { bool multi_app_no_update = false; // Whether to include more than one package in an app. bool multi_package = false; + + // Whether the payload is a rollback. + bool rollback = false; + // The verified boot firmware key version for the rollback image. + string rollback_firmware_version = ""; + // The verified boot kernel key version for the rollback image. + string rollback_kernel_version = ""; }; } // namespace @@ -292,7 +309,8 @@ class OmahaRequestActionTest : public ::testing::Test { void TestRollbackCheck(bool is_consumer_device, int rollback_allowed_milestones, - bool is_policy_loaded); + bool is_policy_loaded, + OmahaResponse* out_response); // Runs and checks a ping test. |ping_only| indicates whether it should send // only a ping or also an updatecheck. @@ -493,8 +511,8 @@ bool OmahaRequestActionTest::TestUpdateCheck( void OmahaRequestActionTest::TestRollbackCheck(bool is_consumer_device, int rollback_allowed_milestones, - bool is_policy_loaded) { - OmahaResponse response; + bool is_policy_loaded, + OmahaResponse* out_response) { fake_update_response_.deadline = "20101020"; ASSERT_TRUE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(), -1, @@ -506,9 +524,9 @@ void OmahaRequestActionTest::TestRollbackCheck(bool is_consumer_device, metrics::CheckResult::kUpdateAvailable, metrics::CheckReaction::kUpdating, metrics::DownloadErrorCode::kUnset, - &response, + out_response, nullptr)); - ASSERT_TRUE(response.update_exists); + ASSERT_TRUE(out_response->update_exists); } // Tests Event requests -- they should always succeed. |out_post_data| @@ -2822,9 +2840,11 @@ TEST_F(OmahaRequestActionTest, NoPolicyEnterpriseDevicesSetMaxRollback) { ReportKeyVersionMetrics(min_kernel_version, min_kernel_version, true)) .Times(1); + OmahaResponse response; TestRollbackCheck(false /* is_consumer_device */, 3 /* rollback_allowed_milestones */, - false /* is_policy_loaded */); + false /* is_policy_loaded */, + &response); // Verify kernel_max_rollforward was set to the current minimum // kernel key version. This has the effect of freezing roll @@ -2854,9 +2874,11 @@ TEST_F(OmahaRequestActionTest, NoPolicyConsumerDevicesSetMaxRollback) { ReportKeyVersionMetrics(min_kernel_version, kRollforwardInfinity, true)) .Times(1); + OmahaResponse response; TestRollbackCheck(true /* is_consumer_device */, 3 /* rollback_allowed_milestones */, - false /* is_policy_loaded */); + false /* is_policy_loaded */, + &response); // Verify that with rollback disabled that kernel_max_rollforward // was set to logical infinity. This is the expected behavior for @@ -2885,9 +2907,11 @@ TEST_F(OmahaRequestActionTest, RollbackEnabledDevicesSetMaxRollback) { ReportKeyVersionMetrics(min_kernel_version, min_kernel_version, true)) .Times(1); + OmahaResponse response; TestRollbackCheck(false /* is_consumer_device */, allowed_milestones, - true /* is_policy_loaded */); + true /* is_policy_loaded */, + &response); // Verify that with rollback enabled that kernel_max_rollforward // was set to the current minimum kernel key version. This has @@ -2917,9 +2941,11 @@ TEST_F(OmahaRequestActionTest, RollbackDisabledDevicesSetMaxRollback) { ReportKeyVersionMetrics(min_kernel_version, kRollforwardInfinity, true)) .Times(1); + OmahaResponse response; TestRollbackCheck(false /* is_consumer_device */, allowed_milestones, - true /* is_policy_loaded */); + true /* is_policy_loaded */, + &response); // Verify that with rollback disabled that kernel_max_rollforward // was set to logical infinity. @@ -2927,4 +2953,30 @@ TEST_F(OmahaRequestActionTest, RollbackDisabledDevicesSetMaxRollback) { EXPECT_EQ(kRollforwardInfinity, fake_hw->GetMaxKernelKeyRollforward()); } +TEST_F(OmahaRequestActionTest, RollbackResponseParsedNoEntries) { + OmahaResponse response; + fake_update_response_.rollback = true; + TestRollbackCheck(false /* is_consumer_device */, + 4 /* rollback_allowed_milestones */, + true /* is_policy_loaded */, + &response); + EXPECT_TRUE(response.is_rollback); +} + +TEST_F(OmahaRequestActionTest, RollbackResponseValidVersionsParsed) { + OmahaResponse response; + fake_update_response_.rollback_firmware_version = "1.2"; + fake_update_response_.rollback_kernel_version = "3.4"; + fake_update_response_.rollback = true; + TestRollbackCheck(false /* is_consumer_device */, + 4 /* rollback_allowed_milestones */, + true /* is_policy_loaded */, + &response); + EXPECT_TRUE(response.is_rollback); + EXPECT_EQ(1, response.rollback_key_version.firmware_key); + EXPECT_EQ(2, response.rollback_key_version.firmware); + EXPECT_EQ(3, response.rollback_key_version.kernel_key); + EXPECT_EQ(4, response.rollback_key_version.kernel); +} + } // namespace chromeos_update_engine diff --git a/omaha_response.h b/omaha_response.h index ef69de27..0ac09df1 100644 --- a/omaha_response.h +++ b/omaha_response.h @@ -21,6 +21,7 @@ #include <sys/stat.h> #include <sys/types.h> +#include <limits> #include <string> #include <vector> @@ -86,12 +87,21 @@ struct OmahaResponse { // True if the returned image is a rollback for the device. bool is_rollback = false; - // Kernel version of the returned rollback image. 0 if the image is not a - // rollback. - uint32_t kernel_version = 0; - // Firmware version of the returned rollback image. 0 if the image is not a - // rollback. - uint32_t firmware_version = 0; + + struct RollbackKeyVersion { + // Kernel key version. 0xffff if the value is unknown. + uint16_t kernel_key = std::numeric_limits<uint16_t>::max(); + // Kernel version. 0xffff if the value is unknown. + uint16_t kernel = std::numeric_limits<uint16_t>::max(); + // Firmware key verison. 0xffff if the value is unknown. + uint16_t firmware_key = std::numeric_limits<uint16_t>::max(); + // Firmware version. 0xffff if the value is unknown. + uint16_t firmware = std::numeric_limits<uint16_t>::max(); + }; + + // Key versions of the returned rollback image. Values are 0xffff if the + // image not a rollback, or the fields were not present. + RollbackKeyVersion rollback_key_version; }; static_assert(sizeof(off_t) == 8, "off_t not 64 bit"); diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc index 775c0a84..3a8439da 100644 --- a/omaha_response_handler_action.cc +++ b/omaha_response_handler_action.cc @@ -16,6 +16,7 @@ #include "update_engine/omaha_response_handler_action.h" +#include <limits> #include <string> #include <base/logging.h> @@ -36,6 +37,7 @@ using chromeos_update_manager::Policy; using chromeos_update_manager::UpdateManager; +using std::numeric_limits; using std::string; namespace chromeos_update_engine { @@ -149,8 +151,20 @@ void OmahaResponseHandlerAction::PerformAction() { system_state_->hardware()->GetMinKernelKeyVersion()); auto min_firmware_key_version = static_cast<uint32_t>( system_state_->hardware()->GetMinFirmwareKeyVersion()); - if (response.kernel_version < min_kernel_key_version || - response.firmware_version < min_firmware_key_version) { + uint32_t kernel_key_version = + static_cast<uint32_t>(response.rollback_key_version.kernel_key) << 16 | + static_cast<uint32_t>(response.rollback_key_version.kernel); + uint32_t firmware_key_version = + static_cast<uint32_t>(response.rollback_key_version.firmware_key) + << 16 | + static_cast<uint32_t>(response.rollback_key_version.firmware); + + // Don't attempt a rollback if the versions are incompatible or the + // target image does not specify the version information. + if (kernel_key_version == numeric_limits<uint32_t>::max() || + firmware_key_version == numeric_limits<uint32_t>::max() || + kernel_key_version < min_kernel_key_version || + firmware_key_version < min_firmware_key_version) { LOG(ERROR) << "Device won't be able to boot up the rollback image."; completer.set_code(ErrorCode::kRollbackNotPossible); return; diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc index 7437f50b..c598c893 100644 --- a/omaha_response_handler_action_unittest.cc +++ b/omaha_response_handler_action_unittest.cc @@ -502,8 +502,10 @@ TEST_F(OmahaResponseHandlerActionTest, RollbackTest) { .size = 1, .hash = kPayloadHashHex}); in.is_rollback = true; - in.kernel_version = 0x00010002; - in.firmware_version = 0x00030004; + in.rollback_key_version.kernel = 1; + in.rollback_key_version.kernel = 2; + in.rollback_key_version.firmware_key = 3; + in.rollback_key_version.firmware = 4; fake_system_state_.fake_hardware()->SetMinKernelKeyVersion(0x00010002); fake_system_state_.fake_hardware()->SetMinFirmwareKeyVersion(0x00030004); @@ -524,8 +526,10 @@ TEST_F(OmahaResponseHandlerActionTest, RollbackKernelVersionErrorTest) { .size = 1, .hash = kPayloadHashHex}); in.is_rollback = true; - in.kernel_version = 0x00010001; // This is lower than the minimum. - in.firmware_version = 0x00030004; + in.rollback_key_version.kernel_key = 1; + in.rollback_key_version.kernel = 1; // This is lower than the minimum. + in.rollback_key_version.firmware_key = 3; + in.rollback_key_version.firmware = 4; fake_system_state_.fake_hardware()->SetMinKernelKeyVersion(0x00010002); fake_system_state_.fake_hardware()->SetMinFirmwareKeyVersion(0x00030004); @@ -545,8 +549,10 @@ TEST_F(OmahaResponseHandlerActionTest, RollbackFirmwareVersionErrorTest) { .size = 1, .hash = kPayloadHashHex}); in.is_rollback = true; - in.kernel_version = 0x00010002; - in.firmware_version = 0x00030003; // This is lower than the minimum. + in.rollback_key_version.kernel_key = 1; + in.rollback_key_version.kernel = 2; + in.rollback_key_version.firmware_key = 3; + in.rollback_key_version.firmware = 3; // This is lower than the minimum. fake_system_state_.fake_hardware()->SetMinKernelKeyVersion(0x00010002); fake_system_state_.fake_hardware()->SetMinFirmwareKeyVersion(0x00030004); |