update_engine: Share payloads for a maximum of five days only.
This helps limit exposure of what version the device may or may not be
running.
BUG=chromium:424823
TEST=New unit test + unit tests pass.
Change-Id: I9db6f8d560e359c2b8a65d1381dc44320ed4b3a1
Reviewed-on: https://chromium-review.googlesource.com/227592
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/constants.h b/constants.h
index 05909fa..0f200cb 100644
--- a/constants.h
+++ b/constants.h
@@ -113,6 +113,9 @@
// The maximum number of payload files to keep in /var/cache/p2p.
const int kMaxP2PFilesToKeep = 3;
+// The maximum number of days to keep a p2p file;
+const int kMaxP2PFileAgeDays = 5;
+
// The default number of UMA buckets for metrics.
const int kNumDefaultUmaBuckets = 50;
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index f241a32..78e3f5c 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -470,7 +470,9 @@
// Setup p2p.
FakeP2PManagerConfiguration *test_conf = new FakeP2PManagerConfiguration();
- p2p_manager_.reset(P2PManager::Construct(test_conf, nullptr, "cros_au", 3));
+ p2p_manager_.reset(P2PManager::Construct(
+ test_conf, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
fake_system_state_.set_p2p_manager(p2p_manager_.get());
}
diff --git a/p2p_manager.cc b/p2p_manager.cc
index 240b44a..abfb3d7 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -96,8 +96,10 @@
public:
P2PManagerImpl(Configuration *configuration,
PrefsInterface *prefs,
+ ClockInterface *clock,
const string& file_extension,
- const int num_files_to_keep);
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age);
// P2PManager methods.
virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy);
@@ -138,6 +140,10 @@
// Utility function used by EnsureP2PRunning() and EnsureP2PNotRunning().
bool EnsureP2P(bool should_be_running);
+ // Utility function to delete a file given by |path| and log the
+ // path as well as |reason|. Returns false on failure.
+ bool DeleteP2PFile(const FilePath& path, const std::string& reason);
+
// The device policy being used or null if no policy is being used.
const policy::DevicePolicy* device_policy_;
@@ -147,6 +153,9 @@
// Object for persisted state.
PrefsInterface* prefs_;
+ // Object for telling the time.
+ ClockInterface* clock_;
+
// A short string unique to the application (for example "cros_au")
// used to mark a file as being owned by a particular application.
const string file_extension_;
@@ -156,6 +165,10 @@
// performing housekeeping.
const int num_files_to_keep_;
+ // If non-zero, files older than this will not be kept after
+ // performing housekeeping.
+ const base::TimeDelta max_file_age_;
+
// The string ".p2p".
static const char kP2PExtension[];
@@ -174,12 +187,16 @@
P2PManagerImpl::P2PManagerImpl(Configuration *configuration,
PrefsInterface *prefs,
+ ClockInterface *clock,
const string& file_extension,
- const int num_files_to_keep)
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age)
: device_policy_(nullptr),
prefs_(prefs),
+ clock_(clock),
file_extension_(file_extension),
- num_files_to_keep_(num_files_to_keep) {
+ num_files_to_keep_(num_files_to_keep),
+ max_file_age_(max_file_age) {
configuration_.reset(configuration != nullptr ? configuration :
new ConfigurationImpl());
}
@@ -318,27 +335,35 @@
return configuration_->GetP2PDir().Append(file_id + GetExt(visibility));
}
-bool P2PManagerImpl::PerformHousekeeping() {
- GDir* dir = nullptr;
- GError* error = nullptr;
- const char* name = nullptr;
- vector<pair<FilePath, Time>> matches;
+bool P2PManagerImpl::DeleteP2PFile(const FilePath& path,
+ const std::string& reason) {
+ LOG(INFO) << "Deleting p2p file " << path.value()
+ << " (reason: " << reason << ")";
+ if (unlink(path.value().c_str()) != 0) {
+ PLOG(ERROR) << "Error deleting p2p file " << path.value();
+ return false;
+ }
+ return true;
+}
- // Go through all files in the p2p dir and pick the ones that match
- // and get their ctime.
+
+bool P2PManagerImpl::PerformHousekeeping() {
+ // Open p2p dir.
FilePath p2p_dir = configuration_->GetP2PDir();
- dir = g_dir_open(p2p_dir.value().c_str(), 0, &error);
+ GError* error = nullptr;
+ GDir* dir = g_dir_open(p2p_dir.value().c_str(), 0, &error);
if (dir == nullptr) {
LOG(ERROR) << "Error opening directory " << p2p_dir.value() << ": "
<< utils::GetAndFreeGError(&error);
return false;
}
- if (num_files_to_keep_ == 0)
- return true;
-
+ // Go through all files and collect their mtime.
string ext_visible = GetExt(kVisible);
string ext_non_visible = GetExt(kNonVisible);
+ bool deletion_failed = false;
+ const char* name = nullptr;
+ vector<pair<FilePath, Time>> matches;
while ((name = g_dir_read_name(dir)) != nullptr) {
if (!(g_str_has_suffix(name, ext_visible.c_str()) ||
g_str_has_suffix(name, ext_non_visible.c_str())))
@@ -351,26 +376,34 @@
continue;
}
- Time time = utils::TimeFromStructTimespec(&statbuf.st_ctim);
- matches.push_back(std::make_pair(file, time));
+ Time time = utils::TimeFromStructTimespec(&statbuf.st_mtim);
+
+ // If instructed to keep only files younger than a given age
+ // (|max_file_age_| != 0), delete files satisfying this criteria
+ // right now. Otherwise add it to a list we'll consider for later.
+ if (clock_ != nullptr && max_file_age_ != base::TimeDelta() &&
+ clock_->GetWallclockTime() - time > max_file_age_) {
+ if (!DeleteP2PFile(file, "file too old"))
+ deletion_failed = true;
+ } else {
+ matches.push_back(std::make_pair(file, time));
+ }
}
g_dir_close(dir);
- // Sort list of matches, newest (biggest time) to oldest (lowest time).
- std::sort(matches.begin(), matches.end(), MatchCompareFunc);
-
- // Delete starting at element num_files_to_keep_.
- vector<pair<FilePath, Time>>::const_iterator i;
- for (i = matches.begin() + num_files_to_keep_; i < matches.end(); ++i) {
- const FilePath& file = i->first;
- LOG(INFO) << "Deleting p2p file " << file.value();
- if (unlink(file.value().c_str()) != 0) {
- PLOG(ERROR) << "Error deleting p2p file " << file.value();
- return false;
+ // If instructed to only keep N files (|max_files_to_keep_ != 0),
+ // sort list of matches, newest (biggest time) to oldest (lowest
+ // time). Then delete starting at element |num_files_to_keep_|.
+ if (num_files_to_keep_ > 0) {
+ std::sort(matches.begin(), matches.end(), MatchCompareFunc);
+ vector<pair<FilePath, Time>>::const_iterator i;
+ for (i = matches.begin() + num_files_to_keep_; i < matches.end(); ++i) {
+ if (!DeleteP2PFile(i->first, "too many files"))
+ deletion_failed = true;
}
}
- return true;
+ return !deletion_failed;
}
// Helper class for implementing LookupUrlForFile().
@@ -774,12 +807,16 @@
P2PManager* P2PManager::Construct(Configuration *configuration,
PrefsInterface *prefs,
+ ClockInterface *clock,
const string& file_extension,
- const int num_files_to_keep) {
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age) {
return new P2PManagerImpl(configuration,
prefs,
+ clock,
file_extension,
- num_files_to_keep);
+ num_files_to_keep,
+ max_file_age);
}
} // namespace chromeos_update_engine
diff --git a/p2p_manager.h b/p2p_manager.h
index ddd0fda..4c6e508 100644
--- a/p2p_manager.h
+++ b/p2p_manager.h
@@ -15,6 +15,7 @@
#include <policy/device_policy.h>
#include <policy/libpolicy.h>
+#include "update_engine/clock_interface.h"
#include "update_engine/prefs_interface.h"
namespace chromeos_update_engine {
@@ -164,11 +165,15 @@
//
// The |num_files_to_keep| parameter specifies how many files to
// keep after performing housekeeping (cf. the PerformHousekeeping()
- // method). If zero is passed, no files will be deleted.
+ // method) - pass zero to allow infinitely many files. The
+ // |max_file_age| parameter specifies the maximum file age after
+ // performing housekeeping (pass zero to allow files of any age).
static P2PManager* Construct(Configuration *configuration,
PrefsInterface *prefs,
+ ClockInterface *clock,
const std::string& file_extension,
- const int num_files_to_keep);
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age);
};
} // namespace chromeos_update_engine
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index 86dba85..fe53955 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -22,6 +22,7 @@
#include <policy/libpolicy.h>
#include <policy/mock_device_policy.h>
+#include "update_engine/fake_clock.h"
#include "update_engine/fake_p2p_manager_configuration.h"
#include "update_engine/prefs.h"
#include "update_engine/test_utils.h"
@@ -61,8 +62,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
EXPECT_FALSE(manager->IsP2PEnabled());
EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
@@ -77,8 +79,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
EXPECT_FALSE(manager->IsP2PEnabled());
prefs.SetBoolean(kPrefsP2PEnabled, true);
EXPECT_TRUE(manager->IsP2PEnabled());
@@ -97,8 +100,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
unique_ptr<policy::MockDevicePolicy> device_policy(
new policy::MockDevicePolicy());
EXPECT_CALL(*device_policy, GetAuP2PEnabled(testing::_)).WillRepeatedly(
@@ -123,8 +127,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
unique_ptr<policy::MockDevicePolicy> device_policy(
new policy::MockDevicePolicy());
EXPECT_CALL(*device_policy, GetAuP2PEnabled(testing::_)).WillRepeatedly(
@@ -152,8 +157,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
unique_ptr<policy::MockDevicePolicy> device_policy(
new policy::MockDevicePolicy());
// We return an empty owner as this is an enterprise.
@@ -177,8 +183,9 @@
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- &prefs, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, &prefs, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
unique_ptr<policy::MockDevicePolicy> device_policy(
new policy::MockDevicePolicy());
// We return an empty owner as this is an enterprise.
@@ -196,9 +203,11 @@
}
// Check that we keep the $N newest files with the .$EXT.p2p extension.
-TEST_F(P2PManagerTest, Housekeeping) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+TEST_F(P2PManagerTest, HousekeepingCountLimit) {
+ // Specifically pass 0 for |max_file_age| to allow files of any age.
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta() /* max_file_age */));
EXPECT_EQ(manager->CountSharedFiles(), 0);
// Generate files with different timestamps matching our pattern and generate
@@ -261,6 +270,93 @@
EXPECT_EQ(manager->CountSharedFiles(), 3);
}
+// Check that we keep files with the .$EXT.p2p extension not older
+// than some specificed age (5 days, in this test).
+TEST_F(P2PManagerTest, HousekeepingAgeLimit) {
+ // We set the cutoff time to be 1 billion seconds (01:46:40 UTC on 9
+ // September 2001 - arbitrary number, but constant to avoid test
+ // flakiness) since the epoch and then we put two files before that
+ // date and three files after.
+ time_t cutoff_time = 1000000000;
+ base::TimeDelta age_limit = base::TimeDelta::FromDays(5);
+
+ // Set the clock just so files with a timestamp before |cutoff_time|
+ // will be deleted at housekeeping.
+ FakeClock fake_clock;
+ fake_clock.SetWallclockTime(base::Time::FromTimeT(cutoff_time) + age_limit);
+
+ // Specifically pass 0 for |num_files_to_keep| to allow files of any age.
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, &fake_clock, "cros_au",
+ 0 /* num_files_to_keep */, age_limit));
+ EXPECT_EQ(manager->CountSharedFiles(), 0);
+
+ // Generate files with different timestamps matching our pattern and generate
+ // other files not matching the pattern.
+ for (int n = 0; n < 5; n++) {
+ base::FilePath file = test_conf_->GetP2PDir().Append(base::StringPrintf(
+ "file_%d.cros_au.p2p", n));
+
+ // With five files and aiming for two of them to be before
+ // |cutoff_time|, we distribute it like this:
+ //
+ // -------- 0 -------- 1 -------- 2 -------- 3 -------- 4 --------
+ // |
+ // cutoff_time
+ //
+ base::Time file_date = base::Time::FromTimeT(cutoff_time) +
+ (n - 2)*base::TimeDelta::FromDays(1) + base::TimeDelta::FromHours(12);
+
+ // The touch(1) command expects input like this
+ // --date="2004-02-27 14:19:13.489392193 +0530"
+ base::Time::Exploded exploded;
+ file_date.UTCExplode(&exploded);
+ string file_date_string = base::StringPrintf(
+ "%d-%02d-%02d %02d:%02d:%02d +0000",
+ exploded.year, exploded.month, exploded.day_of_month,
+ exploded.hour, exploded.minute, exploded.second);
+
+ // Sanity check that we generated the correct string.
+ base::Time parsed_time;
+ EXPECT_TRUE(base::Time::FromUTCString(file_date_string.c_str(),
+ &parsed_time));
+ EXPECT_EQ(parsed_time, file_date);
+
+ EXPECT_EQ(0, System(base::StringPrintf("touch --date=\"%s\" %s",
+ file_date_string.c_str(),
+ file.value().c_str())));
+
+ EXPECT_EQ(0, System(base::StringPrintf(
+ "touch --date=\"%s\" %s/file_%d.OTHER.p2p",
+ file_date_string.c_str(),
+ test_conf_->GetP2PDir().value().c_str(), n)));
+ }
+ // CountSharedFiles() only counts 'cros_au' files.
+ EXPECT_EQ(manager->CountSharedFiles(), 5);
+
+ EXPECT_TRUE(manager->PerformHousekeeping());
+
+ // At this point - after HouseKeeping - we should only have
+ // eight files left.
+ for (int n = 0; n < 5; n++) {
+ string file_name;
+ bool expect;
+
+ expect = (n >= 2);
+ file_name = base::StringPrintf(
+ "%s/file_%d.cros_au.p2p",
+ test_conf_->GetP2PDir().value().c_str(), n);
+ EXPECT_EQ(!!g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS), expect);
+
+ file_name = base::StringPrintf(
+ "%s/file_%d.OTHER.p2p",
+ test_conf_->GetP2PDir().value().c_str(), n);
+ EXPECT_TRUE(g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS));
+ }
+ // CountSharedFiles() only counts 'cros_au' files.
+ EXPECT_EQ(manager->CountSharedFiles(), 3);
+}
+
static bool CheckP2PFile(const string& p2p_dir, const string& file_name,
ssize_t expected_size, ssize_t expected_size_xattr) {
string path = p2p_dir + "/" + file_name;
@@ -352,8 +448,9 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
EXPECT_TRUE(manager->FileShare("foo", 10 * 1000 * 1000));
EXPECT_EQ(manager->FileGetPath("foo"),
test_conf_->GetP2PDir().Append("foo.cros_au.p2p.tmp"));
@@ -375,8 +472,9 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
// First, check that it's not visible.
manager->FileShare("foo", 10*1000*1000);
EXPECT_EQ(manager->FileGetPath("foo"),
@@ -402,8 +500,9 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
bool visible;
// Check that errors are returned if the file does not exist
@@ -443,8 +542,9 @@
// will have to do. E.g. we essentially simulate the various
// behaviours of initctl(8) that we rely on.
TEST_F(P2PManagerTest, StartP2P) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
// Check that we can start the service
test_conf_->SetInitctlStartCommandLine("true");
@@ -461,8 +561,9 @@
// Same comment as for StartP2P
TEST_F(P2PManagerTest, StopP2P) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
// Check that we can start the service
test_conf_->SetInitctlStopCommandLine("true");
@@ -487,8 +588,9 @@
// Like StartP2P, we're mocking the different results that p2p-client
// can return. It's not pretty but it works.
TEST_F(P2PManagerTest, LookupURL) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
- nullptr, "cros_au", 3));
+ unique_ptr<P2PManager> manager(P2PManager::Construct(
+ test_conf_, nullptr, nullptr, "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
GMainLoop *loop = g_main_loop_new(nullptr, FALSE);
// Emulate p2p-client returning valid URL with "fooX", 42 and "cros_au"
diff --git a/real_system_state.cc b/real_system_state.cc
index faac5cd..d61e8c3 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -41,8 +41,9 @@
system_rebooted_ = true;
}
- p2p_manager_.reset(P2PManager::Construct(nullptr, &prefs_, "cros_au",
- kMaxP2PFilesToKeep));
+ p2p_manager_.reset(P2PManager::Construct(
+ nullptr, &prefs_, &clock_, "cros_au", kMaxP2PFilesToKeep,
+ base::TimeDelta::FromDays(kMaxP2PFileAgeDays)));
// Initialize the Update Manager using the default state factory.
chromeos_update_manager::State* um_state =