OSCrypt for POSIX uses libsecret to store a randomised encryption key.

components/os_crypt/os_crypt_util_linux_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "components/os_crypt/key_storage_libsecret.h"
+#include "components/os_crypt/key_storage_linux.h"
+#include "components/os_crypt/libsecret_util_linux.h"
+#include "components/os_crypt/os_crypt.h"
+#include "components/os_crypt/os_crypt_mocker_linux.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+using MockSecretValue = std::string;

[vabr (Chromium), 2016/05/23 12:38:33]

Why are we renaming std::string? Unless there is a concrete benefit in doing
that, I would suggest keeping std::string is more readable.

[cfroussios, 2016/05/30 11:50:17]

The MockSecretValue maintains symmetry between libsecret methods and their
mocks. Someone introducing a new libsecret function and its mock will be coming
with the original signature in mind. I would argue that it is more readable to
acknowledge the type rather than erase it. That said, I don't have a strong
preference.

[vabr (Chromium), 2016/05/30 15:05:37]

I'm afraid I don't understand what you mean by "maintains symmetry", could you
please try to explain that bit again to me?

[cfroussios, 2016/05/30 16:19:41]

By symmetry I mean calling out the type being mocked. Where there is SecretValue
in libsecret, the mock has MockSecretValue.

SecretValue is a complex type in libsecret, separate from text values that
appear as char arrays in the signatures. Here, we can reduce it to an
std::string because
1) In the current implementation, we only need one of its fields.
3) Production code only accesses its fields through functions.
2) The mock function is cast to the correct signature in when setting it up.
Therefore, the pointers to SecretValue can point to anything we find convenient.

I think that expecting a future author, who's looking for what to do with a
SecretValue, to see std::string and deduce how we cast to something else
entirely, is more mental work. Calling out the mocked type gives a trail to
follow. It also makes more apparent what needs to change, if we use more fields
of SecretValue in the future.

[vabr (Chromium), 2016/05/30 18:00:47]

Thanks, this makes great sense to me now. I agree with your choice, please keep
MockSecretValue. It might be a good idea to summarise your explanation in a form
of a code comment, though, in case somebody as slow as me is reading the code in
the future and wondering what this is. :)

[cfroussios, 2016/05/31 10:56:11]

Done.

