Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widen API to allow storing keychain entries for an authenticated user #53

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<project.jdk.version>17</project.jdk.version>

<!-- runtime dependencies -->
<api.version>1.3.1</api.version>
<api.version>1.4.0-beta3</api.version>
<slf4j.version>2.0.13</slf4j.version>

<!-- test dependencies -->
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 8 additions & 7 deletions src/main/java/org/cryptomator/macos/keychain/MacKeychain.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@ class MacKeychain {
/**
* Associates the specified password with the specified key in the system keychain.
*
* @param serviceName Service name
* @param account Unique account identifier
* @param password Passphrase to store
* @param serviceName Service name
* @param account Unique account identifier
* @param password Passphrase to store
* @param requireOsAuthentication Defines, whether the user needs to authenticate to store a passphrase
* @see <a href="https://developer.apple.com/documentation/security/1398366-seckeychainaddgenericpassword">SecKeychainAddGenericPassword</a>
*/
public void storePassword(String serviceName, String account, CharSequence password) throws KeychainAccessException {
public void storePassword(String serviceName, String account, CharSequence password, boolean requireOsAuthentication) throws KeychainAccessException {
ByteBuffer pwBuf = UTF_8.encode(CharBuffer.wrap(password));
byte[] pwBytes = new byte[pwBuf.remaining()];
pwBuf.get(pwBytes);
int errorCode = Native.INSTANCE.storePassword(serviceName.getBytes(UTF_8), account.getBytes(UTF_8), pwBytes);
int errorCode = Native.INSTANCE.storePassword(serviceName.getBytes(UTF_8), account.getBytes(UTF_8), pwBytes, requireOsAuthentication);
Arrays.fill(pwBytes, (byte) 0x00);
Arrays.fill(pwBuf.array(), (byte) 0x00);
if (errorCode != OSSTATUS_SUCCESS) {
Expand Down Expand Up @@ -75,7 +76,7 @@ private boolean tryMigratePassword(String account) {
if (pwBytes == null) {
return false;
}
int errorCode = Native.INSTANCE.storePassword(newServiceName, account.getBytes(UTF_8), pwBytes);
int errorCode = Native.INSTANCE.storePassword(newServiceName, account.getBytes(UTF_8), pwBytes, false);
Arrays.fill(pwBytes, (byte) 0x00);
if (errorCode != OSSTATUS_SUCCESS) {
return false;
Expand Down Expand Up @@ -111,7 +112,7 @@ private Native() {
NativeLibLoader.loadLib();
}

public native int storePassword(byte[] service, byte[] account, byte[] value);
public native int storePassword(byte[] service, byte[] account, byte[] value, boolean requireOsAuthentication);

public native byte[] loadPassword(byte[] service, byte[] account);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public String displayName() {
}

@Override
public void storePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException {
keychain.storePassword(SERVICE_NAME, key, passphrase);
public void storePassphrase(String key, String displayName, CharSequence passphrase, boolean requireOsAuthentication) throws KeychainAccessException {
keychain.storePassword(SERVICE_NAME, key, passphrase, requireOsAuthentication);
}

@Override
Expand All @@ -62,7 +62,7 @@ public void deletePassphrase(String key) throws KeychainAccessException {
@Override
public void changePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException {
if (keychain.deletePassword(SERVICE_NAME, key)) {
keychain.storePassword(SERVICE_NAME, key, passphrase);
keychain.storePassword(SERVICE_NAME, key, passphrase, false);
}
}

Expand Down
99 changes: 76 additions & 23 deletions src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,63 @@
#import "org_cryptomator_macos_keychain_MacKeychain_Native.h"
#import <Foundation/Foundation.h>
#import <Security/Security.h>
#import <LocalAuthentication/LocalAuthentication.h>

JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_storePassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key, jbyteArray password) {
static LAContext *sharedContext = nil;

LAContext* getSharedLAContext(void) {
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
sharedContext = [[LAContext alloc] init];
});
return sharedContext;
}

SecAccessControlRef createAccessControl(void) {
SecAccessControlCreateFlags flags = kSecAccessControlUserPresence;

SecAccessControlRef accessControl = SecAccessControlCreateWithFlags(
kCFAllocatorDefault,
kSecAttrAccessibleWhenUnlocked,
flags,
NULL // Ignore any error
);

return accessControl;
}

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);

// find existing:
NSDictionary *query = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecReturnAttributes: @YES,
(__bridge id)kSecReturnData: @YES,
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne
};
NSDictionary *query = NULL;

if (requireOsAuthentication) {
LAContext *context = getSharedLAContext();

query = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecReturnAttributes: @YES,
(__bridge id)kSecReturnData: @YES,
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne,
(__bridge id)kSecUseAuthenticationContext: context
};
} else {
query = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecReturnAttributes: @YES,
(__bridge id)kSecReturnData: @YES,
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne
};
}

CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
if (status == errSecSuccess && result != NULL) {
Expand All @@ -35,12 +76,23 @@ JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Nati
status = SecItemUpdate((__bridge CFDictionaryRef)query, (__bridge CFDictionaryRef)attributesToUpdate);
} else if (status == errSecItemNotFound) {
// add new:
NSDictionary *attributes = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
};
NSDictionary *attributes = NULL;
if (requireOsAuthentication) {
attributes = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccessControl: (__bridge_transfer id)createAccessControl(),
(__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
};
} else {
attributes = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
};
}
status = SecItemAdd((__bridge CFDictionaryRef)attributes, NULL);
} else {
NSLog(@"Error storing item in keychain. Status code: %d", (int)status);
Expand All @@ -56,14 +108,17 @@ JNIEXPORT jbyteArray JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_000
jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);

