Deprecating tracked_directories as a Vault parameter
in favor of managing them internally in Cryptohomed
Change-Id: I66b1db4a69e64c826b3316ba7ac2705ae12e2872
BUG=chromium-os:9620
TEST=none
Review URL: http://codereview.chromium.org/6598009
diff --git a/cryptohome.cc b/cryptohome.cc
index 0a8c3e5..d8aea4a 100644
--- a/cryptohome.cc
+++ b/cryptohome.cc
@@ -72,7 +72,6 @@
static const char kForceSwitch[] = "force";
static const char kAsyncSwitch[] = "async";
static const char kCreateSwitch[] = "create";
- static const char kTrackedDirsSwitch[] = "tracked_dirs";
} // namespace switches
chromeos::Blob GetSystemSalt(const chromeos::dbus::Proxy& proxy) {
@@ -292,22 +291,6 @@
StringPrintf("Enter the password for <%s>", user.c_str()),
&password);
- const char** tracked_subdirectories = NULL;
- // Defined outside to keep the values in-scope for use in the functions
- // below
- std::vector<std::string> tracked_dirs;
- if (cl->HasSwitch(switches::kTrackedDirsSwitch)) {
- SplitString(cl->GetSwitchValueASCII(switches::kTrackedDirsSwitch), ',',
- &tracked_dirs);
- tracked_subdirectories = new const char*[tracked_dirs.size() + 1];
- int i = 0;
- for (std::vector<std::string>::const_iterator itr = tracked_dirs.begin();
- itr != tracked_dirs.end(); itr++, i++) {
- tracked_subdirectories[i] = itr->c_str();
- }
- tracked_subdirectories[i] = NULL;
- }
-
gboolean done = false;
gint mount_error = 0;
chromeos::glib::ScopedError error;
@@ -317,8 +300,8 @@
user.c_str(),
password.c_str(),
cl->HasSwitch(switches::kCreateSwitch),
- (tracked_subdirectories != NULL),
- tracked_subdirectories,
+ false,
+ NULL,
&mount_error,
&done,
&chromeos::Resetter(&error).lvalue())) {
@@ -333,8 +316,8 @@
user.c_str(),
password.c_str(),
cl->HasSwitch(switches::kCreateSwitch),
- (tracked_subdirectories != NULL),
- tracked_subdirectories,
+ false,
+ NULL,
&async_id,
&chromeos::Resetter(&error).lvalue())) {
printf("Mount call failed: %s.\n", error->message);
@@ -343,9 +326,6 @@
done = client_loop.get_return_status();
}
}
- if (tracked_subdirectories) {
- delete(tracked_subdirectories);
- }
if (!done) {
printf("Mount failed.\n");
} else {
@@ -600,13 +580,6 @@
printf(" Password rounds:\n");
printf(" %d\n", serialized.password_rounds());
}
- if (serialized.tracked_subdirectories_size()) {
- printf(" Tracked subdirectories:\n");
- for (int index = 0; index < serialized.tracked_subdirectories_size();
- index++) {
- printf(" %s\n", serialized.tracked_subdirectories(index).c_str());
- }
- }
} else if (!strcmp(switches::kActions[switches::ACTION_TPM_STATUS],
action.c_str())) {
chromeos::glib::ScopedError error;
diff --git a/cryptohome.xml b/cryptohome.xml
index 1519149..f05d52e 100644
--- a/cryptohome.xml
+++ b/cryptohome.xml
@@ -47,8 +47,8 @@
<arg type="s" name="username" direction="in" />
<arg type="s" name="key" direction="in" />
<arg type="b" name="create_if_missing" direction="in" />
- <arg type="b" name="replace_tracked_subdirectories" direction="in" />
- <arg type="as" name="tracked_subdirectories" direction="in" />
+ <arg type="b" name="deprecated_replace_tracked_subdirectories" direction="in" />
+ <arg type="as" name="deprecated_tracked_subdirectories" direction="in" />
<arg type="i" name="error" direction="out" />
<arg type="b" name="done" direction="out" />
</method>
@@ -56,8 +56,8 @@
<arg type="s" name="username" direction="in" />
<arg type="s" name="key" direction="in" />
<arg type="b" name="create_if_missing" direction="in" />
- <arg type="b" name="replace_tracked_subdirectories" direction="in" />
- <arg type="as" name="tracked_subdirectories" direction="in" />
+ <arg type="b" name="deprecated_replace_tracked_subdirectories" direction="in" />
+ <arg type="as" name="deprecated_tracked_subdirectories" direction="in" />
<arg type="i" name="async_id" direction="out" />
</method>
<method name="MountGuest">
diff --git a/make_tests.cc b/make_tests.cc
index 3490287..a94f6b8 100644
--- a/make_tests.cc
+++ b/make_tests.cc
@@ -28,29 +28,28 @@
// const char* password;
// bool create;
// bool use_old_format;
-// const char* tracked_dirs[2];
// };
const TestUserInfo kDefaultUsers[] = {
- {"testuser0@invalid.domain", "zero", true, false, {NULL}},
- {"testuser1@invalid.domain", "one", true, false, {NULL}},
- {"testuser2@invalid.domain", "two", true, false, {NULL}},
- {"testuser3@invalid.domain", "three", true, false, {NULL}},
- {"testuser4@invalid.domain", "four", true, false, {NULL}},
- {"testuser5@invalid.domain", "five", false, false, {NULL}},
- {"testuser6@invalid.domain", "six", true, false, {NULL}},
- {"testuser7@invalid.domain", "seven", true, true, {NULL}},
- {"testuser8@invalid.domain", "eight", true, false, {NULL}},
- {"testuser9@invalid.domain", "nine", true, false, {"DIR0", NULL}},
- {"testuser10@invalid.domain", "ten", true, false, {"DIR0", NULL}},
- {"testuser11@invalid.domain", "eleven", true, false, {"DIR0", NULL}},
- {"testuser12@invalid.domain", "twelve", false, false, {"DIR0", NULL}},
+ {"testuser0@invalid.domain", "zero", true, false},
+ {"testuser1@invalid.domain", "one", true, false},
+ {"testuser2@invalid.domain", "two", true, false},
+ {"testuser3@invalid.domain", "three", true, false},
+ {"testuser4@invalid.domain", "four", true, false},
+ {"testuser5@invalid.domain", "five", false, false},
+ {"testuser6@invalid.domain", "six", true, false},
+ {"testuser7@invalid.domain", "seven", true, true},
+ {"testuser8@invalid.domain", "eight", true, false},
+ {"testuser9@invalid.domain", "nine", true, false},
+ {"testuser10@invalid.domain", "ten", true, false},
+ {"testuser11@invalid.domain", "eleven", true, false},
+ {"testuser12@invalid.domain", "twelve", false, false},
};
const size_t kDefaultUserCount =
sizeof(kDefaultUsers) / sizeof(kDefaultUsers[0]);
const TestUserInfo kAlternateUsers[] = {
- {"altuser0@invalid.domain", "zero", true, false, {"DIR0", NULL}},
+ {"altuser0@invalid.domain", "zero", true, false},
{"altuser1@invalid.domain", "odin", true, false},
};
const size_t kAlternateUserCount =
@@ -102,10 +101,6 @@
UsernamePasskey up(test_users[i].username, passkey);
bool created;
Mount::MountArgs mount_args;
- if (test_users[i].tracked_dirs[0] != NULL) {
- mount_args.AssignSubdirsNullTerminatedList(
- const_cast<const char**>(test_users[i].tracked_dirs));
- }
mount.EnsureCryptohome(up, mount_args, &created);
if (test_users[i].use_old_format) {
diff --git a/make_tests.h b/make_tests.h
index f28d285..0db0e00 100644
--- a/make_tests.h
+++ b/make_tests.h
@@ -18,7 +18,6 @@
const char* password;
bool create;
bool use_old_format;
- const char* tracked_dirs[2];
};
extern const TestUserInfo kDefaultUsers[];
diff --git a/mount.cc b/mount.cc
index 7c23e5d..9f69d25 100644
--- a/mount.cc
+++ b/mount.cc
@@ -235,15 +235,6 @@
return false;
}
- // TODO(glotov): the following code is deprecated. Remove it.
- if (mount_args.replace_tracked_subdirectories) {
- if (ReplaceTrackedSubdirectories(mount_args.tracked_subdirectories,
- &serialized)) {
- // If the tracked subdirectories changed, re-save the vault keyset
- StoreVaultKeyset(credentials, serialized);
- }
- }
-
crypto_->ClearKeyset();
// Add the decrypted key to the keyring so that ecryptfs can use it
@@ -364,11 +355,10 @@
FilePath user_dir(GetUserDirectory(credentials));
file_util::CreateDirectory(user_dir);
- // Generat a new master key
+ // Generate a new master key
VaultKeyset vault_keyset;
vault_keyset.CreateRandom(*this);
SerializedVaultKeyset serialized;
- ReplaceTrackedSubdirectories(mount_args.tracked_subdirectories, &serialized);
if (!AddVaultKeyset(credentials, vault_keyset, &serialized)) {
platform_->SetMask(original_mask);
return false;
@@ -488,38 +478,6 @@
return result;
}
-bool Mount::ReplaceTrackedSubdirectories(
- const std::vector<std::string>& tracked_subdirectories,
- SerializedVaultKeyset* serialized) const {
- std::set<std::string> existing;
- for (int index = 0; index < serialized->tracked_subdirectories_size();
- ++index) {
- existing.insert(serialized->tracked_subdirectories(index));
- }
- bool new_exists = false;
- for (std::vector<std::string>::const_iterator itr =
- tracked_subdirectories.begin();
- itr != tracked_subdirectories.end();
- ++itr) {
- if (!existing.erase(*itr)) {
- new_exists = true;
- }
- }
- // If there are any subdirectories that were in one set but not the other,
- // then we need to replace
- if (existing.size() || new_exists) {
- serialized->clear_tracked_subdirectories();
- for (std::vector<std::string>::const_iterator itr =
- tracked_subdirectories.begin();
- itr != tracked_subdirectories.end();
- ++itr) {
- serialized->add_tracked_subdirectories(*itr);
- }
- return true;
- }
- return false;
-}
-
void Mount::CleanUnmountedTrackedSubdirectories() const {
FilePath shadow_root(shadow_root_);
file_util::FileEnumerator dir_enumerator(shadow_root, false,
diff --git a/mount.h b/mount.h
index e63ff87..2948340 100644
--- a/mount.h
+++ b/mount.h
@@ -59,30 +59,12 @@
struct MountArgs {
bool create_if_missing;
- bool replace_tracked_subdirectories;
- std::vector<std::string> tracked_subdirectories;
- MountArgs()
- : create_if_missing(false),
- replace_tracked_subdirectories(false) {
- }
-
- void AssignSubdirsNullTerminatedList(const char** tracked_subdirectories) {
- while (*tracked_subdirectories != NULL) {
- this->tracked_subdirectories.push_back(*tracked_subdirectories);
- tracked_subdirectories++;
- }
+ MountArgs() : create_if_missing(false) {
}
void CopyFrom(const MountArgs& rhs) {
this->create_if_missing = rhs.create_if_missing;
- this->replace_tracked_subdirectories = rhs.replace_tracked_subdirectories;
- for (std::vector<std::string>::const_iterator itr =
- rhs.tracked_subdirectories.begin();
- itr != rhs.tracked_subdirectories.end();
- itr++) {
- this->tracked_subdirectories.push_back(*itr);
- }
}
};
@@ -159,14 +141,6 @@
virtual bool CreateTrackedSubdirectories(const Credentials& credentials,
bool is_new) const;
- // Replaces the tracked subdirectories, returning true if a substition was
- // made, or false if the set was the same
- //
- // Parameters
- virtual bool ReplaceTrackedSubdirectories(
- const std::vector<std::string>& tracked_subdirectories,
- SerializedVaultKeyset* serialized) const;
-
// Cleans (removes) content from unmounted tracked subdirectories
virtual void CleanUnmountedTrackedSubdirectories() const;
diff --git a/mount_unittest.cc b/mount_unittest.cc
index 331a9e8..ebfe543 100644
--- a/mount_unittest.cc
+++ b/mount_unittest.cc
@@ -346,73 +346,6 @@
system_salt.size()));
}
-TEST_F(MountTest, ChangeTrackedDirs) {
- // create a Mount instance that points to a good shadow root, test that it
- // will re-save the vault keyset on tracked dirs change
- Mount mount;
- NiceMock<MockTpm> tpm;
- mount.get_crypto()->set_tpm(&tpm);
- mount.set_shadow_root(kImageDir);
- mount.set_skel_source(kSkelDir);
- mount.set_use_tpm(false);
-
- // Test user at index 9 has a tracked dir "DIR0"
- cryptohome::SecureBlob passkey;
- cryptohome::Crypto::PasswordToPasskey(kDefaultUsers[9].password,
- system_salt_, &passkey);
- UsernamePasskey up(kDefaultUsers[9].username, passkey);
-
- EXPECT_TRUE(mount.Init());
-
- // Make sure the keyset has only one tracked directory, "DIR0"
- VaultKeyset vault_keyset;
- SerializedVaultKeyset serialized;
- Mount::MountError error;
- ASSERT_TRUE(mount.DecryptVaultKeyset(up, true, &vault_keyset, &serialized,
- &error));
-
- ASSERT_EQ(1, serialized.tracked_subdirectories_size());
- ASSERT_EQ(0, serialized.tracked_subdirectories(0).compare("DIR0"));
-
- // Make sure the tracked dirs change. serialized starts with DIR0
- std::vector<std::string> new_dirs;
- new_dirs.push_back("DIR0");
- ASSERT_FALSE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has "DIR0"
- ASSERT_EQ(1, serialized.tracked_subdirectories_size());
-
- new_dirs.clear();
- new_dirs.push_back("DIR1");
- ASSERT_TRUE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has "DIR1"
- ASSERT_EQ(1, serialized.tracked_subdirectories_size());
-
- new_dirs.clear();
- new_dirs.push_back("DIR1");
- new_dirs.push_back("DIR0");
- ASSERT_TRUE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has "DIR1", "DIR0"
- ASSERT_EQ(2, serialized.tracked_subdirectories_size());
-
- new_dirs.clear();
- new_dirs.push_back("DIR0");
- new_dirs.push_back("DIR1");
- ASSERT_FALSE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has "DIR1", "DIR0"
- ASSERT_EQ(2, serialized.tracked_subdirectories_size());
-
- new_dirs.clear();
- new_dirs.push_back("DIR0");
- ASSERT_TRUE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has "DIR0"
- ASSERT_EQ(1, serialized.tracked_subdirectories_size());
-
- new_dirs.clear();
- ASSERT_TRUE(mount.ReplaceTrackedSubdirectories(new_dirs, &serialized));
- // serialized now has nothing
- ASSERT_EQ(0, serialized.tracked_subdirectories_size());
-}
-
TEST_F(MountTest, MountCryptohome) {
// checks that cryptohome tries to mount successfully, and tests that the
// tracked directories are created/replaced as expected
@@ -674,6 +607,12 @@
ASSERT_TRUE(file_util::PathExists(vault_path.Append(kCacheDir)));
ASSERT_TRUE(file_util::PathExists(vault_path.Append(kDownloadsDir)));
+ // Check that vault path does not contain user data unencrypted.
+ // Note, that if we had real mount, we would see encrypted file names there;
+ // but with our mock mount, we must see empty directories.
+ EXPECT_TRUE(file_util::IsDirectoryEmpty(vault_path.Append(kCacheDir)));
+ EXPECT_TRUE(file_util::IsDirectoryEmpty(vault_path.Append(kDownloadsDir)));
+
// Check that Cache is clear (because it does not need migration) so
// it should not appear in a home dir.
EXPECT_FALSE(file_util::PathExists(cache_dir));
diff --git a/service.cc b/service.cc
index 7a46129..a7b17ed 100644
--- a/service.cc
+++ b/service.cc
@@ -333,8 +333,8 @@
gboolean Service::Mount(gchar *userid,
gchar *key,
gboolean create_if_missing,
- gboolean replace_tracked_subdirectories,
- gchar** tracked_subdirectories,
+ gboolean deprecated_replace_tracked_subdirectories,
+ gchar** deprecated_tracked_subdirectories,
gint *OUT_error_code,
gboolean *OUT_result,
GError **error) {
@@ -360,11 +360,6 @@
base::WaitableEvent event(true, false);
Mount::MountArgs mount_args;
mount_args.create_if_missing = create_if_missing;
- mount_args.replace_tracked_subdirectories = replace_tracked_subdirectories;
- if (tracked_subdirectories) {
- mount_args.AssignSubdirsNullTerminatedList(
- const_cast<const char**>(tracked_subdirectories));
- }
MountTaskMount* mount_task = new MountTaskMount(NULL,
mount_,
credentials,
@@ -381,8 +376,8 @@
gboolean Service::AsyncMount(gchar *userid,
gchar *key,
gboolean create_if_missing,
- gboolean replace_tracked_subdirectories,
- gchar** tracked_subdirectories,
+ gboolean deprecated_replace_tracked_subdirectories,
+ gchar** deprecated_tracked_subdirectories,
gint *OUT_async_id,
GError **error) {
UsernamePasskey credentials(userid, SecureBlob(key, strlen(key)));
@@ -412,11 +407,6 @@
Mount::MountArgs mount_args;
mount_args.create_if_missing = create_if_missing;
- mount_args.replace_tracked_subdirectories = replace_tracked_subdirectories;
- if (tracked_subdirectories) {
- mount_args.AssignSubdirsNullTerminatedList(
- const_cast<const char**>(tracked_subdirectories));
- }
MountTaskMount* mount_task = new MountTaskMount(this,
mount_,
credentials,
diff --git a/service.h b/service.h
index c3858e1..3b10291 100644
--- a/service.h
+++ b/service.h
@@ -103,18 +103,19 @@
virtual gboolean Mount(gchar *user,
gchar *key,
gboolean create_if_missing,
- gboolean replace_tracked_subdirectories,
- gchar** tracked_subdirectories,
+ gboolean deprecated_replace_tracked_subdirectories,
+ gchar** deprecated_tracked_subdirectories,
gint *OUT_error_code,
gboolean *OUT_result,
GError **error);
- virtual gboolean AsyncMount(gchar *user,
- gchar *key,
- gboolean create_if_missing,
- gboolean replace_tracked_subdirectories,
- gchar** tracked_subdirectories,
- gint *OUT_async_id,
- GError **error);
+ virtual gboolean AsyncMount(
+ gchar *user,
+ gchar *key,
+ gboolean create_if_missing,
+ gboolean deprecated_replace_tracked_subdirectories,
+ gchar** deprecated_tracked_subdirectories,
+ gint *OUT_async_id,
+ GError **error);
virtual gboolean MountGuest(gint *OUT_error_code,
gboolean *OUT_result,
GError **error);
diff --git a/vault_keyset.proto b/vault_keyset.proto
index 13a4ba8..5517045 100644
--- a/vault_keyset.proto
+++ b/vault_keyset.proto
@@ -16,5 +16,5 @@
optional bytes tpm_key = 4;
optional bytes tpm_public_key_hash = 5;
optional int32 password_rounds = 6;
- repeated string tracked_subdirectories = 7;
+ repeated string deprecated_tracked_subdirectories = 7;
}
