mirror of https://github.com/electron/electron
102 lines
4.7 KiB
Diff
102 lines
4.7 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Samuel Attard <sattard@salesforce.com>
|
|
Date: Thu, 29 Sep 2022 16:58:47 -0700
|
|
Subject: feat: ensure mas builds of the same application can use safestorage
|
|
|
|
This change ensures that MAS builds of applications with an equivilant darwin build that share the same name do not fight over access to the same Safe Storage account.
|
|
|
|
Specifically this changes the account name for app "My App" from "My App" to "My App AppStore" if the app is using a MAS build of Electron.
|
|
|
|
We attempt to migrate the safe storage key from the old account, if that migration succeeds we delete the old key and move on.
|
|
|
|
Existing apps that aren't built for the app store should be unimpacted, there is one edge case where a user uses BOTH an AppStore and a darwin build of the same app only one will keep it's access to the safestorage key as during the migration we delete the old account. This is an acceptable edge case as no one should be actively using two versions of the same app.
|
|
|
|
diff --git a/components/os_crypt/sync/keychain_password_mac.mm b/components/os_crypt/sync/keychain_password_mac.mm
|
|
index e7a65e97b51d93bff864889813317fb7c6ffc846..2b93da301b664f90720d2d2b5407be0985d7f334 100644
|
|
--- a/components/os_crypt/sync/keychain_password_mac.mm
|
|
+++ b/components/os_crypt/sync/keychain_password_mac.mm
|
|
@@ -22,6 +22,12 @@
|
|
using KeychainNameContainerType = const base::NoDestructor<std::string>;
|
|
#endif
|
|
|
|
+#if IS_MAS_BUILD()
|
|
+const char kAccountNameSuffix[] = " App Store Key";
|
|
+#else
|
|
+const char kAccountNameSuffix[] = " Key";
|
|
+#endif
|
|
+
|
|
namespace {
|
|
|
|
// These two strings ARE indeed user facing. But they are used to access
|
|
@@ -81,11 +87,18 @@
|
|
std::string KeychainPassword::GetPassword() const {
|
|
UInt32 password_length = 0;
|
|
void* password_data = nullptr;
|
|
+ KeychainPassword::KeychainNameType service_name = GetServiceName();
|
|
+ KeychainPassword::KeychainNameType account_name = GetAccountName();
|
|
+ const std::string account_name_suffix = kAccountNameSuffix;
|
|
+ const std::string suffixed_account_name = account_name + account_name_suffix;
|
|
+
|
|
+ // We should check if the suffixed account exists first
|
|
OSStatus error = keychain_->FindGenericPassword(
|
|
- GetServiceName().size(), GetServiceName().c_str(),
|
|
- GetAccountName().size(), GetAccountName().c_str(), &password_length,
|
|
+ service_name.size(), service_name.c_str(),
|
|
+ suffixed_account_name.size(), suffixed_account_name.c_str(), &password_length,
|
|
&password_data, /*item=*/nullptr);
|
|
|
|
+ // If it exists we can return it immediately
|
|
if (error == noErr) {
|
|
std::string password =
|
|
std::string(static_cast<char*>(password_data), password_length);
|
|
@@ -93,6 +106,49 @@
|
|
return password;
|
|
}
|
|
|
|
+ // If the error was anything other than "it does not exist" we should error out here
|
|
+ // This normally means the account exists but we were deniged access to it
|
|
+ if (error != errSecItemNotFound) {
|
|
+ OSSTATUS_LOG(ERROR, error) << "Keychain lookup for suffixed key failed";
|
|
+ return std::string();
|
|
+ }
|
|
+
|
|
+ // If the suffixed account didn't exist, we should check if the legacy non-suffixed account
|
|
+ // exists. If it does we can use that key and migrate it to the new account
|
|
+ base::apple::ScopedCFTypeRef<SecKeychainItemRef> item_ref;
|
|
+ error = keychain_->FindGenericPassword(
|
|
+ service_name.size(), service_name.c_str(),
|
|
+ account_name.size(), account_name.c_str(), &password_length,
|
|
+ &password_data, item_ref.InitializeInto());
|
|
+
|
|
+ if (error == noErr) {
|
|
+ std::string password =
|
|
+ std::string(static_cast<char*>(password_data), password_length);
|
|
+
|
|
+ // If we found the legacy account name we should copy it over to
|
|
+ // the new suffixed account
|
|
+ error = keychain_->AddGenericPassword(
|
|
+ service_name.size(), service_name.data(), suffixed_account_name.size(),
|
|
+ suffixed_account_name.data(), password.size(), password_data, NULL);
|
|
+
|
|
+ if (error == noErr) {
|
|
+ // If we successfully made the suffixed account we can delete the old
|
|
+ // account to ensure new apps don't try to use it and run into IAM
|
|
+ // issues
|
|
+ error = keychain_->ItemDelete(item_ref.get());
|
|
+ if (error != noErr) {
|
|
+ OSSTATUS_DLOG(ERROR, error) << "Keychain delete for legacy key failed";
|
|
+ }
|
|
+ } else {
|
|
+ OSSTATUS_DLOG(ERROR, error) << "Keychain add for suffixed key failed";
|
|
+ }
|
|
+
|
|
+ keychain_->ItemFreeContent(password_data);
|
|
+ return password;
|
|
+ }
|
|
+
|
|
+ // If the legacy account name was not found, make a new account in the
|
|
+ // with the suffixed name
|
|
if (error == errSecItemNotFound) {
|
|
std::string password = AddRandomPasswordToKeychain(
|
|
*keychain_, GetServiceName(), GetAccountName());
|