fix: complete session persistence across browser updates (macOS)

- Extend keychain_password_mac.mm patch to set kSecAttrAccessGroup on
  keychain writes and reads, ensuring items are stored under the shared
  BrowserOS access group rather than tied to a specific binary signature

- Pin the designated requirement in macos.py to the Team ID via
  certificate leaf[subject.OU] instead of wildcard /* exists */ checks,
  so Keychain ACLs survive across builds with different binary hashes

- Add browseros_keychain_recovery.mm that runs on startup to detect
  broken keychain access (errSecAuthFailed), prompt the user for access,
  and migrate the item to the BrowserOS access group

- Add broadenKeychainACLBeforeUpdate to sparkle_glue.mm that updates
  the keychain item's access group right before Sparkle replaces the
  app bundle, so the new binary can read it immediately

Resolves: TKT-666, TKT-668, TKT-669, TKT-670
This commit is contained in:
Dani Akash
2026-03-26 19:06:49 +05:30
parent 416782b0ac
commit 509bb2a6ab
6 changed files with 344 additions and 9 deletions

View File

@@ -126,7 +126,9 @@ class MacOSSignModule(CommandModule):
run_command(["xattr", "-cs", str(app_path)])
def _sign_all_components(self, app_path: Path, certificate_name: str, ctx: Context) -> None:
if not sign_all_components(app_path, certificate_name, ctx.root_dir, ctx):
_, env_vars = check_environment(ctx.env)
team_id = env_vars.get("team_id", "")
if not sign_all_components(app_path, certificate_name, ctx.root_dir, ctx, team_id=team_id):
raise RuntimeError("Failed to sign all components")
def _verify_signature(self, app_path: Path) -> None:
@@ -413,6 +415,7 @@ def sign_all_components(
certificate_name: str,
root_dir: Path,
ctx: Optional[Context] = None,
team_id: str = "",
) -> bool:
"""Sign all components in the correct order (bottom-up)"""
log_info("🔍 Discovering components to sign...")
@@ -547,11 +550,23 @@ def sign_all_components(
# 8. Finally sign the app bundle
log_info("\n🔏 Signing application bundle...")
requirements = (
'=designated => identifier "com.browseros.BrowserOS" and '
"anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and "
"certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */"
)
# Pin the designated requirement to the Team ID so that Keychain ACLs
# survive across builds with different binary hashes.
if team_id:
requirements = (
f'=designated => identifier "com.browseros.BrowserOS" and '
f"anchor apple generic and "
f"certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and "
f"certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */ and "
f'certificate leaf[subject.OU] = "{team_id}"'
)
else:
log_warning("No team_id provided — using wildcard designated requirement")
requirements = (
'=designated => identifier "com.browseros.BrowserOS" and '
"anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and "
"certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */"
)
# Try multiple locations for app entitlements
entitlements = None
@@ -798,7 +813,8 @@ def sign_app(ctx: Context, create_dmg: bool = True) -> bool:
# Sign all components
if not sign_all_components(
app_path, env_vars["certificate_name"], ctx.root_dir, ctx
app_path, env_vars["certificate_name"], ctx.root_dir, ctx,
team_id=env_vars.get("team_id", ""),
):
return False

View File

@@ -21,6 +21,24 @@ index 0000000000000..24fdd618e6a71
+ ]
+}
+
+if (is_mac) {
+ source_set("keychain_recovery") {
+ sources = [
+ "browseros_keychain_recovery.h",
+ "browseros_keychain_recovery.mm",
+ ]
+
+ deps = [
+ "//base",
+ ]
+
+ frameworks = [
+ "Foundation.framework",
+ "Security.framework",
+ ]
+ }
+}
+
+source_set("prefs") {
+ sources = [
+ "browseros_prefs.cc",

View File

@@ -0,0 +1,28 @@
diff --git a/chrome/browser/browseros/core/browseros_keychain_recovery.h b/chrome/browser/browseros/core/browseros_keychain_recovery.h
new file mode 100644
index 0000000000000..0000000000001
--- /dev/null
+++ b/chrome/browser/browseros/core/browseros_keychain_recovery.h
@@ -0,0 +1,24 @@
+// Copyright 2026 BrowserOS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_BROWSEROS_CORE_BROWSEROS_KEYCHAIN_RECOVERY_H_
+#define CHROME_BROWSER_BROWSEROS_CORE_BROWSEROS_KEYCHAIN_RECOVERY_H_
+
+namespace browseros {
+
+// Checks whether the BrowserOS Safe Storage keychain item is accessible.
+// If access is denied (e.g. due to a signing identity change after an update),
+// attempts interactive recovery by prompting the user. On success, migrates the
+// keychain item to use the BrowserOS access group so future updates don't break
+// access.
+//
+// Must be called early in browser startup, before any cookie or password access
+// triggers os_crypt to read the keychain.
+void MaybeMigrateKeychainAccess();
+
+} // namespace browseros
+
+#endif // CHROME_BROWSER_BROWSEROS_CORE_BROWSEROS_KEYCHAIN_RECOVERY_H_

View File

@@ -0,0 +1,137 @@
diff --git a/chrome/browser/browseros/core/browseros_keychain_recovery.mm b/chrome/browser/browseros/core/browseros_keychain_recovery.mm
new file mode 100644
index 0000000000000..0000000000001
--- /dev/null
+++ b/chrome/browser/browseros/core/browseros_keychain_recovery.mm
@@ -0,0 +1,120 @@
+// Copyright 2026 BrowserOS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/browseros/core/browseros_keychain_recovery.h"
+
+#import <Foundation/Foundation.h>
+#import <Security/Security.h>
+#import <Security/SecCode.h>
+
+#include "base/logging.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace browseros {
+
+namespace {
+
+NSString* GetTeamIdentifier() {
+ SecCodeRef code = NULL;
+ if (SecCodeCopySelf(kSecCSDefaultFlags, &code) != errSecSuccess || !code) {
+ return nil;
+ }
+ CFDictionaryRef info = NULL;
+ OSStatus status =
+ SecCodeCopySigningInformation(code, kSecCSDefaultFlags, &info);
+ CFRelease(code);
+ if (status != errSecSuccess || !info) {
+ return nil;
+ }
+ NSString* teamID = [(__bridge NSDictionary*)info
+ objectForKey:(__bridge NSString*)kSecCodeInfoTeamIdentifier];
+ NSString* result = [teamID copy];
+ CFRelease(info);
+ return result;
+}
+
+NSString* GetAccessGroup() {
+ NSString* teamID = GetTeamIdentifier();
+ if (!teamID || teamID.length == 0) {
+ return nil;
+ }
+ return [NSString stringWithFormat:@"%@.com.browseros", teamID];
+}
+
+} // namespace
+
+void MaybeMigrateKeychainAccess() {
+ @autoreleasepool {
+ NSDictionary* query = @{
+ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService : @"BrowserOS Safe Storage",
+ (__bridge id)kSecAttrAccount : @"BrowserOS",
+ (__bridge id)kSecReturnData : @YES,
+ };
+
+ CFTypeRef result = NULL;
+ OSStatus status =
+ SecItemCopyMatching((__bridge CFDictionaryRef)query, &result);
+
+ if (status == errSecSuccess) {
+ // Access works. Migrate the item to the access group if needed.
+ if (result) {
+ CFRelease(result);
+ }
+ NSString* group = GetAccessGroup();
+ if (group) {
+ NSDictionary* updateQuery = @{
+ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService : @"BrowserOS Safe Storage",
+ (__bridge id)kSecAttrAccount : @"BrowserOS",
+ };
+ NSDictionary* update = @{
+ (__bridge id)kSecAttrAccessGroup : group,
+ };
+ SecItemUpdate((__bridge CFDictionaryRef)updateQuery,
+ (__bridge CFDictionaryRef)update);
+ }
+ LOG(INFO) << "browseros: Keychain access OK";
+ return;
+ }
+
+ if (status == errSecItemNotFound) {
+ LOG(INFO)
+ << "browseros: No keychain item found, will be created on first use";
+ return;
+ }
+
+ // errSecAuthFailed, errSecInteractionNotAllowed, etc.
+ // The item exists but we can't access it — signing identity mismatch.
+ LOG(WARNING) << "browseros: Keychain access denied (status=" << status
+ << "), attempting interactive recovery";
+
+ SecKeychainSetUserInteractionAllowed(TRUE);
+
+ result = NULL;
+ status = SecItemCopyMatching((__bridge CFDictionaryRef)query, &result);
+
+ if (status == errSecSuccess) {
+ LOG(INFO) << "browseros: Keychain access recovered via user interaction";
+ if (result) {
+ CFRelease(result);
+ }
+ // Migrate to access group now that we have access.
+ NSString* group = GetAccessGroup();
+ if (group) {
+ NSDictionary* updateQuery = @{
+ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService : @"BrowserOS Safe Storage",
+ (__bridge id)kSecAttrAccount : @"BrowserOS",
+ };
+ NSDictionary* update = @{
+ (__bridge id)kSecAttrAccessGroup : group,
+ };
+ SecItemUpdate((__bridge CFDictionaryRef)updateQuery,
+ (__bridge CFDictionaryRef)update);
+ }
+ return;
+ }
+
+ LOG(ERROR) << "browseros: Keychain recovery failed (status=" << status
+ << "). User will lose encrypted data.";
+ if (result) {
+ CFRelease(result);
+ }
+ }
+}
+
+} // namespace browseros

View File

@@ -13,6 +13,9 @@ index 0000000000000..25c4843095e4f
+#include <sys/mount.h>
+#include <sys/stat.h>
+
+#import <Security/Security.h>
+#import <Security/SecCode.h>
+
+#include "base/apple/bundle_locations.h"
+#include "base/command_line.h"
+#include "base/logging.h"
@@ -254,6 +257,11 @@ index 0000000000000..25c4843095e4f
+
+- (void)showReadyToInstallAndRelaunch:(void (^)(SPUUserUpdateChoice))reply {
+ VLOG(1) << "Sparkle: Ready to install and relaunch";
+
+ // Broaden keychain ACL before Sparkle replaces the app bundle,
+ // so the new binary can access the existing encryption key.
+ [self.glue broadenKeychainACLBeforeUpdate];
+
+ self.installReplyBlock = reply;
+ [self.glue setInternalStatus:SparkleStatusReadyToInstall];
+}
@@ -536,6 +544,55 @@ index 0000000000000..25c4843095e4f
+ }
+}
+
+#pragma mark - Keychain ACL Migration
+
+- (void)broadenKeychainACLBeforeUpdate {
+ // Before Sparkle replaces the app bundle, update the keychain item's
+ // access group so the new binary (signed by the same Team ID) can read it.
+ SecCodeRef code = NULL;
+ if (SecCodeCopySelf(kSecCSDefaultFlags, &code) != errSecSuccess || !code) {
+ LOG(WARNING) << "Sparkle: Could not get code identity for keychain migration";
+ return;
+ }
+ CFDictionaryRef info = NULL;
+ OSStatus status = SecCodeCopySigningInformation(code, kSecCSDefaultFlags, &info);
+ CFRelease(code);
+ if (status != errSecSuccess || !info) {
+ LOG(WARNING) << "Sparkle: Could not get signing info for keychain migration";
+ return;
+ }
+ NSString* teamID = [(__bridge NSDictionary*)info
+ objectForKey:(__bridge NSString*)kSecCodeInfoTeamIdentifier];
+ CFRelease(info);
+ if (!teamID || teamID.length == 0) {
+ LOG(WARNING) << "Sparkle: No Team ID found — skipping keychain migration";
+ return;
+ }
+
+ NSString* group = [NSString stringWithFormat:@"%@.com.browseros", teamID];
+
+ NSDictionary* query = @{
+ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService : @"BrowserOS Safe Storage",
+ (__bridge id)kSecAttrAccount : @"BrowserOS",
+ };
+ NSDictionary* update = @{
+ (__bridge id)kSecAttrAccessGroup : group,
+ };
+
+ status = SecItemUpdate((__bridge CFDictionaryRef)query,
+ (__bridge CFDictionaryRef)update);
+
+ if (status == errSecSuccess) {
+ VLOG(1) << "Sparkle: Keychain ACL updated with access group "
+ << base::SysNSStringToUTF8(group);
+ } else if (status == errSecItemNotFound) {
+ VLOG(1) << "Sparkle: No keychain item to migrate";
+ } else {
+ LOG(WARNING) << "Sparkle: Failed to update keychain ACL (status=" << status << ")";
+ }
+}
+
+#pragma mark - SPUUpdaterDelegate
+
+- (nullable NSString*)feedURLStringForUpdater:(SPUUpdater*)updater {

View File

@@ -2,7 +2,17 @@ diff --git a/components/os_crypt/common/keychain_password_mac.mm b/components/os
index caa0e420956a3..d60a67a8bacb7 100644
--- a/components/os_crypt/common/keychain_password_mac.mm
+++ b/components/os_crypt/common/keychain_password_mac.mm
@@ -35,8 +35,9 @@
@@ -8,6 +8,9 @@
#import <Security/Security.h>
+// BrowserOS: needed for SecCodeCopySelf / SecCodeCopySigningInformation
+#import <Security/SecCode.h>
+
#include <atomic>
#include "base/apple/ossstatus_logging.h"
@@ -35,8 +38,46 @@
const char kDefaultServiceName[] = "Chrome Safe Storage";
const char kDefaultAccountName[] = "Chrome";
#else
@@ -12,5 +22,74 @@ index caa0e420956a3..d60a67a8bacb7 100644
+const char kDefaultServiceName[] = "BrowserOS Safe Storage";
+const char kDefaultAccountName[] = "BrowserOS";
#endif
+// BrowserOS: Get the Team ID from the running binary's code signature
+// and construct the keychain access group (e.g. "ABC123DEF4.com.browseros").
+NSString* GetBrowserOSAccessGroup() {
+ static NSString* cachedGroup = nil;
+ static dispatch_once_t onceToken;
+ dispatch_once(&onceToken, ^{
+ SecCodeRef code = NULL;
+ if (SecCodeCopySelf(kSecCSDefaultFlags, &code) != errSecSuccess || !code) {
+ return;
+ }
+ CFDictionaryRef info = NULL;
+ if (SecCodeCopySigningInformation(code, kSecCSDefaultFlags, &info) == errSecSuccess && info) {
+ NSString* teamID = [(__bridge NSDictionary*)info
+ objectForKey:(__bridge NSString*)kSecCodeInfoTeamIdentifier];
+ if (teamID.length > 0) {
+ cachedGroup = [[NSString alloc] initWithFormat:@"%@.com.browseros", teamID];
+ }
+ CFRelease(info);
+ }
+ CFRelease(code);
+ });
+ return cachedGroup;
+}
+
// These values are persisted to logs. Entries should not be renumbered and
+
+@@ -58,6 +99,12 @@
+ OSStatus error = keychain.AddGenericPassword(service_name, account_name,
+ base::as_byte_span(password));
+
++ // BrowserOS: update the newly created item to use our access group
++ NSString* group = GetBrowserOSAccessGroup();
++ if (group && error == noErr) {
++ NSDictionary* query = @{
++ (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
++ (__bridge id)kSecAttrService: @(service_name.c_str()),
++ (__bridge id)kSecAttrAccount: @(account_name.c_str()),
++ };
++ NSDictionary* update = @{
++ (__bridge id)kSecAttrAccessGroup: group,
++ };
++ SecItemUpdate((__bridge CFDictionaryRef)query,
++ (__bridge CFDictionaryRef)update);
++ }
++
if (error != noErr) {
OSSSTATUS_DLOG(ERROR, error) << "Keychain add failed";
return base::unexpected(error);
@@ -73,6 +130,14 @@
auto password = keychain.FindGenericPassword(service_name, account_name);
if (password.has_value()) {
+ // BrowserOS: ensure existing items have the correct access group.
+ // This migrates items created before the access group was added.
+ NSString* group = GetBrowserOSAccessGroup();
+ if (group) {
+ NSDictionary* query = @{
+ (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService: @(service_name.c_str()),
+ (__bridge id)kSecAttrAccount: @(account_name.c_str()),
+ };
+ NSDictionary* update = @{
+ (__bridge id)kSecAttrAccessGroup: group,
+ };
+ // Best-effort — ignore errors (item may already have the group).
+ SecItemUpdate((__bridge CFDictionaryRef)query,
+ (__bridge CFDictionaryRef)update);
+ }
uma_result = FindGenericPasswordResult::kPasswordFound;
return std::string(base::as_string_view(*password));