fix: ensure mas builds of the same application can use safestorage (#35864)
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.
This commit is contained in:
Samuel Attard
GitHub
parent
12eade752d
commit
2cda1443fc
2 changed files with 106 additions and 0 deletions
|
|
@ -120,3 +120,4 @@ fix_crash_loading_non-standard_schemes_in_iframes.patch
|
|||
disable_optimization_guide_for_preconnect_feature.patch
|
||||
fix_return_v8_value_from_localframe_requestexecutescript.patch
|
||||
create_browser_v8_snapshot_file_name_fuse.patch
|
||||
feat_ensure_mas_builds_of_the_same_application_can_use_safestorage.patch
|
||||
|
|
|
|||
|
|
@ -0,0 +1,105 @@
|
|||
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/keychain_password_mac.mm b/components/os_crypt/keychain_password_mac.mm
|
||||
index 5589310e2e1f41a6a97e77bb57a7a71cd09a18be..ad81ca6726e265b43f30f12ee3f5a22f943298ac 100644
|
||||
--- a/components/os_crypt/keychain_password_mac.mm
|
||||
+++ b/components/os_crypt/keychain_password_mac.mm
|
||||
@@ -22,6 +22,12 @@
|
||||
using KeychainNameContainerType = const base::NoDestructor<std::string>;
|
||||
#endif
|
||||
|
||||
+#if defined(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
|
||||
@@ -82,11 +88,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, nullptr);
|
||||
|
||||
+ // If it exists we can return it immediately
|
||||
if (error == noErr) {
|
||||
std::string password =
|
||||
std::string(static_cast<char*>(password_data), password_length);
|
||||
@@ -94,9 +107,52 @@
|
||||
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::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());
|
||||
+ keychain_, service_name, suffixed_account_name);
|
||||
return password;
|
||||
}
|
||||
|
||||
Loading…
Reference in a new issue