RCORE-2070 Allow setting a security access group for the metadata rea…

…lm keychain (#7552)

Access groups are shared storage for one or more apps on iOS (and other Apple
platforms). Sharing a metadata Realm between apps requires placing the file in
the access group storage and storing the encryption key in the access group's
keychain.

Including the bundle ID in the service name breaks sharing the key between
apps, as different apps will have different bundle IDs. For everything but
un-sandboxed macOS there wasn't actually any reason to include the bundle ID in
the first place, as each app has its own keychain anyway. As such, this
switches back to not including it. On macOS this continues to include the
bundle ID when not using an access group, as otherwise different applications
could conflict with each other. This means that sharing users between macOS
applications will currently only work if an encryption key is explicitly set or
if the applications have sandboxing enabled.

Since this is slightly changing how keys are stored anyway, it also switches to
using unique keys per server app ID rather than always using "metadata" as the
account name.

The keychain code was mostly multiprocess-safe, but there was one race
condition when two apps generated a new key at once which is fixed.

CHANGELOG.md

@@ -2,7 +2,8 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
-* None.
+* Add `SyncClientConfig::security_access_group` which allows specifying the access group to use for the sync metadata Realm's encryption key. Setting this is required when sharing the metadata Realm between apps on Apple platforms ([#7552](https://github.com/realm/realm-core/pull/7552)).
+* When connecting to multiple server apps, a unique encryption key is used for each of the metadata Realms rather than sharing one between them ([#7552](https://github.com/realm/realm-core/pull/7552)).
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

bindgen/spec.yml

@@ -598,6 +598,9 @@ records:
         type: MetadataMode
         default: MetadataMode::Encryption
       custom_encryption_key: std::optional<EncryptionKey>
+      security_access_group:
+        type: std::string
+        default: ""
       logger_factory: Nullable<LoggerFactory>
       log_level:
         type: LoggerLevel

src/realm.h

@@ -3660,6 +3660,8 @@ RLM_API void realm_sync_client_config_set_default_binding_thread_observer(
     realm_sync_client_config_t* config, realm_on_object_store_thread_callback_t on_thread_create,
     realm_on_object_store_thread_callback_t on_thread_destroy, realm_on_object_store_error_callback_t on_error,
     realm_userdata_t user_data, realm_free_userdata_func_t free_userdata);
+RLM_API void realm_sync_client_config_set_security_access_group(realm_sync_client_config_t*,
+                                                                const char*) RLM_API_NOEXCEPT;
 
 RLM_API realm_sync_config_t* realm_sync_config_new(const realm_user_t*, const char* partition_value) RLM_API_NOEXCEPT;
 RLM_API realm_sync_config_t* realm_flx_sync_config_new(const realm_user_t*) RLM_API_NOEXCEPT;

src/realm/object-store/c_api/sync.cpp

@@ -230,6 +230,13 @@ RLM_API void realm_sync_client_config_set_resumption_delay_backoff_multiplier(re
     config->timeouts.reconnect_backoff_info.resumption_delay_backoff_multiplier = multiplier;
 }
 
+RLM_API void realm_sync_client_config_set_security_access_group(realm_sync_client_config_t* config,
+                                                                const char* group) noexcept
+{
+    config->security_access_group = group;
+}
+
+
 /// Register an app local callback handler for bindings interested in registering callbacks before/after
 /// the ObjectStore thread runs for this app. This only works for the default socket provider implementation.
 /// IMPORTANT: If a function is supplied that handles the exception, it must call abort() or cause the

src/realm/object-store/impl/apple/keychain_helper.cpp

@@ -18,8 +18,8 @@
 
 #include <realm/object-store/impl/apple/keychain_helper.hpp>
 
-#include <realm/util/cf_ptr.hpp>
-#include <realm/util/optional.hpp>
+#include <realm/exceptions.hpp>
+#include <realm/util/cf_str.hpp>
 
 #include <Security/Security.h>
 
@@ -28,33 +28,36 @@
 using namespace realm;
 using util::adoptCF;
 using util::CFPtr;
-using util::retainCF;
+using util::string_view_to_cfstring;
 
 namespace {
 
-std::runtime_error keychain_access_exception(int32_t error_code)
+REALM_NORETURN
+REALM_COLD
+void keychain_access_exception(int32_t error_code)
 {
-    return std::runtime_error(util::format("Keychain returned unexpected status code: %1", error_code));
+    if (auto message = adoptCF(SecCopyErrorMessageString(error_code, nullptr))) {
+        if (auto msg = CFStringGetCStringPtr(message.get(), kCFStringEncodingUTF8)) {
+            throw RuntimeError(ErrorCodes::RuntimeError,
+                               util::format("Keychain returned unexpected status code: %1 (%2)", msg, error_code));
+        }
+        auto length = CFStringGetMaximumSizeForEncoding(CFStringGetLength(message.get()), kCFStringEncodingUTF8) + 1;
+        auto buffer = std::make_unique<char[]>(length);
+        if (CFStringGetCString(message.get(), buffer.get(), length, kCFStringEncodingUTF8)) {
+            throw RuntimeError(
+                ErrorCodes::RuntimeError,
+                util::format("Keychain returned unexpected status code: %1 (%2)", buffer.get(), error_code));
+        }
+    }
+    throw RuntimeError(ErrorCodes::RuntimeError,
+                       util::format("Keychain returned unexpected status code: %1", error_code));
 }
 
 constexpr size_t key_size = 64;
-const CFStringRef s_account = CFSTR("metadata");
-const CFStringRef s_legacy_service = CFSTR("io.realm.sync.keychain");
-
-#if !TARGET_IPHONE_SIMULATOR
-CFPtr<CFStringRef> convert_string(const std::string& string)
-{
-    auto result = adoptCF(CFStringCreateWithBytes(nullptr, reinterpret_cast<const UInt8*>(string.data()),
-                                                  string.size(), kCFStringEncodingASCII, false));
-    if (!result) {
-        throw std::bad_alloc();
-    }
-    return result;
-}
-#endif
+const CFStringRef s_legacy_account = CFSTR("metadata");
+const CFStringRef s_service = CFSTR("io.realm.sync.keychain");
 
-CFPtr<CFMutableDictionaryRef> build_search_dictionary(CFStringRef account, CFStringRef service,
-                                                      __unused util::Optional<std::string> group)
+CFPtr<CFMutableDictionaryRef> build_search_dictionary(CFStringRef account, CFStringRef service, CFStringRef group)
 {
     auto d = adoptCF(
         CFDictionaryCreateMutable(nullptr, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
@@ -65,17 +68,20 @@ CFPtr<CFMutableDictionaryRef> build_search_dictionary(CFStringRef account, CFStr
     CFDictionaryAddValue(d.get(), kSecReturnData, kCFBooleanTrue);
     CFDictionaryAddValue(d.get(), kSecAttrAccount, account);
     CFDictionaryAddValue(d.get(), kSecAttrService, service);
-#if !TARGET_IPHONE_SIMULATOR
-    if (group)
-        CFDictionaryAddValue(d.get(), kSecAttrAccessGroup, convert_string(*group).get());
-#endif
+    if (group) {
+        CFDictionaryAddValue(d.get(), kSecAttrAccessGroup, group);
+        if (__builtin_available(macOS 10.15, iOS 13.0, *)) {
+            CFDictionaryAddValue(d.get(), kSecUseDataProtectionKeychain, kCFBooleanTrue);
+        }
+    }
     return d;
 }
 
 /// Get the encryption key for a given service, returning true if it either exists or the keychain is not usable.
-bool get_key(CFStringRef account, CFStringRef service, util::Optional<std::vector<char>>& result)
+bool get_key(CFStringRef account, CFStringRef service, std::string_view group,
+             std::optional<std::vector<char>>& result, bool result_on_error = true)
 {
-    auto search_dictionary = build_search_dictionary(account, service, none);
+    auto search_dictionary = build_search_dictionary(account, service, string_view_to_cfstring(group).get());
     CFDataRef retained_key_data;
     switch (OSStatus status = SecItemCopyMatching(search_dictionary.get(), (CFTypeRef*)&retained_key_data)) {
         case errSecSuccess: {
@@ -94,111 +100,183 @@ bool get_key(CFStringRef account, CFStringRef service, util::Optional<std::vecto
             // Keychain is locked, and user did not enter the password to unlock it.
         case errSecInvalidKeychain:
             // The keychain is corrupted and cannot be used.
+        case errSecNotAvailable:
+            // There are no keychain files.
         case errSecInteractionNotAllowed:
             // We asked for it to not prompt the user and a prompt was needed
-            return true;
+            return result_on_error;
+        case errSecMissingEntitlement:
+            throw InvalidArgument(util::format("Invalid access group '%1'. Make sure that you have added the access "
+                                               "group to your app's Keychain Access Groups Entitlement.",
+                                               group));
         default:
-            throw keychain_access_exception(status);
+            keychain_access_exception(status);
     }
 }
 
-void set_key(util::Optional<std::vector<char>>& key, CFStringRef account, CFStringRef service)
+bool set_key(std::optional<std::vector<char>>& key, CFStringRef account, CFStringRef service, std::string_view group)
 {
+    // key may be nullopt here if the keychain was inaccessible
     if (!key)
-        return;
+        return false;
 
-    auto search_dictionary = build_search_dictionary(account, service, none);
+    auto search_dictionary = build_search_dictionary(account, service, string_view_to_cfstring(group).get());
     CFDictionaryAddValue(search_dictionary.get(), kSecAttrAccessible, kSecAttrAccessibleAfterFirstUnlock);
-    auto key_data = adoptCF(CFDataCreate(nullptr, reinterpret_cast<const UInt8*>(key->data()), key_size));
+    auto key_data = adoptCF(CFDataCreateWithBytesNoCopy(nullptr, reinterpret_cast<const UInt8*>(key->data()),
+                                                        key_size, kCFAllocatorNull));
     if (!key_data)
         throw std::bad_alloc();
 
     CFDictionaryAddValue(search_dictionary.get(), kSecValueData, key_data.get());
     switch (OSStatus status = SecItemAdd(search_dictionary.get(), nullptr)) {
         case errSecSuccess:
-            return;
+            return true;
         case errSecDuplicateItem:
-            // A keychain item already exists but we didn't fine it in get_key(),
-            // meaning that we didn't have permission to access it.
+            // A keychain item already exists but we didn't find it in get_key().
+            // Either someone else created it between when we last checked and
+            // now or we don't have permission to read it. Try to reread the key
+            // and discard the one we just created in case it's the former
+            if (get_key(account, service, group, key, false))
+                return true;
         case errSecMissingEntitlement:
         case errSecUserCanceled:
         case errSecInteractionNotAllowed:
         case errSecInvalidKeychain:
+        case errSecNotAvailable:
             // We were unable to save the key for "expected" reasons, so proceed unencrypted
-            key = none;
-            return;
+            return false;
         default:
             // Unexpected keychain failure happened
-            throw keychain_access_exception(status);
+            keychain_access_exception(status);
     }
 }
 
-void delete_key(CFStringRef account, CFStringRef service)
+void delete_key(CFStringRef account, CFStringRef service, CFStringRef group)
 {
-    auto search_dictionary = build_search_dictionary(account, service, none);
+    auto search_dictionary = build_search_dictionary(account, service, group);
     auto status = SecItemDelete(search_dictionary.get());
     REALM_ASSERT(status == errSecSuccess || status == errSecItemNotFound);
 }
 
-CFPtr<CFStringRef> get_service_name(bool& have_bundle_id)
+CFPtr<CFStringRef> bundle_service()
 {
-    CFPtr<CFStringRef> service;
     if (CFStringRef bundle_id = CFBundleGetIdentifier(CFBundleGetMainBundle())) {
-        service = adoptCF(CFStringCreateWithFormat(NULL, NULL, CFSTR("%@ - Realm Sync Metadata Key"), bundle_id));
-        have_bundle_id = true;
-    }
-    else {
-        service = retainCF(s_legacy_service);
-        have_bundle_id = false;
+        return adoptCF(CFStringCreateWithFormat(nullptr, nullptr, CFSTR("%@ - Realm Sync Metadata Key"), bundle_id));
     }
-    return service;
+    return CFPtr<CFStringRef>{};
 }
 
 } // anonymous namespace
 
 namespace realm::keychain {
 
-util::Optional<std::vector<char>> get_existing_metadata_realm_key()
+std::optional<std::vector<char>> get_existing_metadata_realm_key(std::string_view app_id,
+                                                                 std::string_view access_group)
 {
-    bool have_bundle_id = false;
-    CFPtr<CFStringRef> service = get_service_name(have_bundle_id);
+    auto cf_app_id = string_view_to_cfstring(app_id);
+    std::optional<std::vector<char>> key;
 
-    // Try retrieving the existing key.
-    util::Optional<std::vector<char>> key;
-    if (get_key(s_account, service.get(), key)) {
+    // If we have a security access groups then keys are stored the same way
+    // everywhere and we don't have any legacy storage methods to handle, so
+    // we just either have a key or we don't.
+    if (access_group.size()) {
+        get_key(cf_app_id.get(), s_service, access_group, key);
         return key;
     }
 
-    if (have_bundle_id) {
-        // See if there's a key stored using the legacy shared keychain item.
-        if (get_key(s_account, s_legacy_service, key)) {
-            // If so, copy it to the per-app keychain item before returning it.
-            set_key(key, s_account, service.get());
+    // When we don't have an access group we check a whole bunch of things because
+    // there's been a variety of ways that we've stored metadata keys over the years.
+    // If we find a key stored in a non-preferred way we copy it to the preferred
+    // location before returning it.
+    //
+    // The original location was (account: "metadata", service: "io.realm.sync.keychain").
+    // For processes with a bundle ID, we then switched to (account: "metadata",
+    // service: "$bundleId - Realm Sync Metadata Key")
+    // The current preferred location on non-macOS (account: appId, service: "io.realm.sync.keychain"),
+    // and on macOS is (account: appId, service: "$bundleId - Realm Sync Metadata Key").
+    //
+    // On everything but macOS the keychain is scoped to the app, so there's no
+    // need to include the bundle ID. On macOS it's user-wide, and we want each
+    // application using Realm to have separate state. Using multiple server apps
+    // in one client is unusual, but when it's done we want each metadata realm to
+    // have a separate key.
+
+#if TARGET_OS_OSX
+    if (auto service = bundle_service()) {
+        if (get_key(cf_app_id.get(), service.get(), {}, key))
+            return key;
+        if (get_key(s_legacy_account, service.get(), {}, key)) {
+            set_key(key, cf_app_id.get(), service.get(), {});
+            return key;
+        }
+        if (get_key(s_legacy_account, s_service, {}, key)) {
+            set_key(key, cf_app_id.get(), service.get(), {});
+            return key;
+        }
+    }
+    else {
+        if (get_key(cf_app_id.get(), s_service, {}, key))
+            return key;
+        if (get_key(s_legacy_account, s_service, {}, key)) {
+            set_key(key, cf_app_id.get(), s_service, {});
+            return key;
+        }
+    }
+#else
+    if (get_key(cf_app_id, s_service, {}, key))
+        return key;
+    if (auto service = bundle_service()) {
+        if (get_key(cf_app_id, service, {}, key)) {
+            set_key(key, cf_app_id, s_service, {});
             return key;
         }
     }
-    return util::none;
+    if (get_key(s_legacy_account, s_service, {}, key)) {
+        set_key(key, cf_app_id, s_service, {});
+        return key;
+    }
+#endif
+
+    return key;
 }
 
-util::Optional<std::vector<char>> create_new_metadata_realm_key()
+std::optional<std::vector<char>> create_new_metadata_realm_key(std::string_view app_id, std::string_view access_group)
 {
-    bool have_bundle_id = false;
-    CFPtr<CFStringRef> service = get_service_name(have_bundle_id);
-
-    util::Optional<std::vector<char>> key;
+    auto cf_app_id = string_view_to_cfstring(app_id);
+    std::optional<std::vector<char>> key;
     key.emplace(key_size);
     arc4random_buf(key->data(), key_size);
-    set_key(key, s_account, service.get());
+
+    // See above for why macOS is different
+#if TARGET_OS_OSX
+    if (!access_group.size()) {
+        if (auto service = bundle_service()) {
+            if (!set_key(key, cf_app_id.get(), service.get(), {}))
+                key.reset();
+            return key;
+        }
+    }
+#endif
+
+    // If we're unable to save the newly created key, clear it and proceed unencrypted
+    if (!set_key(key, cf_app_id.get(), s_service, access_group))
+        key.reset();
     return key;
 }
 
-void delete_metadata_realm_encryption_key()
+void delete_metadata_realm_encryption_key(std::string_view app_id, std::string_view access_group)
 {
-    delete_key(s_account, s_legacy_service);
-    if (CFStringRef bundle_id = CFBundleGetIdentifier(CFBundleGetMainBundle())) {
-        auto service =
-            adoptCF(CFStringCreateWithFormat(NULL, NULL, CFSTR("%@ - Realm Sync Metadata Key"), bundle_id));
-        delete_key(s_account, service.get());
+    auto cf_app_id = string_view_to_cfstring(app_id);
+    if (access_group.size()) {
+        delete_key(cf_app_id.get(), s_service, string_view_to_cfstring(access_group).get());
+        return;
+    }
+
+    delete_key(cf_app_id.get(), s_service, {});
+    delete_key(s_legacy_account, s_service, {});
+    if (auto service = bundle_service()) {
+        delete_key(cf_app_id.get(), service.get(), {});
+        delete_key(s_legacy_account, service.get(), {});
     }
 }