Skip to content

Commit

Permalink
feat: add experimental cookie encryption support on macOS
Browse files Browse the repository at this point in the history
  • Loading branch information
MarshallOfSound committed Jan 27, 2021
1 parent ed126ec commit 3810333
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 7 deletions.
3 changes: 2 additions & 1 deletion BUILD.gn
Expand Up @@ -290,7 +290,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 @@ -329,6 +329,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/upload_list",
Expand Down
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
4 changes: 3 additions & 1 deletion build/fuses/fuses.json → build/fuses/fuses.json5
Expand Up @@ -2,5 +2,7 @@
"_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 is currently only supported on macOS
"cookie_encryption": "1"
}
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 @@ -107,3 +107,4 @@ use_public_apis_to_determine_if_a_font_is_a_system_font_in_mas_build.patch
fix_setparentacessibile_crash_win.patch
fix_export_zlib_symbols.patch
don_t_use_potentially_null_getwebframe_-_view_when_get_blink.patch
make_keychainpassword_service_name_and_service_name_mutable_strings.patch
@@ -0,0 +1,53 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <sattard@slack-corp.com>
Date: Wed, 27 Jan 2021 15:43:35 -0800
Subject: make KeychainPassword::service_name and service_name mutable strings

This should be upstreamable, it's just a type change that makes it
possible for embedders to adjust the service name for the keychain
credential storage.

diff --git a/components/os_crypt/keychain_password_mac.h b/components/os_crypt/keychain_password_mac.h
index 6fda0244667c4eb5d1abb973f4b72d9c59ed2165..45c815620e3e095366de8586ef6a6872b8e04c0b 100644
--- a/components/os_crypt/keychain_password_mac.h
+++ b/components/os_crypt/keychain_password_mac.h
@@ -28,8 +28,8 @@ class COMPONENT_EXPORT(OS_CRYPT) KeychainPassword {
std::string GetPassword() const;

// The service and account names used in Chrome's Safe Storage keychain item.
- static COMPONENT_EXPORT(OS_CRYPT) const char service_name[];
- static COMPONENT_EXPORT(OS_CRYPT) const char account_name[];
+ static COMPONENT_EXPORT(OS_CRYPT) std::string service_name;
+ static COMPONENT_EXPORT(OS_CRYPT) std::string account_name;

private:
const crypto::AppleKeychain& keychain_;
diff --git a/components/os_crypt/keychain_password_mac.mm b/components/os_crypt/keychain_password_mac.mm
index 3b8543488d0bb0ae7c0996db70f9527bc5e34cd8..0214a676ce5e49c59d8e618aa8a56c241ffc1adb 100644
--- a/components/os_crypt/keychain_password_mac.mm
+++ b/components/os_crypt/keychain_password_mac.mm
@@ -48,11 +48,11 @@
// the encryption keyword. So as to not lose encrypted data when system
// locale changes we DO NOT LOCALIZE.
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-const char KeychainPassword::service_name[] = "Chrome Safe Storage";
-const char KeychainPassword::account_name[] = "Chrome";
+std::string KeychainPassword::service_name = "Chrome Safe Storage";
+std::string KeychainPassword::account_name = "Chrome";
#else
-const char KeychainPassword::service_name[] = "Chromium Safe Storage";
-const char KeychainPassword::account_name[] = "Chromium";
+std::string KeychainPassword::service_name = "Chromium Safe Storage";
+std::string KeychainPassword::account_name = "Chromium";
#endif

KeychainPassword::KeychainPassword(const AppleKeychain& keychain)
@@ -64,7 +64,7 @@
UInt32 password_length = 0;
void* password_data = nullptr;
OSStatus error = keychain_.FindGenericPassword(
- strlen(service_name), service_name, strlen(account_name), account_name,
+ service_name.length(), service_name.c_str(), account_name.length(), account_name.c_str(),
&password_length, &password_data, nullptr);

if (error == noErr) {
9 changes: 7 additions & 2 deletions shell/browser/net/network_context_service.cc
Expand Up @@ -8,6 +8,7 @@

#include "chrome/common/chrome_constants.h"
#include "content/public/browser/network_service_instance.h"
#include "electron/fuses.h"
#include "net/net_buildflags.h"
#include "services/network/network_service.h"
#include "shell/browser/browser_process_impl.h"
Expand Down Expand Up @@ -69,9 +70,13 @@ void NetworkContextService::ConfigureNetworkContextParams(
network_context_params->restore_old_session_cookies = false;
network_context_params->persist_session_cookies = false;

// TODO(deepak1556): Matches the existing behavior https://git.io/fxHMl,
// enable encryption as a followup.
// TODO: Make it possible to enable cookie encryption on non-macOS platforms
#if defined(OS_MAC)
network_context_params->enable_encrypted_cookies =
electron::fuses::IsCookieEncryptionEnabled();
#else
network_context_params->enable_encrypted_cookies = false;
#endif

network_context_params->transport_security_persister_path = path;
}
Expand Down
20 changes: 20 additions & 0 deletions shell/browser/net/system_network_context_manager.cc
Expand Up @@ -10,9 +10,13 @@
#include "base/command_line.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/net/chrome_mojo_proxy_resolver_factory.h"
#include "components/os_crypt/keychain_password_mac.h"
#include "components/os_crypt/os_crypt.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/content_features.h"
#include "content/public/common/network_service_util.h"
#include "electron/fuses.h"
#include "mojo/public/cpp/bindings/associated_interface_ptr.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "net/net_buildflags.h"
Expand All @@ -21,6 +25,7 @@
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "shell/browser/browser.h"
#include "shell/browser/electron_browser_client.h"
#include "shell/common/application_info.h"
#include "shell/common/options_switches.h"
Expand Down Expand Up @@ -213,6 +218,21 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
network_service->CreateNetworkContext(
network_context_.BindNewPipeAndPassReceiver(),
CreateNetworkContextParams());

if (electron::fuses::IsCookieEncryptionEnabled()) {
#if defined(OS_WIN) || defined(OS_MAC)
#if defined(OS_MAC)
KeychainPassword::service_name =
electron::Browser::Get()->GetName() + " Safe Storage";
KeychainPassword::account_name = electron::Browser::Get()->GetName();
#endif
// The OSCrypt keys are process bound, so if network service is out of
// process, send it the required key.
if (content::IsOutOfProcessNetworkService()) {
network_service->SetEncryptionKey(OSCrypt::GetRawEncryptionKey());
}
#endif
}
}

network::mojom::NetworkContextParamsPtr
Expand Down

0 comments on commit 3810333

Please sign in to comment.