Compare commits

...

4 Commits

Author SHA1 Message Date
Dani Akash
1c101578dc fix: add missing forward declaration and downgrade happy-path logs
- Add broadenKeychainACLBeforeUpdate to SparkleGlue forward declaration
  so BrowserOSUserDriver can call it without -Werror compile failure
- Downgrade LOG(INFO) to VLOG(1) for normal keychain access paths that
  fire on every startup
2026-03-26 19:52:37 +05:30
Dani Akash
346d82bdf0 fix: use legacy SecKeychain API for interactive keychain recovery
SecKeychainSetUserInteractionAllowed only affects the legacy SecKeychain*
APIs, not SecItemCopyMatching. Replace the recovery path with
SecKeychainFindGenericPassword which actually triggers the macOS system
dialog asking the user to grant keychain access and updates the item's
ACL to include the new binary.
2026-03-26 19:22:01 +05:30
Dani Akash
509bb2a6ab 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
2026-03-26 19:06:49 +05:30
Dani Akash
416782b0ac fix: add keychain-access-groups entitlement to preserve sessions across updates
Add $(TeamIdentifierPrefix)com.browseros to keychain-access-groups in
app-entitlements.plist so that Keychain ACLs are tied to the Team ID
rather than the exact binary signature. This prevents users from losing
all sessions/cookies when BrowserOS is updated via Sparkle, since the
new binary's code signature no longer needs to exactly match the old one.
2026-03-26 14:46:41 +05:30
7 changed files with 359 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,147 @@
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);
+ }
+ VLOG(1) << "browseros: Keychain access OK";
+ return;
+ }
+
+ if (status == errSecItemNotFound) {
+ VLOG(1)
+ << "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 via legacy API";
+
+ // Use the legacy SecKeychainFindGenericPassword API for recovery because
+ // it triggers the macOS system dialog ("BrowserOS wants to use your
+ // keychain") which lets the user grant access and adds this binary to
+ // the item's ACL. The modern SecItemCopyMatching API does not prompt
+ // for ACL updates — it just fails silently on mismatch.
+ const char* serviceName = "BrowserOS Safe Storage";
+ const char* accountName = "BrowserOS";
+ UInt32 passwordLength = 0;
+ void* passwordData = NULL;
+
+ status = SecKeychainFindGenericPassword(
+ NULL, // default keychain
+ static_cast<UInt32>(strlen(serviceName)), serviceName,
+ static_cast<UInt32>(strlen(accountName)), accountName,
+ &passwordLength, &passwordData,
+ NULL);
+
+ if (status == errSecSuccess) {
+ LOG(INFO) << "browseros: Keychain access recovered via user interaction";
+ if (passwordData) {
+ SecKeychainItemFreeContent(NULL, passwordData);
+ }
+ // Migrate to access group now that the binary is in the ACL.
+ // SecItemUpdate will work because the user just granted 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.";
+ }
+}
+
+} // 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"
@@ -113,6 +116,7 @@ index 0000000000000..25c4843095e4f
+- (void)setInternalStatus:(SparkleStatus)status
+ errorMessage:(nullable NSString*)errorMessage;
+- (void)notifyProgress:(SparkleProgress*)progress;
+- (void)broadenKeychainACLBeforeUpdate;
+@end
+
+#pragma mark - BrowserOSUserDriver
@@ -254,6 +258,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 +545,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));

View File

@@ -16,5 +16,9 @@
<true/>
<key>com.apple.security.personal-information.photos-library</key>
<true/>
<key>keychain-access-groups</key>
<array>
<string>$(TeamIdentifierPrefix)com.browseros</string>
</array>
</dict>
</plist>