+
+// Replaces some of LibsecretLoader's methods with mocked ones.
+class MockLibsecretLoader : public LibsecretLoader {
+ public:
+  // Sets up the minimum mock implementation necessary for OSCrypt to work
+  // with Libsecret. Also resets the state to mock a clean database.
+  static bool ResetForOSCrypt();
+
+  // Sets OSCrypt's password in the libsecret mock to a specific value
+  static void SetOSCryptPassword(const char*);
+
+  // Releases memory and restores LibsecretLoader to an uninitialized state.
+  static void TearDown();
+
+ private:
+  static MockSecretValue* stored_password_mock_ptr_;

[vabr (Chromium), 2016/05/23 12:38:33]

Why does this need to be static? It is only reset in the static method, all
other uses are within non-static ones. Could you make this a non-static
unique_ptr?

[cfroussios, 2016/05/30 11:50:17]

mock_secret_password_store_sync needs to be able to set it and
mock_secret_value_get_text needs to return it. Those need to be static
functions, in order to mock libsecret calls. 

[vabr (Chromium), 2016/05/30 15:05:37]

Acknowledged.

+
+  // These methods are used to redirect calls through LibsecretLoader
+  static const gchar* mock_secret_value_get_text(MockSecretValue* value);
+
+  static gboolean mock_secret_password_store_sync(const SecretSchema* schema,
+                                                  const gchar* collection,
+                                                  const gchar* label,
+                                                  const gchar* password,
+                                                  GCancellable* cancellable,
+                                                  GError** error,
+                                                  ...);
+
+  static MockSecretValue* mock_secret_service_lookup_sync(
+      SecretService* service,
+      const SecretSchema* schema,
+      GHashTable* attributes,
+      GCancellable* cancellable,
+      GError** error);
+
+  static void mock_secret_value_unref(gpointer value);
+
+  static GList* mock_secret_service_search_sync(SecretService* service,
+                                                const SecretSchema* schema,
+                                                GHashTable* attributes,
+                                                SecretSearchFlags flags,
+                                                GCancellable* cancellable,
+                                                GError** error);
+};
+
+MockSecretValue* MockLibsecretLoader::stored_password_mock_ptr_ = nullptr;
+
+const gchar* MockLibsecretLoader::mock_secret_value_get_text(
+    MockSecretValue* value) {
+  return value->c_str();
+}
+
+gboolean MockLibsecretLoader::mock_secret_password_store_sync(
+    const SecretSchema* schema,
+    const gchar* collection,
+    const gchar* label,
+    const gchar* password,
+    GCancellable* cancellable,
+    GError** error,
+    ...) {
+  delete stored_password_mock_ptr_;
+  stored_password_mock_ptr_ = new MockSecretValue(password);
+  return true;
+}
+
+MockSecretValue* MockLibsecretLoader::mock_secret_service_lookup_sync(
+    SecretService* service,
+    const SecretSchema* schema,
+    GHashTable* attributes,
+    GCancellable* cancellable,
+    GError** error) {
+  return stored_password_mock_ptr_;
+}
+
+void MockLibsecretLoader::mock_secret_value_unref(gpointer value) {}
+
+GList* MockLibsecretLoader::mock_secret_service_search_sync(
+    SecretService* service,
+    const SecretSchema* schema,
+    GHashTable* attributes,
+    SecretSearchFlags flags,
+    GCancellable* cancellable,
+    GError** error) {
+  *error = nullptr;
+  return nullptr;
+}
+
+// static
+bool MockLibsecretLoader::ResetForOSCrypt() {
+  // 4 methods used by KeyStorageLibsecret::GetKey()
+  secret_password_store_sync =
+      &MockLibsecretLoader::mock_secret_password_store_sync;
+  secret_value_get_text = (decltype(&::secret_value_get_text)) &
+                          MockLibsecretLoader::mock_secret_value_get_text;
+  secret_value_unref = &MockLibsecretLoader::mock_secret_value_unref;
+  secret_service_lookup_sync =
+      (decltype(&::secret_service_lookup_sync)) &
+      MockLibsecretLoader::mock_secret_service_lookup_sync;
+  // 1 method used by LibsecretLoader::EnsureLibsecretLoaded()
+  secret_service_search_sync =
+      &MockLibsecretLoader::mock_secret_service_search_sync;
+
+  delete stored_password_mock_ptr_;
+  stored_password_mock_ptr_ = nullptr;
+  libsecret_loaded_ = true;
+
+  return true;
+}
+
+void MockLibsecretLoader::SetOSCryptPassword(const char* value) {
+  delete stored_password_mock_ptr_;
+  stored_password_mock_ptr_ = new MockSecretValue(value);
+}
+
+void MockLibsecretLoader::TearDown() {
+  delete stored_password_mock_ptr_;
+  stored_password_mock_ptr_ = nullptr;
+  libsecret_loaded_ = false;  // function pointers will be restored when loading

[vabr (Chromium), 2016/05/23 12:38:33]

nit: Start a sentence with a capital letter and end it with a full-stop.

[cfroussios, 2016/05/30 11:50:17]

Done.

+}
+
+class OSCryptLinuxTest : public testing::Test {

[vabr (Chromium), 2016/05/23 12:38:33]

Could all the OSCryptLinuxTests be in a separate file,
os_crypt_linux_unittest.cc? It is unexpected to have them here, and may lead
people to omit adding more tests in the future, because they would think there
are none so far.

[cfroussios, 2016/05/30 11:50:17]

Done.

+ public:
+  OSCryptLinuxTest() = default;
+  ~OSCryptLinuxTest() override = default;
+
+  void SetUp() override {
+    OSCryptMockerLinux::SetUpWithSingleton();
+    key_storage_ = OSCryptMockerLinux::GetInstance();
+  }
+
+  void TearDown() override { OSCryptMockerLinux::TearDown(); }
+
+ protected:
+  OSCryptMockerLinux* key_storage_ = nullptr;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(OSCryptLinuxTest);
+};
+
+TEST_F(OSCryptLinuxTest, VerifyV0) {
+  const std::string originaltext = "hello";
+  std::string ciphertext;
+  std::string decipheredtext;
+
+  key_storage_->ResetTo("");
+  ciphertext = originaltext;  // No encryption
+  ASSERT_TRUE(OSCrypt::DecryptString(ciphertext, &decipheredtext));
+  ASSERT_EQ(originaltext, decipheredtext);
+}
+
+TEST_F(OSCryptLinuxTest, VerifyV10) {
+  const std::string originaltext = "hello";
+  std::string ciphertext;
+  std::string decipheredtext;
+
+  key_storage_->ResetTo("peanuts");
+  ASSERT_TRUE(OSCrypt::EncryptString(originaltext, &ciphertext));
+  key_storage_->ResetTo("not_peanuts");
+  ciphertext = ciphertext.substr(3).insert(0, "v10");
+  ASSERT_TRUE(OSCrypt::DecryptString(ciphertext, &decipheredtext));
+  ASSERT_EQ(originaltext, decipheredtext);
+}
+
+TEST_F(OSCryptLinuxTest, VerifyV11) {
+  const std::string originaltext = "hello";
+  std::string ciphertext;
+  std::string decipheredtext;
+
+  key_storage_->ResetTo("");
+  ASSERT_TRUE(OSCrypt::EncryptString(originaltext, &ciphertext));
+  ASSERT_EQ(ciphertext.substr(0, 3), "v11");
+  ASSERT_TRUE(OSCrypt::DecryptString(ciphertext, &decipheredtext));
+  ASSERT_EQ(originaltext, decipheredtext);
+}
+
+class LibsecretTest : public testing::Test {
+ public:
+  LibsecretTest() = default;
+  ~LibsecretTest() override = default;
+
+  void SetUp() override { MockLibsecretLoader::ResetForOSCrypt(); }
+
+  void TearDown() override { MockLibsecretLoader::TearDown(); }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LibsecretTest);
+};
+
+TEST_F(LibsecretTest, LibsecretRepeats) {
+  KeyStorageLibsecret libsecret;
+  MockLibsecretLoader::ResetForOSCrypt();
+  std::string password = libsecret.GetKey();
+  EXPECT_FALSE(password.empty());
+  std::string password_repeat = libsecret.GetKey();
+  EXPECT_EQ(password, password_repeat);
+}
+
+TEST_F(LibsecretTest, LibsecretCreatesRandomised) {
+  KeyStorageLibsecret libsecret;
+  MockLibsecretLoader::ResetForOSCrypt();
+  std::string password = libsecret.GetKey();
+  MockLibsecretLoader::ResetForOSCrypt();
+  std::string password_new = libsecret.GetKey();
+  EXPECT_NE(password, password_new);
+}
+
+}  // namespace