Skip to content

Commit

Permalink
Merge pull request #53 from purejava/needAuthenticatedUser
Browse files Browse the repository at this point in the history 
Widen API to allow storing keychain entries for an authenticated user
  • Loading branch information
@tobihagemann
tobihagemann authored Sep 18, 2024
2 parents 7393780 + 851965a commit 6cc701b
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 32 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
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</api.version>
<slf4j.version>2.0.13</slf4j.version>

<!-- test dependencies -->
Expand Down
4 changes: 2 additions & 2 deletions src/main/headers/org_cryptomator_macos_keychain_MacKeychain_Native.h
View file

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
View file
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
6 changes: 3 additions & 3 deletions src/main/java/org/cryptomator/macos/keychain/MacSystemKeychainAccess.java
View file
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
42 changes: 29 additions & 13 deletions src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,36 @@
#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;
static LAContext* getSharedLAContext(void) {
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
sharedContext = [[LAContext alloc] init];
});
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);

// find existing:
NSDictionary *query = @{
NSMutableDictionary *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
};
} mutableCopy];
if (requireOsAuthentication) {
LAContext *context = getSharedLAContext();
query[(__bridge id)kSecUseAuthenticationContext] = context;
}
CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, (CFTypeRef *)&result);
if (status == errSecSuccess && result != NULL) {
Expand All @@ -35,12 +49,15 @@ 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 = @{
NSMutableDictionary *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]
};
} mutableCopy];
if (requireOsAuthentication) {
attributes[(__bridge id)kSecAttrAccessControl] = (__bridge_transfer id)SecAccessControlCreateWithFlags(kCFAllocatorDefault, kSecAttrAccessibleWhenUnlocked, kSecAccessControlUserPresence, NULL);
}
status = SecItemAdd((__bridge CFDictionaryRef)attributes, NULL);
} else {
NSLog(@"Error storing item in keychain. Status code: %d", (int)status);
Expand All @@ -57,13 +74,15 @@ JNIEXPORT jbyteArray JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_000
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);

// find existing:
LAContext *context = getSharedLAContext();
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 @@ -88,18 +107,15 @@ JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Nati
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);

// find existing:
LAContext *context = getSharedLAContext();
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
2 changes: 1 addition & 1 deletion src/test/java/org/cryptomator/macos/keychain/MacKeychainTest.java
View file
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
10 changes: 5 additions & 5 deletions src/test/java/org/cryptomator/macos/keychain/MacSystemKeychainAccessTest.java
View file
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

0 comments on commit 6cc701b

Please sign in to comment.