From 3810333adbd947f4889e7cfbe3bd97ab56276165 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Wed, 27 Jan 2021 15:49:04 -0800 Subject: [PATCH] feat: add experimental cookie encryption support on macOS --- BUILD.gn | 3 +- build/fuses/build.py | 4 +- build/fuses/{fuses.json => fuses.json5} | 4 +- docs/tutorial/fuses.md | 2 +- patches/chromium/.patches | 1 + ...ame_and_service_name_mutable_strings.patch | 53 +++++++++++++++++++ shell/browser/net/network_context_service.cc | 9 +++- .../net/system_network_context_manager.cc | 20 +++++++ 8 files changed, 89 insertions(+), 7 deletions(-) rename build/fuses/{fuses.json => fuses.json5} (71%) create mode 100644 patches/chromium/make_keychainpassword_service_name_and_service_name_mutable_strings.patch diff --git a/BUILD.gn b/BUILD.gn index 00f95f8d9c525..7ca7017041559 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -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", @@ -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", diff --git a/build/fuses/build.py b/build/fuses/build.py index f17c08fdb8451..6c56dceb091df 100755 --- a/build/fuses/build.py +++ b/build/fuses/build.py @@ -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'] diff --git a/build/fuses/fuses.json b/build/fuses/fuses.json5 similarity index 71% rename from build/fuses/fuses.json rename to build/fuses/fuses.json5 index 9ff211adb43e3..d86fdd1d0977f 100644 --- a/build/fuses/fuses.json +++ b/build/fuses/fuses.json5 @@ -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" } diff --git a/docs/tutorial/fuses.md b/docs/tutorial/fuses.md index 76141ca314402..90a039a530f7b 100644 --- a/docs/tutorial/fuses.md +++ b/docs/tutorial/fuses.md @@ -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). diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 913224ba1ba67..4a8d5701dacc7 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -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 diff --git a/patches/chromium/make_keychainpassword_service_name_and_service_name_mutable_strings.patch b/patches/chromium/make_keychainpassword_service_name_and_service_name_mutable_strings.patch new file mode 100644 index 0000000000000..03937876ea792 --- /dev/null +++ b/patches/chromium/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 +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) { diff --git a/shell/browser/net/network_context_service.cc b/shell/browser/net/network_context_service.cc index 53689a8d2d427..6beec0e60e70b 100644 --- a/shell/browser/net/network_context_service.cc +++ b/shell/browser/net/network_context_service.cc @@ -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" @@ -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; } diff --git a/shell/browser/net/system_network_context_manager.cc b/shell/browser/net/system_network_context_manager.cc index 8816f2ba308d2..2b81d98fae2bb 100644 --- a/shell/browser/net/system_network_context_manager.cc +++ b/shell/browser/net/system_network_context_manager.cc @@ -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" @@ -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" @@ -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