Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add experimental cookie encryption support #29492

Merged
merged 1 commit into from Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion BUILD.gn
@@ -1,6 +1,7 @@
import("//build/config/locales.gni")
import("//build/config/ui.gni")
import("//build/config/win/manifest.gni")
import("//components/os_crypt/features.gni")
import("//components/spellcheck/spellcheck_build_features.gni")
import("//content/public/app/mac_helpers.gni")
import("//extensions/buildflags/buildflags.gni")
Expand Down Expand Up @@ -296,7 +297,7 @@ templated_file("electron_version_header") {
action("electron_fuses") {
script = "build/fuses/build.py"

inputs = [ "build/fuses/fuses.json" ]
inputs = [ "build/fuses/fuses.json5" ]

outputs = [
"$target_gen_dir/fuses.h",
Expand Down Expand Up @@ -335,6 +336,7 @@ source_set("electron_lib") {
"//components/network_hints/common:mojo_bindings",
"//components/network_hints/renderer",
"//components/network_session_configurator/common",
"//components/os_crypt",
"//components/pref_registry",
"//components/prefs",
"//components/security_state/content",
Expand Down Expand Up @@ -684,6 +686,10 @@ source_set("electron_lib") {
]
libs += [ "uxtheme.lib" ]
}

if (allow_runtime_configurable_key_storage) {
defines += [ "ALLOW_RUNTIME_CONFIGURABLE_KEY_STORAGE" ]
}
}

electron_paks("packed_resources") {
Expand Down
3 changes: 3 additions & 0 deletions build/args/all.gn
Expand Up @@ -30,3 +30,6 @@ enable_pseudolocales = false
is_cfi = false

enable_pak_file_integrity_checks = false

# Make application name configurable at runtime for cookie crypto
allow_runtime_configurable_key_storage = true
4 changes: 2 additions & 2 deletions build/fuses/build.py
Expand Up @@ -49,8 +49,8 @@
}
"""

with open(os.path.join(dir_path, "fuses.json"), 'r') as f:
fuse_defaults = json.load(f)
with open(os.path.join(dir_path, "fuses.json5"), 'r') as f:
fuse_defaults = json.loads(''.join(line for line in f.readlines() if not line.strip()[0] == "/"))

fuse_version = fuse_defaults['_version']
del fuse_defaults['_version']
Expand Down
3 changes: 2 additions & 1 deletion build/fuses/fuses.json → build/fuses/fuses.json5
Expand Up @@ -2,5 +2,6 @@
"_comment": "Modifying the fuse schema in any breaking way should result in the _version prop being incremented. NEVER remove a fuse or change its meaning, instead mark it as removed with 'r'",
"_schema": "0 == off, 1 == on, r == removed fuse",
"_version": 1,
"run_as_node": "1"
"run_as_node": "1",
"cookie_encryption": "0"
}
2 changes: 1 addition & 1 deletion docs/tutorial/fuses.md
Expand Up @@ -51,4 +51,4 @@ Somewhere in the Electron binary there will be a sequence of bytes that look lik

To flip a fuse you find its position in the fuse wire and change it to "0" or "1" depending on the state you'd like.

You can view the current schema [here](https://github.com/electron/electron/blob/master/build/fuses/fuses.json).
You can view the current schema [here](https://github.com/electron/electron/blob/master/build/fuses/fuses.json5).
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -102,3 +102,4 @@ add_setter_for_browsermainloop_result_code.patch
chore_allow_overriding_of_enable_pak_file_integrity_checks.patch
make_include_of_stack_trace_h_unconditional.patch
build_libc_as_static_library.patch
support_runtime_configurable_key_storage_on_linux_os_crypto.patch
@@ -0,0 +1,288 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <samuel.r.attard@gmail.com>
Date: Mon, 10 May 2021 17:10:25 -0700
Subject: support runtime configurable key storage on linux (os_crypto)

This modifies the OsCrypt::Config struct used on linux to support
runtime configurable application names which are used in the Keyring and
LibSecret implementations of os_crypt on linux.

Change-Id: Ifc287b589f118da8fcd5afaf39e5ba7ffe46f5fd

diff --git a/components/os_crypt/key_storage_config_linux.h b/components/os_crypt/key_storage_config_linux.h
index a856604756aa65c52171a9eff84ba2b316d8609c..72c16682e5df615ab84f67af66cc36c2b76c30e3 100644
--- a/components/os_crypt/key_storage_config_linux.h
+++ b/components/os_crypt/key_storage_config_linux.h
@@ -26,6 +26,12 @@ struct COMPONENT_EXPORT(OS_CRYPT) Config {
std::string store;
// The product name to use for permission prompts.
std::string product_name;
+ // The application name to store the key under. For Chromium/Chrome builds
+ // leave this unset and it will default correctly. This config option is
+ // for embedders to provide their application name in place of "Chromium".
+ // Only used when the allow_runtime_configurable_key_storage feature is
+ // enabled.
+ std::string application_name;
// A runner on the main thread for gnome-keyring to be called from.
// TODO(crbug/466975): Libsecret and KWallet don't need this. We can remove
// this when we stop supporting keyring.
diff --git a/components/os_crypt/key_storage_keyring.cc b/components/os_crypt/key_storage_keyring.cc
index 29720b8e8ff81791970e054050b37bbc5f338eb3..3ad654cf4a893f497ac4567aea77f8d1f2e1525f 100644
--- a/components/os_crypt/key_storage_keyring.cc
+++ b/components/os_crypt/key_storage_keyring.cc
@@ -15,12 +15,6 @@

namespace {

-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-const char kApplicationName[] = "chrome";
-#else
-const char kApplicationName[] = "chromium";
-#endif
-
const GnomeKeyringPasswordSchema kSchema = {
GNOME_KEYRING_ITEM_GENERIC_SECRET,
{{"application", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING}, {nullptr}}};
@@ -28,8 +22,10 @@ const GnomeKeyringPasswordSchema kSchema = {
} // namespace

KeyStorageKeyring::KeyStorageKeyring(
- scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner)
- : main_thread_runner_(main_thread_runner) {}
+ scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner,
+ std::string application_name)
+ : main_thread_runner_(main_thread_runner),
+ application_name_(std::move(application_name)) {}

KeyStorageKeyring::~KeyStorageKeyring() {}

@@ -49,7 +45,8 @@ absl::optional<std::string> KeyStorageKeyring::GetKeyImpl() {
gchar* password_c = nullptr;
GnomeKeyringResult result =
GnomeKeyringLoader::gnome_keyring_find_password_sync_ptr(
- &kSchema, &password_c, "application", kApplicationName, nullptr);
+ &kSchema, &password_c, "application", application_name_.c_str(),
+ nullptr);
if (result == GNOME_KEYRING_RESULT_OK) {
password = password_c;
GnomeKeyringLoader::gnome_keyring_free_password_ptr(password_c);
@@ -71,7 +68,7 @@ absl::optional<std::string> KeyStorageKeyring::AddRandomPasswordInKeyring() {
GnomeKeyringResult result =
GnomeKeyringLoader::gnome_keyring_store_password_sync_ptr(
&kSchema, nullptr /* default keyring */, KeyStorageLinux::kKey,
- password.c_str(), "application", kApplicationName, nullptr);
+ password.c_str(), "application", application_name_.c_str(), nullptr);
if (result != GNOME_KEYRING_RESULT_OK) {
VLOG(1) << "OSCrypt failed to store generated password to gnome-keyring";
return absl::nullopt;
diff --git a/components/os_crypt/key_storage_keyring.h b/components/os_crypt/key_storage_keyring.h
index 26a3f587b49cdea3e4913ce0e08eeade6d1853a0..598c8b8ad35efa002a29dd98c9cba98453bef5ac 100644
--- a/components/os_crypt/key_storage_keyring.h
+++ b/components/os_crypt/key_storage_keyring.h
@@ -20,8 +20,9 @@ class SingleThreadTaskRunner;
// Specialisation of KeyStorageLinux that uses Libsecret.
class COMPONENT_EXPORT(OS_CRYPT) KeyStorageKeyring : public KeyStorageLinux {
public:
- explicit KeyStorageKeyring(
- scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner);
+ KeyStorageKeyring(
+ scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner,
+ std::string application_name);
~KeyStorageKeyring() override;

protected:
@@ -37,6 +38,8 @@ class COMPONENT_EXPORT(OS_CRYPT) KeyStorageKeyring : public KeyStorageLinux {
// Keyring calls need to originate from the main thread.
scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner_;

+ const std::string application_name_;
+
DISALLOW_COPY_AND_ASSIGN(KeyStorageKeyring);
};

diff --git a/components/os_crypt/key_storage_keyring_unittest.cc b/components/os_crypt/key_storage_keyring_unittest.cc
index aba31971dc33f4b60c2847fd037c419baf084eaa..93fc216d54168eb3fd7e9f1b23f71a77f6e6735d 100644
--- a/components/os_crypt/key_storage_keyring_unittest.cc
+++ b/components/os_crypt/key_storage_keyring_unittest.cc
@@ -130,7 +130,7 @@ class GnomeKeyringTest : public testing::Test {
};

GnomeKeyringTest::GnomeKeyringTest()
- : task_runner_(new base::TestSimpleTaskRunner()), keyring_(task_runner_) {
+ : task_runner_(new base::TestSimpleTaskRunner()), keyring_(task_runner_, "chromium") {
MockGnomeKeyringLoader::ResetForOSCrypt();
}

diff --git a/components/os_crypt/key_storage_libsecret.cc b/components/os_crypt/key_storage_libsecret.cc
index 0857cc1874f4153541c7f21dd8cc6bdc1d59b39d..ccc60522791508f02dc759e55e781dbb9627967b 100644
--- a/components/os_crypt/key_storage_libsecret.cc
+++ b/components/os_crypt/key_storage_libsecret.cc
@@ -14,12 +14,6 @@

namespace {

-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-const char kApplicationName[] = "chrome";
-#else
-const char kApplicationName[] = "chromium";
-#endif
-
const SecretSchema kKeystoreSchemaV2 = {
"chrome_libsecret_os_crypt_password_v2",
SECRET_SCHEMA_DONT_MATCH_NAME,
@@ -64,6 +58,9 @@ void AnalyseKeyHistory(GList* secret_items) {

} // namespace

+KeyStorageLibsecret::KeyStorageLibsecret(std::string application_name)
+ : application_name_(std::move(application_name)) {}
+
absl::optional<std::string>
KeyStorageLibsecret::AddRandomPasswordInLibsecret() {
std::string password;
@@ -71,7 +68,7 @@ KeyStorageLibsecret::AddRandomPasswordInLibsecret() {
GError* error = nullptr;
bool success = LibsecretLoader::secret_password_store_sync(
&kKeystoreSchemaV2, nullptr, KeyStorageLinux::kKey, password.c_str(),
- nullptr, &error, "application", kApplicationName, nullptr);
+ nullptr, &error, "application", application_name_.c_str(), nullptr);
if (error) {
VLOG(1) << "Libsecret lookup failed: " << error->message;
g_error_free(error);
@@ -88,7 +85,7 @@ KeyStorageLibsecret::AddRandomPasswordInLibsecret() {

absl::optional<std::string> KeyStorageLibsecret::GetKeyImpl() {
LibsecretAttributesBuilder attrs;
- attrs.Append("application", kApplicationName);
+ attrs.Append("application", application_name_);

LibsecretLoader::SearchHelper helper;
helper.Search(&kKeystoreSchemaV2, attrs.Get(),
diff --git a/components/os_crypt/key_storage_libsecret.h b/components/os_crypt/key_storage_libsecret.h
index 4759d076ea0017a41b398491dd24dde76a61e463..5292e21b8e7e1a873591e474571adb5b4ed8fe16 100644
--- a/components/os_crypt/key_storage_libsecret.h
+++ b/components/os_crypt/key_storage_libsecret.h
@@ -15,7 +15,7 @@
// Specialisation of KeyStorageLinux that uses Libsecret.
class COMPONENT_EXPORT(OS_CRYPT) KeyStorageLibsecret : public KeyStorageLinux {
public:
- KeyStorageLibsecret() = default;
+ explicit KeyStorageLibsecret(std::string application_name);
~KeyStorageLibsecret() override = default;

protected:
@@ -26,6 +26,8 @@ class COMPONENT_EXPORT(OS_CRYPT) KeyStorageLibsecret : public KeyStorageLinux {
private:
absl::optional<std::string> AddRandomPasswordInLibsecret();

+ const std::string application_name_;
+
DISALLOW_COPY_AND_ASSIGN(KeyStorageLibsecret);
};

diff --git a/components/os_crypt/key_storage_libsecret_unittest.cc b/components/os_crypt/key_storage_libsecret_unittest.cc
index ebe9a6b4bcbf748cd7f7e5fcf941b78ab1835749..a17bbc1f8061b33d55444919f546256d63bc809b 100644
--- a/components/os_crypt/key_storage_libsecret_unittest.cc
+++ b/components/os_crypt/key_storage_libsecret_unittest.cc
@@ -236,7 +236,7 @@ class LibsecretTest : public testing::Test {
};

TEST_F(LibsecretTest, LibsecretRepeats) {
- KeyStorageLibsecret libsecret;
+ KeyStorageLibsecret libsecret("chromium");
MockLibsecretLoader::ResetForOSCrypt();
g_password_store.Pointer()->SetPassword("initial password");
absl::optional<std::string> password = libsecret.GetKey();
@@ -248,7 +248,7 @@ TEST_F(LibsecretTest, LibsecretRepeats) {
}

TEST_F(LibsecretTest, LibsecretCreatesRandomised) {
- KeyStorageLibsecret libsecret;
+ KeyStorageLibsecret libsecret("chromium");
MockLibsecretLoader::ResetForOSCrypt();
absl::optional<std::string> password = libsecret.GetKey();
MockLibsecretLoader::ResetForOSCrypt();
diff --git a/components/os_crypt/key_storage_linux.cc b/components/os_crypt/key_storage_linux.cc
index 8feb147c96baa2f853a647eb648297b4b747f515..53b1903ff04043e2d058deda086e116a0e00fdca 100644
--- a/components/os_crypt/key_storage_linux.cc
+++ b/components/os_crypt/key_storage_linux.cc
@@ -11,6 +11,7 @@
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/nix/xdg_util.h"
+#include "base/no_destructor.h"
#include "base/sequenced_task_runner.h"
#include "base/synchronization/waitable_event.h"
#include "base/task_runner_util.h"
@@ -147,12 +148,29 @@ std::unique_ptr<KeyStorageLinux> KeyStorageLinux::CreateService(
std::unique_ptr<KeyStorageLinux> KeyStorageLinux::CreateServiceInternal(
os_crypt::SelectedLinuxBackend selected_backend,
const os_crypt::Config& config) {
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
+ static const base::NoDestructor<std::string> kDefaultApplicationName("chrome");
+#else
+ static const base::NoDestructor<std::string> kDefaultApplicationName("chromium");
+#endif
+
std::unique_ptr<KeyStorageLinux> key_storage;

+#if defined(USE_LIBSECRET) || defined(USE_KEYRING)
+#if defined(ALLOW_RUNTIME_CONFIGURABLE_KEY_STORAGE)
+ std::string application_name = config.application_name;
+ if (application_name.empty()) {
+ application_name = *kDefaultApplicationName;
+ }
+#else
+ std::string application_name = *kDefaultApplicationName;
+#endif
+#endif
+
#if defined(USE_LIBSECRET)
if (selected_backend == os_crypt::SelectedLinuxBackend::GNOME_ANY ||
selected_backend == os_crypt::SelectedLinuxBackend::GNOME_LIBSECRET) {
- key_storage = std::make_unique<KeyStorageLibsecret>();
+ key_storage = std::make_unique<KeyStorageLibsecret>(std::move(application_name));
if (key_storage->WaitForInitOnTaskRunner()) {
VLOG(1) << "OSCrypt using Libsecret as backend.";
return key_storage;
@@ -164,8 +182,8 @@ std::unique_ptr<KeyStorageLinux> KeyStorageLinux::CreateServiceInternal(
#if defined(USE_KEYRING)
if (selected_backend == os_crypt::SelectedLinuxBackend::GNOME_ANY ||
selected_backend == os_crypt::SelectedLinuxBackend::GNOME_KEYRING) {
- key_storage =
- std::make_unique<KeyStorageKeyring>(config.main_thread_runner);
+ key_storage = std::make_unique<KeyStorageKeyring>(config.main_thread_runner,
+ std::move(application_name));
if (key_storage->WaitForInitOnTaskRunner()) {
VLOG(1) << "OSCrypt using Keyring as backend.";
return key_storage;
diff --git a/services/network/network_service.cc b/services/network/network_service.cc
index 02c8af43258f6a31f3cc667e73f55f8f67330b51..bea06f626cc2b302ca8a25c753c37cea0d4c9e8c 100644
--- a/services/network/network_service.cc
+++ b/services/network/network_service.cc
@@ -622,6 +622,7 @@ void NetworkService::SetCryptConfig(mojom::CryptConfigPtr crypt_config) {
auto config = std::make_unique<os_crypt::Config>();
config->store = crypt_config->store;
config->product_name = crypt_config->product_name;
+ config->application_name = crypt_config->application_name;
config->main_thread_runner = base::ThreadTaskRunnerHandle::Get();
config->should_use_preference = crypt_config->should_use_preference;
config->user_data_path = crypt_config->user_data_path;
diff --git a/services/network/public/mojom/network_service.mojom b/services/network/public/mojom/network_service.mojom
index 3a1abc4d1b64ad9480f5218cd1de8fd61a064f34..0edbe59d3a70b5bca759cd8a726baded6b81ea71 100644
--- a/services/network/public/mojom/network_service.mojom
+++ b/services/network/public/mojom/network_service.mojom
@@ -99,6 +99,13 @@ struct CryptConfig {
// The product name to use for permission prompts.
string product_name;

+ // The application name to store the crypto key against. For Chromium/Chrome
+ // builds leave this unset and it will default correctly. This config option
+ // is for embedders to provide their application name in place of "Chromium".
+ // Only used when the allow_runtime_configurable_key_storage feature is
+ // enabled
+ string application_name;
+
// Controls whether preference on using or ignoring backends is used.
bool should_use_preference;