false, an existing entry requiring authentication keeps its protection, and updating it requires the user to authenticate.
+ * @see SecItemAdd
*/
public void storePassword(String serviceName, String account, CharSequence password, boolean requireOsAuthentication) throws KeychainAccessException {
ByteBuffer pwBuf = UTF_8.encode(CharBuffer.wrap(password));
@@ -40,49 +40,28 @@ public void storePassword(String serviceName, String account, CharSequence passw
*
* @param serviceName Service name
* @param account Unique account identifier
- * @return password or null if no such keychain entry could be loaded from the keychain.
- * @see SecKeychainFindGenericPassword
+ * @return password or null if no such keychain entry exists.
+ * @throws KeychainAccessException If loading fails for any reason other than a missing entry, e.g. because authentication was cancelled
+ * @see SecItemCopyMatching
*/
- public char[] loadPassword(String serviceName, String account) {
- byte[] pwBytes = Native.INSTANCE.loadPassword(serviceName.getBytes(UTF_8), account.getBytes(UTF_8));
- if (pwBytes == null) {
- // this if-statement is a workaround for https://github.com/cryptomator/integrations-mac/issues/13:
- if ("Cryptomator".equals(serviceName) && tryMigratePassword(account)) {
- // on success: retry
- return loadPassword(serviceName, account);
- }
- return null;
- } else {
- CharBuffer pwBuf = UTF_8.decode(ByteBuffer.wrap(pwBytes));
- char[] pw = new char[pwBuf.remaining()];
- pwBuf.get(pw);
- Arrays.fill(pwBytes, (byte) 0x00);
- Arrays.fill(pwBuf.array(), (char) 0x00);
- return pw;
- }
+ public StoredPassword loadPassword(String serviceName, String account) throws KeychainAccessException {
+ return toStoredPassword(Native.INSTANCE.loadPassword(serviceName.getBytes(UTF_8), account.getBytes(UTF_8)));
}
- /**
- * Fix for Issue 13.
- * Attempts to find the account with the old hard-coded service name 'Cryptomator\0'
- *
- * @param account Unique account identifier
- * @return true on success
- */
- private boolean tryMigratePassword(String account) {
- byte[] oldServiceName = {'C', 'r', 'y', 'p', 't', 'o', 'm', 'a', 't', 'o', 'r', '\0'};
- byte[] newServiceName = {'C', 'r', 'y', 'p', 't', 'o', 'm', 'a', 't', 'o', 'r'};
- byte[] pwBytes = Native.INSTANCE.loadPassword(oldServiceName, account.getBytes(UTF_8));
- if (pwBytes == null) {
- return false;
+ // visible for testing
+ static StoredPassword toStoredPassword(LoadResult result) throws KeychainAccessException {
+ if (result.status() == OSSTATUS_NOT_FOUND) {
+ return null;
+ } else if (result.status() != OSSTATUS_SUCCESS || result.password() == null) {
+ throw new KeychainAccessException("Failed to load password. Error code " + result.status());
}
- int errorCode = Native.INSTANCE.storePassword(newServiceName, account.getBytes(UTF_8), pwBytes, false);
+ byte[] pwBytes = result.password();
+ CharBuffer pwBuf = UTF_8.decode(ByteBuffer.wrap(pwBytes));
+ char[] pw = new char[pwBuf.remaining()];
+ pwBuf.get(pw);
Arrays.fill(pwBytes, (byte) 0x00);
- if (errorCode != OSSTATUS_SUCCESS) {
- return false;
- }
- Native.INSTANCE.deletePassword(oldServiceName, account.getBytes(UTF_8));
- return true;
+ Arrays.fill(pwBuf.array(), (char) 0x00);
+ return new StoredPassword(pw, result.dataProtectionKeychain());
}
/**
@@ -91,7 +70,7 @@ private boolean tryMigratePassword(String account) {
* @param serviceName Service name
* @param account Unique account identifier
* @return true if the passwords has been deleted, false if no entry for the given key exists.
- * @see SecKeychainItemDelete
+ * @see SecItemDelete
*/
public boolean deletePassword(String serviceName, String account) throws KeychainAccessException {
int errorCode = Native.INSTANCE.deletePassword(serviceName.getBytes(UTF_8), account.getBytes(UTF_8));
@@ -105,12 +84,22 @@ public boolean deletePassword(String serviceName, String account) throws Keychai
}
/**
- * Tests whether biometric authentication via Touch ID is supported and allowed on the device
+ * Tests whether the user can authenticate via Touch ID or, as a fallback, their password. Also tests whether passwords requiring such authentication can be stored.
*
- * @return true if biometric authentication is available, false otherwise
+ * @return true if passwords requiring user authentication are supported, false otherwise
+ */
+ public boolean isOsAuthenticationAvailable() {
+ return Native.INSTANCE.isOsAuthenticationAvailable();
+ }
+
+ /**
+ * @param requiresOsAuthentication Whether the keychain entry is stored in the data protection keychain, where {@link #storePassword(String, String, CharSequence, boolean)} only stores entries requiring user presence
*/
- public boolean isTouchIDavailable() {
- return Native.INSTANCE.isTouchIDavailable();
+ record StoredPassword(char[] value, boolean requiresOsAuthentication) {
+ }
+
+ // constructed via JNI by class name and constructor signature, so renaming or reordering its components breaks the native loadPassword
+ record LoadResult(int status, byte[] password, boolean dataProtectionKeychain) {
}
// initialization-on-demand pattern, as loading the .dylib is an expensive operation
@@ -123,11 +112,11 @@ private Native() {
public native int storePassword(byte[] service, byte[] account, byte[] value, boolean requireOsAuthentication);
- public native byte[] loadPassword(byte[] service, byte[] account);
+ public native LoadResult loadPassword(byte[] service, byte[] account);
public native int deletePassword(byte[] service, byte[] account);
- public native boolean isTouchIDavailable();
+ public native boolean isOsAuthenticationAvailable();
}
}
diff --git a/src/main/java/org/cryptomator/macos/keychain/MacSystemKeychainAccess.java b/src/main/java/org/cryptomator/macos/keychain/MacSystemKeychainAccess.java
index a1926aa..3810386 100644
--- a/src/main/java/org/cryptomator/macos/keychain/MacSystemKeychainAccess.java
+++ b/src/main/java/org/cryptomator/macos/keychain/MacSystemKeychainAccess.java
@@ -36,8 +36,9 @@ public void storePassphrase(String key, String displayName, CharSequence passphr
}
@Override
- public char[] loadPassphrase(String key) {
- return keychain.loadPassword(SERVICE_NAME, key);
+ public char[] loadPassphrase(String key) throws KeychainAccessException {
+ var password = keychain.loadPassword(SERVICE_NAME, key);
+ return password != null ? password.value() : null;
}
@Override
diff --git a/src/main/java/org/cryptomator/macos/keychain/TouchIdKeychainAccess.java b/src/main/java/org/cryptomator/macos/keychain/TouchIdKeychainAccess.java
index 74c701f..4cd96e4 100644
--- a/src/main/java/org/cryptomator/macos/keychain/TouchIdKeychainAccess.java
+++ b/src/main/java/org/cryptomator/macos/keychain/TouchIdKeychainAccess.java
@@ -6,11 +6,14 @@
import org.cryptomator.integrations.keychain.KeychainAccessException;
import org.cryptomator.integrations.keychain.KeychainAccessProvider;
+import java.nio.CharBuffer;
+import java.util.Arrays;
+
/**
- * Stores passwords in the macOS system keychain. Requires an authenticated user to do so.
+ * Stores passwords in the macOS system keychain, requiring an authenticated user to load them.
* Authentication is done via TouchID or password as a fallback, when TouchID is not available.
*
- * Items are stored in the default keychain with the service name Cryptomator, unless configured otherwise
+ * Items are stored in the data protection keychain with the service name Cryptomator, unless configured otherwise
* using the system property cryptomator.integrationsMac.keychainServiceName.
*/
@Priority(1010)
@@ -37,13 +40,32 @@ public void storePassphrase(String key, String displayName, CharSequence passphr
}
@Override
- public char[] loadPassphrase(String key) {
- return keychain.loadPassword(SERVICE_NAME, key);
+ public char[] loadPassphrase(String key) throws KeychainAccessException {
+ var password = keychain.loadPassword(SERVICE_NAME, key);
+ if (password == null) {
+ return null;
+ } else if (password.requiresOsAuthentication()) {
+ return password.value();
+ }
+ // the entry was stored without user presence (e.g. by MacSystemKeychainAccess): protect it, then load it again through the authenticated path
+ try {
+ keychain.storePassword(SERVICE_NAME, key, CharBuffer.wrap(password.value()), true);
+ } finally {
+ Arrays.fill(password.value(), '\0');
+ }
+ var protectedPassword = keychain.loadPassword(SERVICE_NAME, key);
+ if (protectedPassword == null || !protectedPassword.requiresOsAuthentication()) {
+ if (protectedPassword != null) {
+ Arrays.fill(protectedPassword.value(), '\0');
+ }
+ throw new KeychainAccessException("Failed to protect stored passphrase with user presence");
+ }
+ return protectedPassword.value();
}
@Override
public boolean isSupported() {
- return keychain.isTouchIDavailable();
+ return keychain.isOsAuthenticationAvailable();
}
@Override
diff --git a/src/main/native/Integrations.xcodeproj/project.pbxproj b/src/main/native/Integrations.xcodeproj/project.pbxproj
index a1c46cb..a114563 100644
--- a/src/main/native/Integrations.xcodeproj/project.pbxproj
+++ b/src/main/native/Integrations.xcodeproj/project.pbxproj
@@ -207,7 +207,7 @@
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = "$(JAVA_HOME)/include/**";
JAVA_HOME = "$(HOME)/.sdkman/candidates/java/current";
- MACOSX_DEPLOYMENT_TARGET = 10.13;
+ MACOSX_DEPLOYMENT_TARGET = 11.0;
MTL_ENABLE_DEBUG_INFO = YES;
ONLY_ACTIVE_ARCH = YES;
SDKROOT = macosx;
@@ -260,7 +260,7 @@
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = "$(JAVA_HOME)/include/**";
JAVA_HOME = "$(HOME)/.sdkman/candidates/java/current";
- MACOSX_DEPLOYMENT_TARGET = 10.13;
+ MACOSX_DEPLOYMENT_TARGET = 11.0;
MTL_ENABLE_DEBUG_INFO = NO;
SDKROOT = macosx;
USER_HEADER_SEARCH_PATHS = ../headers;
diff --git a/src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m b/src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
index 2741bc7..157c2b0 100644
--- a/src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
+++ b/src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
@@ -20,116 +20,157 @@
return sharedContext;
}
-JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_storePassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key, jbyteArray password, jboolean requireOsAuthentication) {
- jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
- jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);
- jbyte *pwStr = (*env)->GetByteArrayElements(env, password, NULL);
- jsize length = (*env)->GetArrayLength(env, password);
+// returns nil without further JNI calls, if a previous call left an exception pending
+static NSString* stringFromByteArray(JNIEnv *env, jbyteArray array) {
+ if ((*env)->ExceptionCheck(env)) {
+ return nil;
+ }
+ jbyte *bytes = (*env)->GetByteArrayElements(env, array, NULL);
+ if (bytes == NULL) {
+ return nil;
+ }
+ NSString *string = [[NSString alloc] initWithBytes:bytes length:(*env)->GetArrayLength(env, array) encoding:NSUTF8StringEncoding];
+ (*env)->ReleaseByteArrayElements(env, array, bytes, JNI_ABORT);
+ return string;
+}
- NSString *serviceString = [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding];
- NSString *keyString = [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding];
+// Items requiring user presence live in the data protection keychain, all others in the file-based keychain. Storing and loading target one of them explicitly,
+// because a query spanning both reports a failed authentication in the data protection keychain as errSecItemNotFound.
+static NSDictionary* itemQuery(NSString *service, NSString *account, BOOL dataProtectionKeychain) {
+ return @{
+ (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
+ (__bridge id)kSecAttrService: service,
+ (__bridge id)kSecAttrAccount: account,
+ (__bridge id)kSecUseDataProtectionKeychain: @(dataProtectionKeychain)
+ };
+}
+
+// without an entitlement granting a keychain access group, the data protection keychain is unavailable, so depending on the operation, it reports either status:
+static BOOL isAbsentFromDataProtectionKeychain(OSStatus status) {
+ return status == errSecItemNotFound || status == errSecMissingEntitlement;
+}
+
+JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_storePassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key, jbyteArray password, jboolean requireOsAuthentication) {
+ NSString *serviceString = stringFromByteArray(env, service);
+ NSString *keyString = stringFromByteArray(env, key);
+ jbyte *pwStr = (*env)->ExceptionCheck(env) ? NULL : (*env)->GetByteArrayElements(env, password, NULL);
OSStatus status = errSecParam;
- if (serviceString && keyString) { // guard both NSStrings not to be NIL
- // find existing:
- NSMutableDictionary *query = [@{
- (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
- (__bridge id)kSecAttrService: serviceString,
- (__bridge id)kSecAttrAccount: keyString,
- (__bridge id)kSecReturnAttributes: @YES,
- (__bridge id)kSecReturnData: @YES,
- (__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne
- } mutableCopy];
+ if (serviceString && keyString && pwStr) {
+ NSData *passwordData = [NSData dataWithBytes:pwStr length:(*env)->GetArrayLength(env, password)];
+ BOOL protectedItemStored = NO;
if (requireOsAuthentication) {
- LAContext *context = getSharedLAContext();
- query[(__bridge id)kSecUseAuthenticationContext] = context;
- }
- CFDictionaryRef result = NULL;
- status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
- if (status == errSecSuccess && result != NULL) {
- // update existing:
- NSDictionary *attributesToUpdate = @{
- (__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
- };
- status = SecItemUpdate((__bridge CFDictionaryRef)query, (__bridge CFDictionaryRef)attributesToUpdate);
- } else if (status == errSecItemNotFound) {
- // add new:
- NSMutableDictionary *attributes = [@{
- (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
- (__bridge id)kSecAttrService: serviceString,
- (__bridge id)kSecAttrAccount: keyString,
- (__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
- } mutableCopy];
- if (requireOsAuthentication) {
- attributes[(__bridge id)kSecAttrAccessControl] = (__bridge_transfer id)SecAccessControlCreateWithFlags(kCFAllocatorDefault, kSecAttrAccessibleWhenUnlocked, kSecAccessControlUserPresence, NULL);
+ id accessControl = (__bridge_transfer id)SecAccessControlCreateWithFlags(kCFAllocatorDefault, kSecAttrAccessibleWhenUnlocked, kSecAccessControlUserPresence, NULL);
+ if (accessControl) {
+ NSDictionary *protectedQuery = itemQuery(serviceString, keyString, YES);
+ status = SecItemDelete((__bridge CFDictionaryRef)protectedQuery);
+ if (status == errSecSuccess || status == errSecItemNotFound) {
+ NSMutableDictionary *attributes = [protectedQuery mutableCopy];
+ attributes[(__bridge id)kSecValueData] = passwordData;
+ attributes[(__bridge id)kSecAttrAccessControl] = accessControl;
+ status = SecItemAdd((__bridge CFDictionaryRef)attributes, NULL);
+ }
+ protectedItemStored = status == errSecSuccess;
}
- status = SecItemAdd((__bridge CFDictionaryRef)attributes, NULL);
} else {
+ // an existing protected item takes precedence when loading, so update it in place. The update keeps its protection and requires the user to authenticate:
+ NSDictionary *valueAttributes = @{(__bridge id)kSecValueData: passwordData};
+ NSMutableDictionary *protectedQuery = [itemQuery(serviceString, keyString, YES) mutableCopy];
+ protectedQuery[(__bridge id)kSecUseAuthenticationContext] = getSharedLAContext();
+ status = SecItemUpdate((__bridge CFDictionaryRef)protectedQuery, (__bridge CFDictionaryRef)valueAttributes);
+ protectedItemStored = status == errSecSuccess;
+ if (isAbsentFromDataProtectionKeychain(status)) {
+ NSDictionary *query = itemQuery(serviceString, keyString, NO);
+ status = SecItemUpdate((__bridge CFDictionaryRef)query, (__bridge CFDictionaryRef)valueAttributes);
+ if (status == errSecItemNotFound) {
+ NSMutableDictionary *attributes = [query mutableCopy];
+ attributes[(__bridge id)kSecValueData] = passwordData;
+ status = SecItemAdd((__bridge CFDictionaryRef)attributes, NULL);
+ }
+ }
+ }
+ if (protectedItemStored) {
+ // delete the unprotected item only once the protected one is stored, so that a failed store never loses the password:
+ OSStatus deleteStatus = SecItemDelete((__bridge CFDictionaryRef)itemQuery(serviceString, keyString, NO));
+ if (deleteStatus != errSecItemNotFound) {
+ status = deleteStatus;
+ }
+ }
+ if (status != errSecSuccess) {
NSLog(@"Error storing item in keychain. Status code: %d", (int)status);
}
} else {
- NSLog(@"Invalid UTF-8 input: service or key string conversion failed");
+ NSLog(@"Invalid input: service or key string conversion failed or password unavailable");
}
- (*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
- (*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
- (*env)->ReleaseByteArrayElements(env, password, pwStr, JNI_ABORT);
+ if (pwStr) {
+ (*env)->ReleaseByteArrayElements(env, password, pwStr, JNI_ABORT);
+ }
return status;
}
-JNIEXPORT jbyteArray JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_loadPassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key) {
- jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
- jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);
+JNIEXPORT jobject JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_loadPassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key) {
+ NSString *serviceString = stringFromByteArray(env, service);
+ NSString *keyString = stringFromByteArray(env, key);
- NSString *serviceString = [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding];
- NSString *keyString = [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding];
-
- jbyteArray password = NULL;
+ OSStatus status = errSecParam;
+ NSData *passwordData = nil;
+ jboolean dataProtectionKeychain = JNI_FALSE;
if (serviceString && keyString) { // guard both NSStrings not to be NIL
- // find existing:
- LAContext *context = getSharedLAContext();
- NSDictionary *query = @{
- (__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
- (__bridge id)kSecAttrService: serviceString,
- (__bridge id)kSecAttrAccount: keyString,
- (__bridge id)kSecReturnAttributes: @YES,
- (__bridge id)kSecReturnData: @YES,
- (__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne,
- (__bridge id)kSecUseAuthenticationContext: context
- };
- CFDictionaryRef result = NULL;
- OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
+ // find existing, preferring the protected item:
+ NSMutableDictionary *query = [itemQuery(serviceString, keyString, YES) mutableCopy];
+ query[(__bridge id)kSecReturnData] = @YES;
+ query[(__bridge id)kSecMatchLimit] = (__bridge id)kSecMatchLimitOne;
+ query[(__bridge id)kSecUseAuthenticationContext] = getSharedLAContext();
+ CFTypeRef result = NULL;
+ status = SecItemCopyMatching((__bridge CFDictionaryRef)query, &result);
+ if (status == errSecSuccess) {
+ dataProtectionKeychain = JNI_TRUE;
+ } else if (isAbsentFromDataProtectionKeychain(status)) {
+ query[(__bridge id)kSecUseDataProtectionKeychain] = @NO;
+ status = SecItemCopyMatching((__bridge CFDictionaryRef)query, &result);
+ }
if (status == errSecSuccess && result != NULL) {
- // retrieve password:
- NSDictionary *attributes = (__bridge_transfer NSDictionary *)result;
- NSData *passwordData = attributes[(__bridge id)kSecValueData];
- password = (*env)->NewByteArray(env, (int)passwordData.length);
- (*env)->SetByteArrayRegion(env, password, 0, (int)passwordData.length, (jbyte *)passwordData.bytes);
+ passwordData = (__bridge_transfer NSData *)result;
} else if (status != errSecItemNotFound) {
NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
}
} else {
- NSLog(@"Invalid UTF-8 input: service or key string conversion failed");
+ NSLog(@"Invalid input: service or key string conversion failed");
}
- (*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
- (*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
- return password;
+ if ((*env)->ExceptionCheck(env)) {
+ return NULL;
+ }
+ jbyteArray password = NULL;
+ if (passwordData) {
+ password = (*env)->NewByteArray(env, (int)passwordData.length);
+ if (password == NULL) {
+ return NULL;
+ }
+ (*env)->SetByteArrayRegion(env, password, 0, (int)passwordData.length, (jbyte *)passwordData.bytes);
+ }
+ jclass resultClass = (*env)->FindClass(env, "org/cryptomator/macos/keychain/MacKeychain$LoadResult");
+ if (resultClass == NULL) {
+ return NULL;
+ }
+ jmethodID resultConstructor = (*env)->GetMethodID(env, resultClass, "