Skip to content
This repository has been archived by the owner on Nov 8, 2021. It is now read-only.

Store sync metadata keychain info on per-app basis #521

Merged
merged 8 commits into from
Aug 28, 2017
Merged

Store sync metadata keychain info on per-app basis #521

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4236715
Allow the keychain service to be customized when configuring sync met…
austinzheng Aug 21, 2017
9bafa2a
Make object store get bundle identifier
austinzheng Aug 21, 2017
9828bee
Remove irrelevant message
austinzheng Aug 21, 2017
da5d3ed
Address feedback
austinzheng Aug 24, 2017
9a917a7
Address feedback
austinzheng Aug 24, 2017
11a40be
Fix issue with `retainCF` and null pointers
austinzheng Aug 26, 2017
1688498
Remove unnecessary retain/release
austinzheng Aug 28, 2017
d9c727a
Address feedback
austinzheng Aug 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 65 additions & 34 deletions src/impl/apple/keychain_helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,17 @@

using realm::util::CFPtr;
using realm::util::adoptCF;
using realm::util::retainCF;

namespace realm {
namespace keychain {

KeychainAccessException::KeychainAccessException(int32_t error_code)
: std::runtime_error(util::format("Keychain returned unexpected status code: %1", error_code)) { }

CFPtr<CFStringRef> convert_string(const std::string& string);
CFPtr<CFMutableDictionaryRef> build_search_dictionary(const std::string& account, const std::string& service,
util::Optional<std::string> group);
namespace {

constexpr size_t key_size = 64;

CFPtr<CFStringRef> convert_string(const std::string& string)
{
Expand All @@ -50,66 +51,96 @@ CFPtr<CFStringRef> convert_string(const std::string& string)
return result;
}

CFPtr<CFMutableDictionaryRef> build_search_dictionary(const std::string& account, const std::string& service,
CFPtr<CFMutableDictionaryRef> build_search_dictionary(CFStringRef account, CFStringRef service,
__unused util::Optional<std::string> group)
{
auto d = adoptCF(CFDictionaryCreateMutable(nullptr, 0, &kCFTypeDictionaryKeyCallBacks,
&kCFTypeDictionaryValueCallBacks));
if (!d) {
if (!d)
throw std::bad_alloc();
}

CFDictionaryAddValue(d.get(), kSecClass, kSecClassGenericPassword);
CFDictionaryAddValue(d.get(), kSecReturnData, kCFBooleanTrue);
CFDictionaryAddValue(d.get(), kSecAttrAccessible, kSecAttrAccessibleAlways);
CFDictionaryAddValue(d.get(), kSecAttrAccount, convert_string(account).get());
CFDictionaryAddValue(d.get(), kSecAttrService, convert_string(service).get());
CFDictionaryAddValue(d.get(), kSecAttrAccount, account);
CFDictionaryAddValue(d.get(), kSecAttrService, service);
#if TARGET_IPHONE_SIMULATOR
#else
if (group) {
if (group)
CFDictionaryAddValue(d.get(), kSecAttrAccessGroup, convert_string(*group).get());
}
#endif
return d;
}

std::vector<char> metadata_realm_encryption_key()
/// Get the encryption key for a given service, returning it only if it exists.
util::Optional<std::vector<char>> get_key(CFStringRef account, CFStringRef service)
{
const size_t key_size = 64;
const std::string service = "io.realm.sync.keychain";
const std::string account = "metadata";

auto search_dictionary = build_search_dictionary(account, service, none);
CFDataRef retained_key_data;
if (OSStatus status = SecItemCopyMatching(search_dictionary.get(), (CFTypeRef *)&retained_key_data)) {
if (status != errSecItemNotFound) {
if (status != errSecItemNotFound)
throw KeychainAccessException(status);
}

// Key was not found. Generate a new key, store it, and return it.
std::vector<char> key(key_size);
arc4random_buf(key.data(), key_size);
auto key_data = adoptCF(CFDataCreate(nullptr, reinterpret_cast<const UInt8 *>(key.data()), key_size));
if (!key_data) {
throw std::bad_alloc();
}

CFDictionaryAddValue(search_dictionary.get(), kSecValueData, key_data.get());
if (OSStatus status = SecItemAdd(search_dictionary.get(), nullptr)) {
throw KeychainAccessException(status);
}

return key;
// Key was not found.
return none;
}
CFPtr<CFDataRef> key_data = adoptCF(retained_key_data);

// Key was previously stored. Extract it.
if (key_size != CFDataGetLength(key_data.get())) {
CFPtr<CFDataRef> key_data = adoptCF(retained_key_data);
if (key_size != CFDataGetLength(key_data.get()))
throw std::runtime_error("Password stored in keychain was not expected size.");
}

auto key_bytes = reinterpret_cast<const char *>(CFDataGetBytePtr(key_data.get()));
return std::vector<char>(key_bytes, key_bytes + key_size);
}

void set_key(const std::vector<char>& key, CFStringRef account, CFStringRef service)
{
auto search_dictionary = build_search_dictionary(account, service, none);
auto key_data = adoptCF(CFDataCreate(nullptr, reinterpret_cast<const UInt8 *>(key.data()), key_size));
if (!key_data)
throw std::bad_alloc();

CFDictionaryAddValue(search_dictionary.get(), kSecValueData, key_data.get());
if (OSStatus status = SecItemAdd(search_dictionary.get(), nullptr))
throw KeychainAccessException(status);
}

} // anonymous namespace

std::vector<char> metadata_realm_encryption_key(bool check_legacy_service)
{
CFStringRef account = CFSTR("metadata");
CFStringRef default_service = CFSTR("io.realm.sync.keychain");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think legacy_service would better reflect how the relationship between this and the check_legacy_service argument.


CFPtr<CFStringRef> managed_service;
if (auto bundle_id = retainCF(CFBundleGetIdentifier(CFBundleGetMainBundle()))) {
managed_service = adoptCF(CFStringCreateWithFormat(NULL, NULL,
CFSTR("%@ - Realm Sync Metadata Key"),
bundle_id.get()));
} else {
managed_service = retainCF(default_service);
check_legacy_service = false;
}

// Try retrieving the key.
CFStringRef service = managed_service.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be inclined to ditch this local and just add the .get() calls in the handful of places it is necessary. It's effectively free to call, and would avoid the need for the slightly odd managed_ prefix on the variable.

if (auto existing_key = get_key(account, service)) {
return *existing_key;
} else if (check_legacy_service) {
// See if there's a key in the old shared keychain.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"See if there's a key stored using the legacy shared keychain item"?

if (auto existing_legacy_key = get_key(account, default_service)) {
// If so, copy it to the per-app keychain before returning it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keychain isn't per application, the item name is.

set_key(*existing_legacy_key, account, service);
return *existing_legacy_key;
}
}
// Make a completely new key.
std::vector<char> key(key_size);
arc4random_buf(key.data(), key_size);
set_key(key, account, service);
return key;
}

} // keychain
} // realm
2 changes: 1 addition & 1 deletion src/impl/apple/keychain_helper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
namespace realm {
namespace keychain {

std::vector<char> metadata_realm_encryption_key();
std::vector<char> metadata_realm_encryption_key(bool check_legacy_service);

class KeychainAccessException : public std::runtime_error {
public:
Expand Down
2 changes: 1 addition & 1 deletion src/sync/impl/sync_metadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ SyncMetadataManager::SyncMetadataManager(std::string path,
config.schema_mode = SchemaMode::Automatic;
#if REALM_PLATFORM_APPLE
if (should_encrypt && !encryption_key) {
encryption_key = keychain::metadata_realm_encryption_key();
encryption_key = keychain::metadata_realm_encryption_key(File::exists(path));
}
#endif
if (should_encrypt) {
Expand Down