From adf55bb43b5dbfcf2de5f321658446856c4c8a99 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Wed, 15 Sep 2021 11:25:24 -0700 Subject: [PATCH] Fix wrong default service name and account name are used when getting keychain password. Also fix getting correct keychain password when importing from chrome, brave and chromium --- .../os_crypt/keychain_password_mac.h | 29 -------- .../os_crypt/keychain_password_mac.mm | 71 ++++++++++--------- ...ts-os_crypt-keychain_password_mac.mm.patch | 23 +++--- 3 files changed, 54 insertions(+), 69 deletions(-) delete mode 100644 chromium_src/components/os_crypt/keychain_password_mac.h diff --git a/chromium_src/components/os_crypt/keychain_password_mac.h b/chromium_src/components/os_crypt/keychain_password_mac.h deleted file mode 100644 index 1d2bffd1b2d9..000000000000 --- a/chromium_src/components/os_crypt/keychain_password_mac.h +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (c) 2021 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_OS_CRYPT_KEYCHAIN_PASSWORD_MAC_H_ -#define BRAVE_CHROMIUM_SRC_COMPONENTS_OS_CRYPT_KEYCHAIN_PASSWORD_MAC_H_ - -#define KeychainPassword KeychainPassword_ChromiumImpl -#include "../../../../components/os_crypt/keychain_password_mac.h" -#undef KeychainPassword - -class COMPONENT_EXPORT(OS_CRYPT) KeychainPassword - : public KeychainPassword_ChromiumImpl { - public: - KeychainPassword(const crypto::AppleKeychain& keychain); - ~KeychainPassword(); - - // The service and account names used in Brave's Safe Storage keychain item. - // Hides base implementation - static COMPONENT_EXPORT(OS_CRYPT) KeychainNameType& GetServiceName(); - static COMPONENT_EXPORT(OS_CRYPT) KeychainNameType& GetAccountName(); - - private: - KeychainPassword(const KeychainPassword&) = delete; - KeychainPassword& operator=(const KeychainPassword&) = delete; -}; - -#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_OS_CRYPT_KEYCHAIN_PASSWORD_MAC_H_ diff --git a/chromium_src/components/os_crypt/keychain_password_mac.mm b/chromium_src/components/os_crypt/keychain_password_mac.mm index 3f8b37788779..163debc2e6bd 100644 --- a/chromium_src/components/os_crypt/keychain_password_mac.mm +++ b/chromium_src/components/os_crypt/keychain_password_mac.mm @@ -5,48 +5,55 @@ #include "components/os_crypt/keychain_password_mac.h" -#include +#include #include "base/command_line.h" -#define BRAVE_KEYCHAIN_PASSWORD_GET_PASSWORD \ - std::unique_ptr service_name, account_name; \ - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); \ - if (command_line->HasSwitch("import-chrome")) { \ - service_name = std::make_unique("Chrome Safe Storage"); \ - account_name = std::make_unique("Chrome"); \ - } else if (command_line->HasSwitch("import-chromium") || \ - command_line->HasSwitch("import-brave")) { \ - service_name = std::make_unique("Chromium Safe Storage"); \ - account_name = std::make_unique("Chromium"); \ - } else { \ - service_name = std::make_unique( \ - ::KeychainPassword::GetServiceName().c_str()); \ - account_name = std::make_unique( \ - ::KeychainPassword::GetAccountName().c_str()); \ - } - -#define KeychainPassword KeychainPassword_ChromiumImpl -#include "../../../../components/os_crypt/keychain_password_mac.mm" -#undef KeychainPassword -#undef BRAVE_KEYCHAIN_PASSWORD_GET_PASSWORD +namespace { const char kBraveDefaultServiceName[] = "Brave Safe Storage"; const char kBraveDefaultAccountName[] = "Brave"; -// static -KeychainPassword::KeychainNameType& KeychainPassword::GetServiceName() { - static KeychainNameContainerType service_name(kBraveDefaultServiceName); +KeychainPassword::KeychainNameType& GetBraveServiceName(); +KeychainPassword::KeychainNameType& GetBraveAccountName(); + +} + +#define BRAVE_GET_SERVICE_NAME return GetBraveServiceName(); +#define BRAVE_GET_ACCOUNT_NAME return GetBraveAccountName(); +#include "../../../../components/os_crypt/keychain_password_mac.mm" +#undef BRAVE_GET_SERVICE_NAME +#undef BRAVE_GET_ACCOUNT_NAME + +namespace { + +std::pair GetServiceAndAccountName() { + std::string service_name, account_name; + base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); + if (command_line->HasSwitch("import-chrome")) { + service_name = std::string("Chrome Safe Storage"); + account_name = std::string("Chrome"); + } else if (command_line->HasSwitch("import-chromium") || + command_line->HasSwitch("import-brave")) { + service_name = std::string("Chromium Safe Storage"); + account_name = std::string("Chromium"); + } else { + service_name = std::string(kBraveDefaultServiceName); + account_name = std::string(kBraveDefaultAccountName); + } + return std::make_pair(service_name, account_name); +} + +KeychainPassword::KeychainNameType& GetBraveServiceName() { + static KeychainNameContainerType service_name( + GetServiceAndAccountName().first); return *service_name; } -// static -KeychainPassword::KeychainNameType& KeychainPassword::GetAccountName() { - static KeychainNameContainerType account_name(kBraveDefaultAccountName); +KeychainPassword::KeychainNameType& GetBraveAccountName() { + static KeychainNameContainerType account_name( + GetServiceAndAccountName().second); return *account_name; } -KeychainPassword::KeychainPassword(const AppleKeychain& keychain) - : KeychainPassword_ChromiumImpl(keychain) {} - -KeychainPassword::~KeychainPassword() = default; +} diff --git a/patches/components-os_crypt-keychain_password_mac.mm.patch b/patches/components-os_crypt-keychain_password_mac.mm.patch index 135404a8bbf3..c44a92376376 100644 --- a/patches/components-os_crypt-keychain_password_mac.mm.patch +++ b/patches/components-os_crypt-keychain_password_mac.mm.patch @@ -1,12 +1,19 @@ diff --git a/components/os_crypt/keychain_password_mac.mm b/components/os_crypt/keychain_password_mac.mm -index 5589310e2e1f41a6a97e77bb57a7a71cd09a18be..dcc59064077b667171e3b49d522e71a2acb76af8 100644 +index 5589310e2e1f41a6a97e77bb57a7a71cd09a18be..af1be3edb2b6135bf93c5fdd42781a92a32ddb9a 100644 --- a/components/os_crypt/keychain_password_mac.mm +++ b/components/os_crypt/keychain_password_mac.mm -@@ -80,6 +80,7 @@ KeychainPassword::KeychainPassword(const AppleKeychain& keychain) - KeychainPassword::~KeychainPassword() = default; +@@ -64,12 +64,14 @@ std::string AddRandomPasswordToKeychain(const AppleKeychain& keychain, - std::string KeychainPassword::GetPassword() const { -+ BRAVE_KEYCHAIN_PASSWORD_GET_PASSWORD - UInt32 password_length = 0; - void* password_data = nullptr; - OSStatus error = keychain_.FindGenericPassword( + // static + KeychainPassword::KeychainNameType& KeychainPassword::GetServiceName() { ++ BRAVE_GET_SERVICE_NAME + static KeychainNameContainerType service_name(kDefaultServiceName); + return *service_name; + } + + // static + KeychainPassword::KeychainNameType& KeychainPassword::GetAccountName() { ++ BRAVE_GET_ACCOUNT_NAME + static KeychainNameContainerType account_name(kDefaultAccountName); + return *account_name; + }