AU: Use Omaha ID rather than MAC address in delta updater
Review URL: http://codereview.chromium.org/467051
diff --git a/delta_diff_generator_unittest.cc b/delta_diff_generator_unittest.cc
index ef6e7b1..b549041 100644
--- a/delta_diff_generator_unittest.cc
+++ b/delta_diff_generator_unittest.cc
@@ -83,18 +83,18 @@
EXPECT_EQ(0, System(StringPrintf(
"ln -f -s '%s' '%s/compress_link'", kWellCompressingFilename, base_c)));
-// Things that will encode differently:
-EXPECT_EQ(0, System(StringPrintf("mkdir -p '%s/encoding'", base_c)));
-EXPECT_EQ(0, System(StringPrintf("echo nochange > '%s/encoding/nochange'",
- base_c)));
-EXPECT_EQ(0, System(StringPrintf("echo -n > '%s/encoding/onebyte'", base_c)));
-EXPECT_EQ(0, System(StringPrintf("echo -n > '%s/encoding/long_new'",
- base_c)));
-// Random 1 MiB byte length file
-EXPECT_TRUE(WriteFile((base +
- "/encoding/long_small_change").c_str(),
- reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString)));
+ // Things that will encode differently:
+ EXPECT_EQ(0, System(StringPrintf("mkdir -p '%s/encoding'", base_c)));
+ EXPECT_EQ(0, System(StringPrintf("echo nochange > '%s/encoding/nochange'",
+ base_c)));
+ EXPECT_EQ(0, System(StringPrintf("echo -n > '%s/encoding/onebyte'", base_c)));
+ EXPECT_EQ(0, System(StringPrintf("echo -n > '%s/encoding/long_new'",
+ base_c)));
+ // Random 1 MiB byte length file
+ EXPECT_TRUE(utils::WriteFile((base +
+ "/encoding/long_small_change").c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
}
// base points to a folder that was passed to GenerateFilesAtPath().
// This edits some, so that one can make a diff from the original data
diff --git a/install_action_unittest.cc b/install_action_unittest.cc
index 6f2cf2d..68796e6 100644
--- a/install_action_unittest.cc
+++ b/install_action_unittest.cc
@@ -20,7 +20,7 @@
using chromeos_update_engine::DeltaDiffGenerator;
using chromeos_update_engine::kRandomString;
using chromeos_update_engine::System;
-using chromeos_update_engine::WriteFile;
+using chromeos_update_engine::utils::WriteFile;
using std::set;
using std::string;
using std::vector;
diff --git a/integration_unittest.cc b/integration_unittest.cc
index 671240f..e8eca56 100644
--- a/integration_unittest.cc
+++ b/integration_unittest.cc
@@ -133,6 +133,9 @@
ASSERT_EQ(0, System(string("rm -rf ") + kTestDir));
ASSERT_EQ(0, system("rm -f /tmp/update_engine_test_postinst_out.txt"));
ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir + "/etc"));
+ ASSERT_EQ(0, system((string("mkdir -p ") + kTestDir +
+ utils::kStatefulPartition +
+ "/etc").c_str()));
ASSERT_TRUE(WriteFileString(string(kTestDir) + "/etc/lsb-release",
"GOOGLE_RELEASE=0.2.0.0\n"
"GOOGLE_TRACK=unittest-track"));
diff --git a/omaha_request_prep_action.cc b/omaha_request_prep_action.cc
index 21d5799..8c015d8 100644
--- a/omaha_request_prep_action.cc
+++ b/omaha_request_prep_action.cc
@@ -4,7 +4,9 @@
#include "update_engine/omaha_request_prep_action.h"
#include <sys/utsname.h>
+#include <errno.h>
#include <string>
+#include "base/string_util.h"
#include "update_engine/utils.h"
using std::string;
@@ -12,11 +14,19 @@
// This gathers local system information and prepares info used by the
// update check action.
+namespace {
+const string OmahaIdPath() {
+ return chromeos_update_engine::utils::kStatefulPartition + "/etc/omaha_id";
+}
+} // namespace {}
+
namespace chromeos_update_engine {
void OmahaRequestPrepAction::PerformAction() {
// TODO(adlr): honor force_full_update_
- const string machine_id(GetMachineId());
+ ScopedActionCompleter completer(processor_, this);
+ string machine_id;
+ TEST_AND_RETURN(GetMachineId(&machine_id));
const string version(GetLsbValue("GOOGLE_RELEASE"));
const string sp(version + "_" + GetMachineType());
const string track(GetLsbValue("GOOGLE_TRACK"));
@@ -33,28 +43,49 @@
CHECK(HasOutputPipe());
SetOutputObject(out);
- processor_->ActionComplete(this, true);
+ completer.set_success(true);
}
-std::string OmahaRequestPrepAction::GetMachineId() const {
- FILE* fp = popen("/sbin/ifconfig", "r");
- if (!fp)
- return "";
- string data;
- for (;;) {
- char buffer[1000];
- size_t r = fread(buffer, 1, sizeof(buffer), fp);
- if (r <= 0)
- break;
- data.insert(data.end(), buffer, buffer + r);
+namespace {
+const size_t kGuidDataByteLength = 128 / 8;
+const string::size_type kGuidStringLength = 38;
+// Formats 16 bytes (128 bits) of data as a GUID:
+// "{XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX}" where X is a hex digit
+string GuidFromData(const unsigned char data[kGuidDataByteLength]) {
+ return StringPrintf(
+ "{%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X}",
+ data[0], data[1], data[2], data[3], data[4], data[5], data[6], data[7],
+ data[8], data[9], data[10], data[11], data[12], data[13], data[14],
+ data[15]);
+}
+}
+
+// Returns true on success.
+bool OmahaRequestPrepAction::GetMachineId(std::string* out_id) const {
+ // See if we have an existing Machine ID
+ const string omaha_id_path = root_ + OmahaIdPath();
+
+ if (utils::ReadFileToString(omaha_id_path, out_id) &&
+ out_id->size() == kGuidStringLength) {
+ return true;
}
- fclose(fp);
- // scan data for MAC address
- string::size_type pos = data.find(" HWaddr ");
- if (pos == string::npos)
- return "";
- // 3 * 6 - 1 is the number of bytes of the hwaddr.
- return data.substr(pos + strlen(" HWaddr "), 3 * 6 - 1);
+
+ // Create a new ID
+ int rand_fd = open("/dev/urandom", O_RDONLY, 0);
+ TEST_AND_RETURN_FALSE_ERRNO(rand_fd >= 0);
+ ScopedFdCloser rand_fd_closer(&rand_fd);
+ unsigned char buf[kGuidDataByteLength];
+ size_t bytes_read = 0;
+ while (bytes_read < sizeof(buf)) {
+ ssize_t rc = read(rand_fd, buf + bytes_read, sizeof(buf) - bytes_read);
+ TEST_AND_RETURN_FALSE_ERRNO(rc > 0);
+ bytes_read += rc;
+ }
+ string guid = GuidFromData(buf);
+ TEST_AND_RETURN_FALSE(
+ utils::WriteFile(omaha_id_path.c_str(), guid.data(), guid.size()));
+ *out_id = guid;
+ return true;
}
std::string OmahaRequestPrepAction::GetLsbValue(const std::string& key) const {
diff --git a/omaha_request_prep_action.h b/omaha_request_prep_action.h
index ca7fd17..cb82a9f 100644
--- a/omaha_request_prep_action.h
+++ b/omaha_request_prep_action.h
@@ -49,7 +49,7 @@
private:
// Gets a machine-local ID (for now, first MAC address we find)
- std::string GetMachineId() const;
+ bool GetMachineId(std::string* out_id) const;
// Fetches the value for a given key from
// /mnt/stateful_partition/etc/lsb-release if possible. Failing that,
diff --git a/omaha_request_prep_action_unittest.cc b/omaha_request_prep_action_unittest.cc
index 19da5e5..d5a95e0 100644
--- a/omaha_request_prep_action_unittest.cc
+++ b/omaha_request_prep_action_unittest.cc
@@ -64,26 +64,22 @@
}
namespace {
-// Returns true iff str is formatted as a mac address
-bool IsValidMac(const string& str) {
- if (str.size() != (3 * 6 - 1))
- return false;
- for (unsigned int i = 0; i < str.size(); i++) {
- char c = str[i];
- switch (i % 3) {
- case 0: // fall through
- case 1:
- if ((c >= '0') && (c <= '9'))
- break;
- if ((c >= 'a') && (c <= 'f'))
- break;
- if ((c >= 'A') && (c <= 'F'))
- break;
- return false;
- case 2:
- if (c == ':')
- break;
- return false;
+bool IsHexDigit(char c) {
+ return ((c >= '0') && (c <= '9')) ||
+ ((c >= 'a') && (c <= 'f')) ||
+ ((c >= 'A') && (c <= 'F'));
+}
+
+// Returns true iff str is formatted as a GUID. Example GUID:
+// "{2251FFAD-DBAB-4E53-8B3A-18F98BB4EB80}"
+bool IsValidGuid(const string& str) {
+ TEST_AND_RETURN_FALSE(str.size() == 38);
+ TEST_AND_RETURN_FALSE((*str.begin() == '{') && (*str.rbegin() == '}'));
+ for (string::size_type i = 1; i < (str.size() - 1); ++i) {
+ if ((i == 9) || (i == 14) || (i == 19) || (i == 24)) {
+ TEST_AND_RETURN_FALSE(str[i] == '-');
+ } else {
+ TEST_AND_RETURN_FALSE(IsHexDigit(str[i]));
}
}
return true;
@@ -110,15 +106,17 @@
TEST_F(OmahaRequestPrepActionTest, SimpleTest) {
ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir + "/etc"));
+ ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir +
+ utils::kStatefulPartition + "/etc"));
{
ASSERT_TRUE(WriteFileString(
kTestDir + "/etc/lsb-release",
"GOOGLE_FOO=bar\nGOOGLE_RELEASE=0.2.2.3\nGOOGLE_TRACK=footrack"));
UpdateCheckParams out;
EXPECT_TRUE(DoTest(false, &out));
- EXPECT_TRUE(IsValidMac(out.machine_id));
+ EXPECT_TRUE(IsValidGuid(out.machine_id)) << "id: " << out.machine_id;
// for now we're just using the machine id here
- EXPECT_TRUE(IsValidMac(out.user_id));
+ EXPECT_TRUE(IsValidGuid(out.user_id)) << "id: " << out.user_id;
EXPECT_EQ("Chrome OS", out.os_platform);
EXPECT_EQ(string("0.2.2.3_") + GetMachineType(), out.os_sp);
EXPECT_EQ("{87efface-864d-49a5-9bb3-4b050a7c227a}", out.app_id);
@@ -131,15 +129,17 @@
TEST_F(OmahaRequestPrepActionTest, MissingTrackTest) {
ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir + "/etc"));
+ ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir +
+ utils::kStatefulPartition + "/etc"));
{
ASSERT_TRUE(WriteFileString(
kTestDir + "/etc/lsb-release",
"GOOGLE_FOO=bar\nGOOGLE_RELEASE=0.2.2.3\nGOOGLE_TRXCK=footrack"));
UpdateCheckParams out;
EXPECT_TRUE(DoTest(false, &out));
- EXPECT_TRUE(IsValidMac(out.machine_id));
+ EXPECT_TRUE(IsValidGuid(out.machine_id));
// for now we're just using the machine id here
- EXPECT_TRUE(IsValidMac(out.user_id));
+ EXPECT_TRUE(IsValidGuid(out.user_id));
EXPECT_EQ("Chrome OS", out.os_platform);
EXPECT_EQ(string("0.2.2.3_") + GetMachineType(), out.os_sp);
EXPECT_EQ("{87efface-864d-49a5-9bb3-4b050a7c227a}", out.app_id);
@@ -152,6 +152,8 @@
TEST_F(OmahaRequestPrepActionTest, ConfusingReleaseTest) {
ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir + "/etc"));
+ ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir +
+ utils::kStatefulPartition + "/etc"));
{
ASSERT_TRUE(WriteFileString(
kTestDir + "/etc/lsb-release",
@@ -159,9 +161,9 @@
"GOOGLE_RELEASE=0.2.2.3\nGOOGLE_TRXCK=footrack"));
UpdateCheckParams out;
EXPECT_TRUE(DoTest(false, &out));
- EXPECT_TRUE(IsValidMac(out.machine_id));
+ EXPECT_TRUE(IsValidGuid(out.machine_id)) << out.machine_id;
// for now we're just using the machine id here
- EXPECT_TRUE(IsValidMac(out.user_id));
+ EXPECT_TRUE(IsValidGuid(out.user_id));
EXPECT_EQ("Chrome OS", out.os_platform);
EXPECT_EQ(string("0.2.2.3_") + GetMachineType(), out.os_sp);
EXPECT_EQ("{87efface-864d-49a5-9bb3-4b050a7c227a}", out.app_id);
@@ -172,4 +174,26 @@
EXPECT_EQ(0, System(string("rm -rf ") + kTestDir));
}
+TEST_F(OmahaRequestPrepActionTest, MachineIdPersistsTest) {
+ ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir + "/etc"));
+ ASSERT_EQ(0, System(string("mkdir -p ") + kTestDir +
+ utils::kStatefulPartition + "/etc"));
+ ASSERT_TRUE(WriteFileString(
+ kTestDir + "/etc/lsb-release",
+ "GOOGLE_FOO=GOOGLE_RELEASE=1.2.3.4\n"
+ "GOOGLE_RELEASE=0.2.2.3\nGOOGLE_TRXCK=footrack"));
+ UpdateCheckParams out1;
+ EXPECT_TRUE(DoTest(false, &out1));
+ string machine_id;
+ EXPECT_TRUE(utils::ReadFileToString(
+ kTestDir +
+ utils::kStatefulPartition + "/etc/omaha_id",
+ &machine_id));
+ EXPECT_EQ(machine_id, out1.machine_id);
+ UpdateCheckParams out2;
+ EXPECT_TRUE(DoTest(false, &out2));
+ EXPECT_EQ(machine_id, out2.machine_id);
+ EXPECT_EQ(0, System(string("rm -rf ") + kTestDir));
+}
+
} // namespace chromeos_update_engine
diff --git a/test_utils.cc b/test_utils.cc
index 6346bf3..d916458 100644
--- a/test_utils.cc
+++ b/test_utils.cc
@@ -25,22 +25,12 @@
namespace chromeos_update_engine {
-bool WriteFile(const char* path, const char* data, int data_len) {
- DirectFileWriter writer;
- TEST_AND_RETURN_FALSE_ERRNO(0 == writer.Open(path,
- O_WRONLY | O_CREAT | O_TRUNC,
- 0666));
- ScopedFileWriterCloser closer(&writer);
- TEST_AND_RETURN_FALSE_ERRNO(data_len == writer.Write(data, data_len));
- return true;
-}
-
bool WriteFileVector(const std::string& path, const std::vector<char>& data) {
- return WriteFile(path.c_str(), &data[0], data.size());
+ return utils::WriteFile(path.c_str(), &data[0], data.size());
}
bool WriteFileString(const std::string& path, const std::string& data) {
- return WriteFile(path.c_str(), data.data(), data.size());
+ return utils::WriteFile(path.c_str(), data.data(), data.size());
}
off_t FileSize(const string& path) {
diff --git a/test_utils.h b/test_utils.h
index 642158b..6274a22 100644
--- a/test_utils.h
+++ b/test_utils.h
@@ -17,7 +17,6 @@
// Writes the data passed to path. The file at path will be overwritten if it
// exists. Returns true on success, false otherwise.
-bool WriteFile(const char* path, const char* data, int data_len);
bool WriteFileVector(const std::string& path, const std::vector<char>& data);
bool WriteFileString(const std::string& path, const std::string& data);
diff --git a/utils.cc b/utils.cc
index 693062d..9fa0906 100644
--- a/utils.cc
+++ b/utils.cc
@@ -8,12 +8,14 @@
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>
+#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <algorithm>
#include "chromeos/obsolete_logging.h"
+#include "update_engine/file_writer.h"
using std::min;
using std::string;
@@ -23,6 +25,16 @@
namespace utils {
+bool WriteFile(const char* path, const char* data, int data_len) {
+ DirectFileWriter writer;
+ TEST_AND_RETURN_FALSE_ERRNO(0 == writer.Open(path,
+ O_WRONLY | O_CREAT | O_TRUNC,
+ 0666));
+ ScopedFileWriterCloser closer(&writer);
+ TEST_AND_RETURN_FALSE_ERRNO(data_len == writer.Write(data, data_len));
+ return true;
+}
+
bool ReadFile(const std::string& path, std::vector<char>* out) {
CHECK(out);
FILE* fp = fopen(path.c_str(), "r");
diff --git a/utils.h b/utils.h
index e475783..7920f3c 100644
--- a/utils.h
+++ b/utils.h
@@ -15,6 +15,10 @@
namespace utils {
+// Writes the data passed to path. The file at path will be overwritten if it
+// exists. Returns true on success, false otherwise.
+bool WriteFile(const char* path, const char* data, int data_len);
+
// Returns the entire contents of the file at path. Returns true on success.
bool ReadFile(const std::string& path, std::vector<char>* out);
bool ReadFileToString(const std::string& path, std::string* out);