arc: arc-data-snapshotd: TakeSnapshot impl
Add implementation of TakeSnapshot method with unit tests.
Extend file_utils by signature and verification operations of snapshot
directory.
BUG=b:160387490
TEST=unit tests
Cq-Depend: chromium:2424223
Change-Id: Ibc3a99063768a1234b51f36e1d1c5e2ca04a71c3
(cherry picked from commit e5307d0b8d0ca023630e596b0f22830c39059235)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2403541
Tested-by: Polina Bondarenko <pbond@chromium.org>
Auto-Submit: Polina Bondarenko <pbond@chromium.org>
Commit-Queue: Polina Bondarenko <pbond@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Oleksandr Peletskyi <peletskyi@google.com>
diff --git a/arc/data-snapshotd/BUILD.gn b/arc/data-snapshotd/BUILD.gn
index 0fbe3cf..31f3a22 100644
--- a/arc/data-snapshotd/BUILD.gn
+++ b/arc/data-snapshotd/BUILD.gn
@@ -19,6 +19,7 @@
"libbootlockbox-client",
"libbrillo",
"libchrome-${libbase_ver}",
+ "libselinux",
"openssl",
"protobuf",
"system_api",
@@ -38,7 +39,10 @@
source_set("libarc-data-snapshotd") {
configs += [ ":common_pkg_deps" ]
- sources = [ "dbus_adaptor.cc" ]
+ sources = [
+ "dbus_adaptor.cc",
+ "file_utils.cc",
+ ]
deps = [ ":arc-data-snapshotd-protos" ]
}
@@ -67,8 +71,6 @@
"libbrillo",
"libchrome-${libbase_ver}",
"libchrome-test-${libbase_ver}",
- "libselinux",
- "openssl",
]
}
executable("arc-data-snapshotd_test") {
@@ -77,14 +79,10 @@
"//common-mk:test",
]
deps = [
- ":arc-data-snapshotd-protos",
":data_snapshotd_adaptors",
":libarc-data-snapshotd",
"//common-mk/testrunner:testrunner",
]
- sources = [
- "dbus_adaptor_test.cc",
- "file_utils.cc",
- ]
+ sources = [ "dbus_adaptor_test.cc" ]
}
}
diff --git a/arc/data-snapshotd/dbus_adaptor.cc b/arc/data-snapshotd/dbus_adaptor.cc
index 3c3581a..c6d16cf 100644
--- a/arc/data-snapshotd/dbus_adaptor.cc
+++ b/arc/data-snapshotd/dbus_adaptor.cc
@@ -4,15 +4,19 @@
#include "arc/data-snapshotd/dbus_adaptor.h"
-#include <string>
#include <utility>
+#include <vector>
+#include <base/bind.h>
+#include <base/callback_helpers.h>
#include <base/files/file_util.h>
#include <base/logging.h>
+#include <base/macros.h>
#include <base/memory/ptr_util.h>
+#include <brillo/data_encoding.h>
+#include <brillo/cryptohome.h>
#include <crypto/scoped_openssl_types.h>
#include <crypto/rsa_private_key.h>
-#include <openssl/sha.h>
#include "arc/data-snapshotd/file_utils.h"
#include "bootlockbox-client/bootlockbox/boot_lockbox_client.h"
@@ -27,15 +31,19 @@
"/mnt/stateful_partition/unencrypted/arc-data-snapshot/";
constexpr char kLastSnapshotPath[] = "last";
constexpr char kPreviousSnapshotPath[] = "previous";
+constexpr char kHomeRootDirectory[] = "/home/root";
} // namespace
// BootLockbox snapshot keys:
const char kLastSnapshotPublicKey[] = "snapshot_public_key_last";
const char kPreviousSnapshotPublicKey[] = "snapshot_public_key_previous";
+// Android data directory name:
+const char kAndroidDataDirectory[] = "android-data";
DBusAdaptor::DBusAdaptor()
: DBusAdaptor(base::FilePath(kCommonSnapshotPath),
+ base::FilePath(kHomeRootDirectory),
cryptohome::BootLockboxClient::CreateBootLockboxClient()) {}
DBusAdaptor::~DBusAdaptor() = default;
@@ -43,9 +51,10 @@
// static
std::unique_ptr<DBusAdaptor> DBusAdaptor::CreateForTesting(
const base::FilePath& snapshot_directory,
+ const base::FilePath& home_root_directory,
std::unique_ptr<cryptohome::BootLockboxClient> boot_lockbox_client) {
- return base::WrapUnique(
- new DBusAdaptor(snapshot_directory, std::move(boot_lockbox_client)));
+ return base::WrapUnique(new DBusAdaptor(
+ snapshot_directory, home_root_directory, std::move(boot_lockbox_client)));
}
void DBusAdaptor::RegisterAsync(
@@ -59,7 +68,7 @@
true /* failure_is_fatal */));
}
-bool DBusAdaptor::GenerateKeyPair(brillo::ErrorPtr* error) {
+bool DBusAdaptor::GenerateKeyPair() {
// TODO(b/160387490): Implement showing a spinner screen.
std::string last_public_key_digest;
// Try to move last snapshot to previous for consistency.
@@ -69,7 +78,7 @@
!last_public_key_digest.empty()) {
if (boot_lockbox_client_->Store(kPreviousSnapshotPublicKey,
last_public_key_digest) &&
- ClearSnapshot(error, false /* last */) &&
+ ClearSnapshot(false /* last */) &&
base::Move(last_snapshot_directory_, previous_snapshot_directory_)) {
boot_lockbox_client_->Store(kLastSnapshotPublicKey, "");
} else {
@@ -77,7 +86,7 @@
}
}
// Clear last snapshot - a new one will be created soon.
- ClearSnapshot(error, true /* last */);
+ ClearSnapshot(true /* last */);
// Generate a key pair.
public_key_info_.clear();
@@ -93,16 +102,8 @@
}
// Store a new public key digest.
- std::vector<uint8_t> digest;
- digest.resize(SHA256_DIGEST_LENGTH);
- if (!SHA256((const unsigned char*)public_key_info_.data(),
- public_key_info_.size(), digest.data()) ||
- digest.empty()) {
- LOG(ERROR) << "Failed to calculate digest of public key.";
- return false;
- }
- if (!boot_lockbox_client_->Store(kLastSnapshotPublicKey,
- std::string(digest.begin(), digest.end()))) {
+ std::string encoded_digest = CalculateEncodedSha256Digest(public_key_info_);
+ if (!boot_lockbox_client_->Store(kLastSnapshotPublicKey, encoded_digest)) {
LOG(ERROR) << "Failed to store a public key in BootLockbox.";
return false;
}
@@ -115,7 +116,59 @@
return true;
}
-bool DBusAdaptor::ClearSnapshot(brillo::ErrorPtr* error, bool last) {
+bool DBusAdaptor::TakeSnapshot(const std::string& account_id) {
+ if (!private_key_ || public_key_info_.empty()) {
+ LOG(ERROR) << "Private or public key does not exist.";
+ return false;
+ }
+ if (base::DirectoryExists(last_snapshot_directory_)) {
+ LOG(ERROR) << "Snapshot directory already exists. Should be cleared first.";
+ return false;
+ }
+
+ std::string userhash = brillo::cryptohome::home::SanitizeUserName(account_id);
+ base::FilePath android_data_dir =
+ home_root_directory_.Append(userhash).Append(kAndroidDataDirectory);
+ if (!base::DirectoryExists(android_data_dir)) {
+ LOG(ERROR) << "Android data directory does not exist for user "
+ << account_id;
+ return false;
+ }
+ if (base::IsLink(android_data_dir)) {
+ LOG(ERROR) << android_data_dir.value()
+ << " is a symbolic link, not snapshotting.";
+ return false;
+ }
+
+ if (!base::CopyDirectory(android_data_dir, last_snapshot_directory_,
+ true /* recursive */)) {
+ LOG(ERROR) << "Failed to copy snapshot directory from "
+ << android_data_dir.value() << " to "
+ << last_snapshot_directory_.value();
+ return false;
+ }
+ // This callback will be executed or released before the end of this function.
+ base::ScopedClosureRunner snapshot_clearer(
+ base::BindOnce(base::IgnoreResult(&DBusAdaptor::ClearSnapshot),
+ base::Unretained(this), true /* last */));
+ if (!StorePublicKey(last_snapshot_directory_, public_key_info_))
+ return false;
+ if (!StoreUserhash(last_snapshot_directory_, userhash))
+ return false;
+ if (!SignAndStoreHash(last_snapshot_directory_, private_key_.get(),
+ inode_verification_enabled_)) {
+ return false;
+ }
+ // Snapshot saved correctly, release closure without running it.
+ ignore_result(snapshot_clearer.Release());
+
+ // Dispose keys.
+ private_key_.reset();
+ public_key_info_.clear();
+ return true;
+}
+
+bool DBusAdaptor::ClearSnapshot(bool last) {
base::FilePath dir(last ? last_snapshot_directory_
: previous_snapshot_directory_);
if (!base::DirectoryExists(dir)) {
@@ -131,11 +184,13 @@
DBusAdaptor::DBusAdaptor(
const base::FilePath& snapshot_directory,
+ const base::FilePath& home_root_directory,
std::unique_ptr<cryptohome::BootLockboxClient> boot_lockbox_client)
: org::chromium::ArcDataSnapshotdAdaptor(this),
last_snapshot_directory_(snapshot_directory.Append(kLastSnapshotPath)),
previous_snapshot_directory_(
snapshot_directory.Append(kPreviousSnapshotPath)),
+ home_root_directory_(home_root_directory),
boot_lockbox_client_(std::move(boot_lockbox_client)) {
DCHECK(boot_lockbox_client_);
}
diff --git a/arc/data-snapshotd/dbus_adaptor.h b/arc/data-snapshotd/dbus_adaptor.h
index 409c053..1a7c016 100644
--- a/arc/data-snapshotd/dbus_adaptor.h
+++ b/arc/data-snapshotd/dbus_adaptor.h
@@ -6,6 +6,7 @@
#define ARC_DATA_SNAPSHOTD_DBUS_ADAPTOR_H_
#include <memory>
+#include <string>
#include <vector>
#include <base/files/file_path.h>
@@ -34,6 +35,8 @@
// BootLockbox snapshot keys:
extern const char kLastSnapshotPublicKey[];
extern const char kPreviousSnapshotPublicKey[];
+// Android data directory name:
+extern const char kAndroidDataDirectory[];
// Implements the "org.chromium.ArcDataSnapshotdInterface" D-Bus interface
// exposed by the arc-data-snapshotd daemon (see constants for the API methods
@@ -48,6 +51,7 @@
static std::unique_ptr<DBusAdaptor> CreateForTesting(
const base::FilePath& snapshot_directory,
+ const base::FilePath& home_root_directory,
std::unique_ptr<cryptohome::BootLockboxClient> boot_lockbox_client);
// Registers the D-Bus object that the arc-data-snapshotd daemon exposes and
@@ -57,8 +61,9 @@
// Implementation of the "org.chromium.ArcDataSnapshotdInterface" D-Bus
// interface:
- bool GenerateKeyPair(brillo::ErrorPtr* error) override;
- bool ClearSnapshot(brillo::ErrorPtr* error, bool last) override;
+ bool GenerateKeyPair() override;
+ bool ClearSnapshot(bool last) override;
+ bool TakeSnapshot(const std::string& account_id) override;
const base::FilePath& get_last_snapshot_directory() const {
return last_snapshot_directory_;
@@ -67,9 +72,23 @@
return previous_snapshot_directory_;
}
+ // Use this method only for testing.
+ // Inode verification of snapshot directory is enabled in production by
+ // default.
+ // In production the integrity of the persisting snapshot directory is
+ // verified, inode values should stay the same.
+ //
+ // Using this method, the inode verification for snapshot directories can be
+ // disabled for testing. It is needed to ensure the integrity of snapshot
+ // directories after copying it (inodes change).
+ void set_inode_verification_enabled_for_testing(bool enabled) {
+ inode_verification_enabled_ = enabled;
+ }
+
private:
DBusAdaptor(
const base::FilePath& snapshot_directory,
+ const base::FilePath& home_root_directory,
std::unique_ptr<cryptohome::BootLockboxClient> boot_lockbox_client);
// Manages the D-Bus interfaces exposed by the arc-data-snapshotd daemon.
@@ -78,6 +97,8 @@
// Snapshot directory paths:
const base::FilePath last_snapshot_directory_;
const base::FilePath previous_snapshot_directory_;
+ // Home root directory.
+ const base::FilePath home_root_directory_;
// Manages the communication with BootLockbox.
std::unique_ptr<cryptohome::BootLockboxClient> boot_lockbox_client_;
@@ -88,6 +109,9 @@
// GenerateKeyPair. The key is valid only when |private_key_| is set.
// Should be stored on disk once |private_key_| is disposed.
std::vector<uint8_t> public_key_info_;
+ // Inode verification of snapshot directories is enabled in production ny
+ // default.
+ bool inode_verification_enabled_ = true;
};
} // namespace data_snapshotd
diff --git a/arc/data-snapshotd/dbus_adaptor_test.cc b/arc/data-snapshotd/dbus_adaptor_test.cc
index a379661..1730394 100644
--- a/arc/data-snapshotd/dbus_adaptor_test.cc
+++ b/arc/data-snapshotd/dbus_adaptor_test.cc
@@ -9,6 +9,8 @@
#include <base/files/file_path.h>
#include <base/files/file_util.h>
#include <base/files/scoped_temp_dir.h>
+#include <brillo/cryptohome.h>
+#include <brillo/data_encoding.h>
#include <dbus/bus.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -34,10 +36,11 @@
namespace {
constexpr char kRandomDir[] = "data";
-constexpr char kRandomFile[] = "hash";
+constexpr char kRandomFile[] = "random file";
constexpr char kContent[] = "content";
constexpr char kFakeLastSnapshotPublicKey[] = "fake_public_key";
-
+constexpr char kFakeAccountID[] = "fake_account_id";
+constexpr char kFakeAccountID2[] = "fake_aacount_id_2";
MATCHER_P(nEq, expected, "") {
return expected != arg;
}
@@ -63,20 +66,36 @@
class DBusAdaptorTest : public testing::Test {
public:
DBusAdaptorTest() : bus_(new dbus::Bus{dbus::Bus::Options{}}) {
+ brillo::cryptohome::home::SetSystemSalt(&salt_);
EXPECT_TRUE(root_tempdir_.CreateUniqueTempDir());
+
auto boot_lockbox_client =
std::make_unique<testing::StrictMock<MockBootLockboxClient>>(bus_);
boot_lockbox_client_ = boot_lockbox_client.get();
dbus_adaptor_ = DBusAdaptor::CreateForTesting(
- root_tempdir_.GetPath(), std::move(boot_lockbox_client));
+ root_tempdir_.GetPath(), root_tempdir_.GetPath(),
+ std::move(boot_lockbox_client));
+
+ user_directory_ = root_tempdir_.GetPath().Append(hash(kFakeAccountID));
+ EXPECT_TRUE(base::CreateDirectory(user_directory_));
}
DBusAdaptor* dbus_adaptor() { return dbus_adaptor_.get(); }
- const base::FilePath& last_snapshot_dir() {
+ const base::FilePath& last_snapshot_dir() const {
return dbus_adaptor_->get_last_snapshot_directory();
}
- const base::FilePath& previous_snapshot_dir() {
+ const base::FilePath& previous_snapshot_dir() const {
return dbus_adaptor_->get_previous_snapshot_directory();
}
+ base::FilePath android_data_dir() const {
+ return user_directory_.Append(kAndroidDataDirectory);
+ }
+ base::FilePath random_dir() const {
+ return root_tempdir_.GetPath().Append(kRandomDir);
+ }
+ std::string hash(const std::string& account_id) const {
+ return brillo::cryptohome::home::SanitizeUserName(account_id);
+ }
+
// Creates |dir| and fills in with random content.
void CreateDir(const base::FilePath& dir) {
EXPECT_TRUE(base::CreateDirectory(dir));
@@ -88,10 +107,12 @@
MockBootLockboxClient* boot_lockbox_client() { return boot_lockbox_client_; }
private:
+ std::string salt_ = "salt";
scoped_refptr<dbus::Bus> bus_;
MockBootLockboxClient* boot_lockbox_client_;
std::unique_ptr<DBusAdaptor> dbus_adaptor_;
base::ScopedTempDir root_tempdir_;
+ base::FilePath user_directory_;
};
TEST_F(DBusAdaptorTest, ClearSnapshotBasic) {
@@ -101,15 +122,15 @@
CreateDir(previous_snapshot_dir());
EXPECT_TRUE(base::DirectoryExists(previous_snapshot_dir()));
- EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(nullptr, false /* last */));
+ EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(false /* last */));
EXPECT_FALSE(base::DirectoryExists(previous_snapshot_dir()));
- EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(nullptr, false /* last */));
+ EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(false /* last */));
- EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(nullptr, true /* last */));
+ EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(true /* last */));
EXPECT_FALSE(base::DirectoryExists(last_snapshot_dir()));
- EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(nullptr, true /* last */));
+ EXPECT_TRUE(dbus_adaptor()->ClearSnapshot(true /* last */));
}
// Test successful basic flow with no pre-existing snapshots.
@@ -117,7 +138,7 @@
EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
.WillOnce(Return(true));
EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
- EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
}
// Test successful basic flow with pre-existing snapshots.
@@ -138,7 +159,6 @@
CalculateDirectoryCryptographicHash(previous_dir);
EXPECT_FALSE(previous_hash.empty());
- EXPECT_NE(previous_dir.DebugString(), last_dir.DebugString());
EXPECT_NE(last_hash, previous_hash);
}
@@ -158,7 +178,7 @@
.WillOnce(Return(true));
EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
- EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
{
SnapshotDirectory previous_dir;
EXPECT_TRUE(ReadSnapshotDirectory(previous_snapshot_dir(), &previous_dir));
@@ -167,7 +187,6 @@
EXPECT_FALSE(previous_hash.empty());
// Check that the last snapshot has been moved to previous snapshot dir.
EXPECT_EQ(previous_hash, last_hash);
- EXPECT_EQ(previous_dir.DebugString(), last_dir.DebugString());
}
}
@@ -177,15 +196,14 @@
// Attempt failure: last snapshot dir => previous snapshot dir.
EXPECT_CALL(*boot_lockbox_client(), Read(Eq(kLastSnapshotPublicKey), _))
- .WillOnce(Invoke(
- [](const std::string& key, std::string* value) { return false; }));
+ .WillOnce(Return(false));
EXPECT_CALL(*boot_lockbox_client(),
Store(Eq(kLastSnapshotPublicKey), nEq("")))
.WillOnce(Return(true));
EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
// Generating key pair should be still successful.
- EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
}
// Test successful flow with pre-existing last snapshot empty key.
@@ -204,7 +222,7 @@
EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
// Generating key pair should be still successful.
- EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
}
// Test success flow with pre-existing snapshots and moving error.
@@ -228,7 +246,7 @@
// Generating key pair should be still successful, because the last snapshot
// will be re-generated anyway.
- EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
}
// Test failure flow when storing freshly generated public key is failed.
@@ -238,7 +256,7 @@
Store(Eq(kLastSnapshotPublicKey), nEq("")))
.WillOnce(Return(false));
- EXPECT_FALSE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_FALSE(dbus_adaptor()->GenerateKeyPair());
}
// Test failure flow when finalizing BootLockbox is attempted.
@@ -249,7 +267,143 @@
// Fail once the finalization of BootLockbox is attempted.
EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(false));
- EXPECT_FALSE(dbus_adaptor()->GenerateKeyPair(nullptr));
+ EXPECT_FALSE(dbus_adaptor()->GenerateKeyPair());
+}
+
+// Test failure flow when the keys were not generated.
+TEST_F(DBusAdaptorTest, TakeSnapshotNoPrivateKeyFailure) {
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test failure flow when the last snapshot directory already exists.
+TEST_F(DBusAdaptorTest, TakeSnapshotLastSnapshotExistFailure) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+
+ CreateDir(last_snapshot_dir());
+ EXPECT_TRUE(base::DirectoryExists(last_snapshot_dir()));
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test failure flow when android-data directory does not exist.
+TEST_F(DBusAdaptorTest, TakeSnapshotAndroidDataDirNotExist) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+ EXPECT_FALSE(base::DirectoryExists(android_data_dir()));
+
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test failure flow when android-data is file.
+TEST_F(DBusAdaptorTest, TakeSnapshotAndroidDataNotDirFile) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+ // Create a file instead of android-data directory.
+ EXPECT_TRUE(base::WriteFile(android_data_dir(), kContent, strlen(kContent)));
+ EXPECT_TRUE(base::PathExists(android_data_dir()));
+ EXPECT_FALSE(base::DirectoryExists(android_data_dir()));
+
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test failure flow when android-data is a sym link.
+TEST_F(DBusAdaptorTest, TakeSnapshotAndroidDataSymLink) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+ // Create a symlink.
+ CreateDir(random_dir());
+ EXPECT_TRUE(base::CreateSymbolicLink(random_dir(), android_data_dir()));
+ EXPECT_TRUE(base::IsLink(android_data_dir()));
+ EXPECT_TRUE(base::DirectoryExists(android_data_dir()));
+
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test failure flow when android-data is a fifo.
+TEST_F(DBusAdaptorTest, TakeSnapshotAndroidDataFiFo) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+ // Create a fifo android-data.
+ mkfifo(android_data_dir().value().c_str(), 0666);
+ EXPECT_TRUE(base::PathExists(android_data_dir()));
+ EXPECT_FALSE(base::DirectoryExists(android_data_dir()));
+
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+}
+
+// Test basic TakeSnapshot success flow.
+TEST_F(DBusAdaptorTest, TakeSnapshotSuccess) {
+ // In this test the copied snapshot directory is verified against the origin
+ // android data directory. Inodes verification must be disabled, because the
+ // inode values are changed after copying.
+ // In production, it is not the case, because the directorys' integrity is
+ // verified against itself and inode values should persist.
+ dbus_adaptor()->set_inode_verification_enabled_for_testing(
+ false /* enabled */);
+ std::string expected_public_key_digest;
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Invoke([&expected_public_key_digest](
+ const std::string& key, const std::string& digest) {
+ expected_public_key_digest = digest;
+ return true;
+ }));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+
+ CreateDir(android_data_dir());
+ EXPECT_TRUE(base::DirectoryExists(android_data_dir()));
+ // Store userhash to ensure that userhash stays the same.
+ EXPECT_TRUE(StoreUserhash(android_data_dir(), hash(kFakeAccountID)));
+ SnapshotDirectory android_dir;
+ EXPECT_TRUE(ReadSnapshotDirectory(android_data_dir(), &android_dir,
+ false /* inode_verification_enabled */));
+ std::vector<uint8_t> android_data_hash =
+ CalculateDirectoryCryptographicHash(android_dir);
+ EXPECT_FALSE(android_data_hash.empty());
+
+ EXPECT_TRUE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+ EXPECT_TRUE(base::DirectoryExists(last_snapshot_dir()));
+ SnapshotDirectory last_dir;
+ EXPECT_TRUE(ReadSnapshotDirectory(last_snapshot_dir(), &last_dir,
+ false /* inode_verification_enabled */));
+ std::vector<uint8_t> last_snapshot_hash =
+ CalculateDirectoryCryptographicHash(last_dir);
+ EXPECT_FALSE(last_snapshot_hash.empty());
+ EXPECT_EQ(android_data_hash, last_snapshot_hash);
+
+ // Verification for another account ID should fail.
+ EXPECT_FALSE(VerifyHash(last_snapshot_dir(), hash(kFakeAccountID2),
+ expected_public_key_digest,
+ false /* inode_verification_enabled */));
+ EXPECT_TRUE(VerifyHash(last_snapshot_dir(), hash(kFakeAccountID),
+ expected_public_key_digest,
+ false /* inode_verification_enabled */));
+ dbus_adaptor()->set_inode_verification_enabled_for_testing(
+ true /* enabled */);
+}
+
+// Test failure flow if TakeSnapshot is invoked twice.
+TEST_F(DBusAdaptorTest, TakeSnapshotDouble) {
+ EXPECT_CALL(*boot_lockbox_client(), Store(Eq(kLastSnapshotPublicKey), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*boot_lockbox_client(), Finalize()).WillOnce(Return(true));
+ EXPECT_TRUE(dbus_adaptor()->GenerateKeyPair());
+
+ CreateDir(android_data_dir());
+ EXPECT_TRUE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
+
+ CreateDir(android_data_dir());
+ EXPECT_FALSE(dbus_adaptor()->TakeSnapshot(kFakeAccountID));
}
} // namespace data_snapshotd
diff --git a/arc/data-snapshotd/dbus_bindings/org.chromium.ArcDataSnapshotd.xml b/arc/data-snapshotd/dbus_bindings/org.chromium.ArcDataSnapshotd.xml
index 3fb74ac..03a23b9 100644
--- a/arc/data-snapshotd/dbus_bindings/org.chromium.ArcDataSnapshotd.xml
+++ b/arc/data-snapshotd/dbus_bindings/org.chromium.ArcDataSnapshotd.xml
@@ -12,17 +12,47 @@
Public key is stored in BootlockBox.
The method should be called before the start of user session.
</tp:docstring>
+ <annotation name="org.chromium.DBus.Method.Kind" value="simple"/>
+ <arg type="b" name="success" direction="out">
+ <tp:docstring>
+ Result of the operation.
+ </tp:docstring>
+ </arg>
</method>
<method name="ClearSnapshot">
<tp:docstring>
Clears snapshot.
</tp:docstring>
+ <annotation name="org.chromium.DBus.Method.Kind" value="simple"/>
<arg type="b" name="last" direction="in">
<tp:docstring>
Specifies a snapshot to clear.
If true, the last snapshot directory is cleared.
</tp:docstring>
</arg>
+ <arg type="b" name="success" direction="out">
+ <tp:docstring>
+ Result of the operation.
+ </tp:docstring>
+ </arg>
+ </method>
+ <method name="TakeSnapshot">
+ <tp:docstring>
+ Saves and signs a snapshot.
+ </tp:docstring>
+ <annotation name="org.chromium.DBus.Method.Kind" value="simple"/>
+ <arg type="s" name="account_id" direction="in">
+ <tp:docstring>
+ Specifies an account ID of a user, that a snapshot is taken for.
+ The caller (Chrome) is responsible for executing this method only for
+ managed guest session account.
+ </tp:docstring>
+ </arg>
+ <arg type="b" name="success" direction="out">
+ <tp:docstring>
+ Result of the operation.
+ </tp:docstring>
+ </arg>
</method>
</interface>
</node>
diff --git a/arc/data-snapshotd/file_utils.cc b/arc/data-snapshotd/file_utils.cc
index f34387b..9b6a231 100644
--- a/arc/data-snapshotd/file_utils.cc
+++ b/arc/data-snapshotd/file_utils.cc
@@ -5,7 +5,6 @@
#include "arc/data-snapshotd/file_utils.h"
#include <algorithm>
-#include <string>
#include <utility>
#if USE_SELINUX
@@ -19,13 +18,26 @@
#include <base/files/file_enumerator.h>
#include <base/files/file_util.h>
#include <base/logging.h>
+#include <brillo/data_encoding.h>
+#include <crypto/rsa_private_key.h>
+#include <crypto/signature_creator.h>
+#include <crypto/signature_verifier.h>
#include <openssl/sha.h>
namespace arc {
namespace data_snapshotd {
+namespace {
+
+constexpr char kHashFile[] = "hash";
+constexpr char kPublicKeyFile[] = "public_key_info";
+constexpr char kUserhashFile[] = "userhash";
+
+} // namespace
+
bool ReadSnapshotDirectory(const base::FilePath& dir,
- SnapshotDirectory* snapshot_directory) {
+ SnapshotDirectory* snapshot_directory,
+ bool inode_verification_enabled) {
if (!snapshot_directory) {
LOG(ERROR) << "snapshot_directory is nullptr";
return false;
@@ -38,10 +50,13 @@
std::vector<SnapshotFile> snapshot_files;
for (auto file = dir_enumerator.Next(); !file.empty();
file = dir_enumerator.Next()) {
- std::string relative_path =
- file.value().substr(dir.value().size(), std::string::npos);
+ base::FilePath relative_path;
+ if (!dir.IsParent(file) || !dir.AppendRelativePath(file, &relative_path)) {
+ LOG(ERROR) << dir.value() << " is not a parent of " << file.value();
+ return false;
+ }
SnapshotFile snapshot_file;
- snapshot_file.set_name(std::move(relative_path));
+ snapshot_file.set_name(relative_path.value());
std::string contents;
if (!dir_enumerator.GetInfo().IsDirectory() &&
!base::ReadFileToString(file, &contents)) {
@@ -69,7 +84,7 @@
if (con != nullptr) {
free(con);
}
-#endif
+#endif // USE_SELINUX
struct stat stat_buf;
if (lstat(file.value().c_str(), &stat_buf)) {
@@ -77,7 +92,8 @@
return false;
}
Stat* stat_value = snapshot_file.mutable_stat();
- stat_value->set_ino(stat_buf.st_ino);
+ if (inode_verification_enabled)
+ stat_value->set_ino(stat_buf.st_ino);
stat_value->set_mode(stat_buf.st_mode);
stat_value->set_uid(stat_buf.st_uid);
stat_value->set_gid(stat_buf.st_gid);
@@ -87,9 +103,14 @@
}
std::sort(snapshot_files.begin(), snapshot_files.end(),
[](const SnapshotFile& a, const SnapshotFile& b) {
+ // Sort lexicographically by name.
return a.name() < b.name();
});
for (auto file : snapshot_files) {
+ if (file.name() == kHashFile)
+ continue;
+ if (file.name() == kPublicKeyFile)
+ continue;
*snapshot_directory->mutable_files()->Add() = file;
}
return true;
@@ -105,12 +126,180 @@
}
hash.resize(SHA256_DIGEST_LENGTH);
if (!SHA256((const unsigned char*)serialized.data(), serialized.size(),
- hash.data())) {
+ hash.data()) ||
+ hash.empty()) {
LOG(ERROR) << "Failed to calculate digest of serialized SnapshotDirectory.";
return std::vector<uint8_t>();
}
return hash;
}
+bool StorePublicKey(const base::FilePath& dir,
+ const std::vector<uint8_t>& public_key_info) {
+ if (public_key_info.empty()) {
+ LOG(ERROR) << "Empty public key info";
+ return false;
+ }
+ if (!base::DirectoryExists(dir)) {
+ LOG(ERROR) << "Directory " << dir.value() << " does not exist.";
+ return false;
+ }
+ std::string encoded_public_key = brillo::data_encoding::Base64Encode(
+ public_key_info.data(), public_key_info.size());
+ if (!base::WriteFile(dir.Append(kPublicKeyFile), encoded_public_key.data(),
+ encoded_public_key.length())) {
+ LOG(ERROR) << "Failed to write public key info to file "
+ << dir.Append(kPublicKeyFile);
+ return false;
+ }
+ return true;
+}
+
+bool StoreUserhash(const base::FilePath& dir, const std::string& userhash) {
+ if (userhash.empty()) {
+ LOG(ERROR) << "Empty user hash";
+ return false;
+ }
+ if (!base::DirectoryExists(dir)) {
+ LOG(ERROR) << "Directory " << dir.value() << " does not exist.";
+ return false;
+ }
+ if (!base::WriteFile(dir.Append(kUserhashFile), userhash.c_str(),
+ userhash.size())) {
+ LOG(ERROR) << "Failed to write userhash to file "
+ << dir.Append(kUserhashFile);
+ return false;
+ }
+ return true;
+}
+
+bool SignAndStoreHash(const base::FilePath& dir,
+ crypto::RSAPrivateKey* private_key,
+ bool inode_verification_enabled) {
+ if (!private_key) {
+ LOG(ERROR) << "nullptr private key";
+ return false;
+ }
+ if (!base::DirectoryExists(dir)) {
+ LOG(ERROR) << "Directory " << dir.value() << " does not exist.";
+ return false;
+ }
+ SnapshotDirectory snapshot_dir;
+ if (!ReadSnapshotDirectory(dir, &snapshot_dir, inode_verification_enabled)) {
+ return false;
+ }
+ std::vector<uint8_t> hash = CalculateDirectoryCryptographicHash(snapshot_dir);
+ if (hash.empty()) {
+ return false;
+ }
+ std::vector<uint8_t> signature;
+ if (!crypto::SignatureCreator::Sign(
+ private_key, crypto::SignatureCreator::HashAlgorithm::SHA256,
+ hash.data(), hash.size(), &signature)) {
+ LOG(ERROR) << "Failed to sign directory contents: " << dir.value();
+ return false;
+ }
+ std::string encoded_signature =
+ brillo::data_encoding::Base64Encode(signature.data(), signature.size());
+ if (!base::WriteFile(dir.Append(kHashFile), encoded_signature.data(),
+ encoded_signature.length())) {
+ LOG(ERROR) << "Failed to write a signature to file "
+ << dir.Append(kHashFile);
+ return false;
+ }
+ return true;
+}
+
+bool VerifyHash(const base::FilePath& dir,
+ const std::string& expected_userhash,
+ const std::string& expected_public_key_digest,
+ bool inode_verification_enabled) {
+ if (!base::DirectoryExists(dir)) {
+ LOG(ERROR) << "Directory " << dir.value() << " does not exist.";
+ return false;
+ }
+ if (expected_public_key_digest.empty()) {
+ LOG(ERROR) << "Public key digest is empty.";
+ return false;
+ }
+ std::string userhash;
+ if (!base::ReadFileToString(dir.Append(kUserhashFile), &userhash)) {
+ LOG(ERROR) << "Failed to read userhash for file "
+ << dir.Append(kUserhashFile);
+ return false;
+ }
+ if (userhash != expected_userhash) {
+ LOG(ERROR) << "Requested to load snapshot for unsupported account.";
+ return false;
+ }
+ std::string encoded_public_key;
+ if (!base::ReadFileToString(dir.Append(kPublicKeyFile),
+ &encoded_public_key)) {
+ LOG(ERROR) << "Failed to read public key info from file "
+ << dir.Append(kPublicKeyFile);
+ return false;
+ }
+ std::vector<uint8_t> public_key;
+ if (!brillo::data_encoding::Base64Decode(encoded_public_key, &public_key)) {
+ LOG(ERROR) << "Failed to decode public key.";
+ return false;
+ }
+ std::string encoded_public_key_digest =
+ CalculateEncodedSha256Digest(public_key);
+ if (encoded_public_key_digest.empty()) {
+ return false;
+ }
+ if (encoded_public_key_digest.compare(expected_public_key_digest)) {
+ LOG(ERROR) << "Public key has been modified.";
+ return false;
+ }
+
+ std::string contents;
+ if (!base::ReadFileToString(dir.Append(kHashFile), &contents)) {
+ LOG(ERROR) << "Failed to read signed hash from file "
+ << dir.Append(kHashFile);
+ return false;
+ }
+ std::vector<uint8_t> signature;
+ if (!brillo::data_encoding::Base64Decode(contents, &signature)) {
+ LOG(ERROR) << "Failed to decode signature.";
+ return false;
+ }
+ crypto::SignatureVerifier verifier;
+ if (!verifier.VerifyInit(
+ crypto::SignatureVerifier::SignatureAlgorithm::RSA_PKCS1_SHA256,
+ signature.data(), signature.size(), public_key.data(),
+ public_key.size())) {
+ LOG(ERROR) << "Failed to initilize signature verifier.";
+ return false;
+ }
+
+ SnapshotDirectory snapshot_dir;
+ if (!ReadSnapshotDirectory(dir, &snapshot_dir, inode_verification_enabled)) {
+ return false;
+ }
+ std::string serialized;
+ if (!snapshot_dir.SerializeToString(&serialized)) {
+ LOG(ERROR) << "Failed to serialize snapshot dir";
+ return false;
+ }
+ verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(serialized.c_str()),
+ serialized.size());
+ return verifier.VerifyFinal();
+}
+
+std::string CalculateEncodedSha256Digest(const std::vector<uint8_t>& value) {
+ // Store a new public key digest.
+ std::vector<uint8_t> digest;
+ digest.resize(SHA256_DIGEST_LENGTH);
+ if (!SHA256((const unsigned char*)value.data(), value.size(),
+ digest.data()) ||
+ digest.empty()) {
+ LOG(ERROR) << "Failed to calculate digest of public key.";
+ return "";
+ }
+ return brillo::data_encoding::Base64Encode(digest.data(), digest.size());
+}
+
} // namespace data_snapshotd
} // namespace arc
diff --git a/arc/data-snapshotd/file_utils.h b/arc/data-snapshotd/file_utils.h
index 039257b..7b06644 100644
--- a/arc/data-snapshotd/file_utils.h
+++ b/arc/data-snapshotd/file_utils.h
@@ -5,26 +5,75 @@
#ifndef ARC_DATA_SNAPSHOTD_FILE_UTILS_H_
#define ARC_DATA_SNAPSHOTD_FILE_UTILS_H_
+#include <string>
#include <vector>
#include <base/files/file_path.h>
#include "proto/directory.pb.h"
+namespace crypto {
+
+class RSAPrivateKey;
+
+} // namespace crypto
+
namespace arc {
namespace data_snapshotd {
// Extracts all files and file info for all files from |dir| and fills in
// |snapshot_directory| object, that should be non-nullptr.
+// Set |inode_verification_enabled| to false only for testing to disable the
+// inode integrity check for snapshot directories.
// Returns true in case of success and false in case of any error.
bool ReadSnapshotDirectory(const base::FilePath& dir,
- SnapshotDirectory* snapshot_directory);
+ SnapshotDirectory* snapshot_directory,
+ bool inode_verification_enabled = true);
// Calculates SHA256 hash for serialized |dir|.
// In case of any error returns empty hash.
std::vector<uint8_t> CalculateDirectoryCryptographicHash(
const SnapshotDirectory& dir);
+// Encodes and stores |public_key_info| on disk in |dir|/public_key_info file.
+// |dir| is an existing directory, where a prospective snapshot signed with
+// a corresponding private key will be stored.
+// Returns false in case of any error.
+bool StorePublicKey(const base::FilePath& dir,
+ const std::vector<uint8_t>& public_key_info);
+
+// Stores |userhash| on disk in |dir|/userhash file.
+// |dir| is an existing directory, where a prospective snapshot signed with
+// a corresponding private key will be stored.
+// Returns false in case of any error.
+bool StoreUserhash(const base::FilePath& dir, const std::string& userhash);
+
+// Calculates SHA256 hash for |dir| (excluding |dir|/hash file), signs it with
+// |private_key| and stores in |dir|/hash file.
+// Set |inode_verification_enabled| to false only for testing to disable the
+// inode integrity check for snapshot directories.
+// Returns false in case of any error.
+bool SignAndStoreHash(const base::FilePath& dir,
+ crypto::RSAPrivateKey* private_key,
+ bool inode_verification_enabled);
+
+// Verifies signed hash stored in a |dir|/hash file by public key stored in a
+// |dir|/public_key_info.
+// Verifies |expected_userhash| is equal to the userhash stored in
+// |dir|/userhash.
+// Verifies integrity of |dir|/public_key_info by sha256 hash
+// |expected_public_key_digest|.
+// Set |inode_verification_enabled| to false only for testing to disable the
+// inode integrity check for snapshot directories.
+// Returns false in case of any error.
+bool VerifyHash(const base::FilePath& dir,
+ const std::string& expected_userhash,
+ const std::string& expected_public_key_digest,
+ bool inode_verification_enabled);
+
+// Calculates SHA256 hash of |value| and encodes it.
+// Returns an empty string in case of any error.
+std::string CalculateEncodedSha256Digest(const std::vector<uint8_t>& value);
} // namespace data_snapshotd
} // namespace arc
diff --git a/arc/data-snapshotd/init/arc-data-snapshotd.conf b/arc/data-snapshotd/init/arc-data-snapshotd.conf
index 83892a6..0a8c5a9 100644
--- a/arc/data-snapshotd/init/arc-data-snapshotd.conf
+++ b/arc/data-snapshotd/init/arc-data-snapshotd.conf
@@ -37,6 +37,8 @@
# -b /run/dbus: shared socket file for talking with the D-Bus daemon;
# -b /mnt/stateful_partition/unencrypted/arc-data-snapshot: arc data snapshot
# directory;
+# -b /opt/google/containers/android/rootfs/android-data: bind mounted
+# android-data directory;
# -S: apply seccomp filters.
script
logger -t "${UPSTART_JOB}" "Start arc-data-snapshotd"
@@ -48,6 +50,7 @@
-k 'tmpfs,/run,tmpfs,MS_NODEV|MS_NOSUID|MS_NOEXEC,mode=755,size=10M' \
-b /run/dbus \
-b /mnt/stateful_partition/unencrypted/arc-data-snapshot \
+ -b /opt/google/containers/android/rootfs/android-data \
-S /usr/share/policy/arc-data-snapshotd-seccomp.policy \
-- /usr/bin/arc-data-snapshotd
end script
