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
brave · Sep 15, 2021 · adf55bb · adf55bb
1 parent c2c022b
commit adf55bb
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 69 deletions.
diff --git a/chromium_src/components/os_crypt/keychain_password_mac.h b/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
@@ -5,48 +5,55 @@
 
 #include "components/os_crypt/keychain_password_mac.h"
 
-#include <memory>
+#include <utility>
 
 #include "base/command_line.h"
 
-#define BRAVE_KEYCHAIN_PASSWORD_GET_PASSWORD                                \
-  std::unique_ptr<std::string> service_name, account_name;                  \
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); \
-  if (command_line->HasSwitch("import-chrome")) {                           \
-    service_name = std::make_unique<std::string>("Chrome Safe Storage");    \
-    account_name = std::make_unique<std::string>("Chrome");                 \
-  } else if (command_line->HasSwitch("import-chromium") ||                  \
-             command_line->HasSwitch("import-brave")) {                     \
-    service_name = std::make_unique<std::string>("Chromium Safe Storage");  \
-    account_name = std::make_unique<std::string>("Chromium");               \
-  } else {                                                                  \
-    service_name = std::make_unique<std::string>(                           \
-        ::KeychainPassword::GetServiceName().c_str());                      \
-    account_name = std::make_unique<std::string>(                           \
-        ::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<std::string, std::string> 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
@@ -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;
+ }