LAContext *context = getSharedLAContext();

// find existing:
NSDictionary *query = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecReturnAttributes: @YES,
(__bridge id)kSecReturnData: @YES,
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne,
(__bridge id)kSecUseAuthenticationContext: context
};
CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
Expand All @@ -87,19 +142,17 @@ JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Nati
jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);

LAContext *context = getSharedLAContext();

// find existing:
NSDictionary *query = @{
(__bridge id)kSecClass: (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrService: [NSString stringWithCString:(char *)serviceStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecAttrAccount: [NSString stringWithCString:(char *)keyStr encoding:NSUTF8StringEncoding],
(__bridge id)kSecMatchLimit: (__bridge id)kSecMatchLimitOne
(__bridge id)kSecUseAuthenticationContext: context
};
CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
if (status == errSecSuccess && result != NULL) {
// delete:
status = SecItemDelete((__bridge CFDictionaryRef)query);
} else if (status != errSecItemNotFound) {
OSStatus status = SecItemDelete((__bridge CFDictionaryRef)query);
if (status != errSecSuccess) {
NSLog(@"Error deleting item from keychain. Status code: %d", (int)status);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class WithStoredPassword {

@BeforeEach
public void setup() throws KeychainAccessException {
keychain.storePassword("service", "account", storedPw);
keychain.storePassword("service", "account", storedPw, false);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ public void testDisplayName() {
public void testStoreSuccess() throws KeychainAccessException {
keychainAccess.storePassphrase("key", "pass");

Mockito.verify(keychain).storePassword("Cryptomator", "key", "pass");
Mockito.verify(keychain).storePassword("Cryptomator", "key", "pass", false);
}

@Test
@DisplayName("storePassphrase() fails")
public void testStoreError() throws KeychainAccessException {
KeychainAccessException e = new KeychainAccessException("fail.");
Mockito.doThrow(e).when(keychain).storePassword(Mockito.eq("Cryptomator"), Mockito.any(), Mockito.any());
Mockito.doThrow(e).when(keychain).storePassword(Mockito.eq("Cryptomator"), Mockito.any(), Mockito.any(), Mockito.eq(false));

KeychainAccessException thrown = Assertions.assertThrows(KeychainAccessException.class, () -> {
keychainAccess.storePassphrase("key", "pass");
keychainAccess.storePassphrase("key", "", "pass", false);
});
Assertions.assertSame(thrown, e);
}
Expand Down Expand Up @@ -93,7 +93,7 @@ public void testChangeSuccess() throws KeychainAccessException {

keychainAccess.changePassphrase("key", "newpass");

Mockito.verify(keychain).storePassword("Cryptomator", "key", "newpass");
Mockito.verify(keychain).storePassword("Cryptomator", "key", "newpass", false);
}

@Test
Expand All @@ -103,7 +103,7 @@ public void testChangeNotFound() throws KeychainAccessException {

keychainAccess.changePassphrase("key", "newpass");

Mockito.verify(keychain, Mockito.never()).storePassword("Cryptomator", "key", "newpass");
Mockito.verify(keychain, Mockito.never()).storePassword("Cryptomator", "key", "newpass", false);
}

@Test
Expand Down
Loading