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.

Sign up for GitHub

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

Jump to bottom

Replace the deprecated SecKeychain with SecItem #46

Merged
merged 7 commits into from
Jul 1, 2024
Merged

Replace the deprecated SecKeychain with SecItem #46

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c73db9
Implement MacSystemKeychainAccess using SecItem API
purejava Feb 18, 2024
9fc29ce
kSecValueData must be of type NSData
purejava May 26, 2024
372d4fe
Set access control to default for testing purposes
purejava May 26, 2024
cd0cb40
Reduce to code without access control
purejava May 27, 2024
77c6e10
Fix indentation
purejava May 30, 2024
85a1a50
Bump the maven-build-plugins group across 1 directory with 8 updates …
dependabot[bot] Jun 3, 2024
0fcd405
Minor refactorings
tobihagemann Jul 1, 2024
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
165 changes: 85 additions & 80 deletions src/main/native/org_cryptomator_macos_keychain_MacKeychain_Native.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,117 +7,122 @@
//

#import "org_cryptomator_macos_keychain_MacKeychain_Native.h"
#import <Foundation/Foundation.h>
#import <Security/Security.h>

JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_storePassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key, jbyteArray password) {
const int serviceLen = (*env)->GetArrayLength(env, service);
jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
const int keyLen = (*env)->GetArrayLength(env, key);
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);
const int pwLen = (*env)->GetArrayLength(env, password);
jbyte *pwStr = (*env)->GetByteArrayElements(env, password, NULL);
jsize length = (*env)->GetArrayLength(env, password);

// find existing:
SecKeychainItemRef itemRef = NULL;
OSStatus status = SecKeychainFindGenericPassword(
NULL, // default keychain
serviceLen, // length of service name
(char *)serviceStr, // service name
keyLen, // length of account name
(char *)keyStr, // account name
NULL, // length of password
NULL, // pointer to password data
&itemRef // the item reference
);
if (status == errSecSuccess) {
NSDictionary *searchAttributes = @{
(__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)searchAttributes, (CFTypeRef *)&result);

if (status == errSecSuccess && result != NULL) {
// update existing:
status = SecKeychainItemModifyAttributesAndData(
itemRef, // the item reference
NULL, // no change to attributes
pwLen, // length of password
pwStr // pointer to password data
);
NSDictionary *changeAttributes = @{
(__bridge id)kSecValueData: [NSData dataWithBytes:pwStr length:length]
};

status = SecItemUpdate((__bridge CFDictionaryRef)searchAttributes, (__bridge CFDictionaryRef)changeAttributes);

} else if (status == errSecItemNotFound) {
// add new:
status = SecKeychainAddGenericPassword(
NULL, // default keychain
serviceLen, // length of service name
(char *)serviceStr, // service name
keyLen, // length of account name
(char *)keyStr, // account name
pwLen, // length of password
pwStr, // pointer to password data
NULL // the item reference
);
NSDictionary *keychainAttributes = @{
(__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)keychainAttributes, NULL);

} else {
NSLog(@"Error storing item in keychain. Status code: %d", (int)status);
}

(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, password, pwStr, JNI_ABORT);
if (itemRef) {
CFRelease(itemRef);
}

return status;
}

JNIEXPORT jbyteArray JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_loadPassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key) {
const int serviceLen = (*env)->GetArrayLength(env, service);
jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
const int keyLen = (*env)->GetArrayLength(env, key);
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);
void *pwStr = NULL;
UInt32 pwLen;
OSStatus status = SecKeychainFindGenericPassword(
NULL, // default keychain
serviceLen, // length of service name
(char *)serviceStr, // service name
keyLen, // length of account name
(char *)keyStr, // account name
&pwLen, // length of password
&pwStr, // pointer to password data
NULL // the item reference
);

jbyteArray result;
if (status == errSecSuccess) {
result = (*env)->NewByteArray(env, pwLen);
(*env)->SetByteArrayRegion(env, result, 0, pwLen, pwStr);
} else {
result = NULL;
}

// Create a dictionary of search attributes to find the item in the keychain
NSDictionary *searchAttributes = @{
(__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)searchAttributes, (CFTypeRef *)&result);

(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
if (pwStr) {
SecKeychainItemFreeContent(NULL, pwStr);

if (status == errSecSuccess && result != NULL) {
NSDictionary *attributes = (__bridge_transfer NSDictionary *)result;
NSData *passwordData = attributes[(__bridge id)kSecValueData];

NSUInteger pwLen = [passwordData length];
const void *pwBytes = [passwordData bytes];

jbyteArray password = (*env)->NewByteArray(env, (int)pwLen);
(*env)->SetByteArrayRegion(env, password, 0, (int)pwLen, (jbyte *)pwBytes);

return password;

} else if (status != errSecItemNotFound) {
NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
}
return result;

return NULL; // empty
Copy link

Choose a reason for hiding this comment

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

Refactor to simplify error handling in password retrieval.

-    } else if (status != errSecItemNotFound) {
-        NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
-    }
+    } else {
+        NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
+    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// Create a dictionary of search attributes to find the item in the keychain
NSDictionary *searchAttributes = @{
(__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)searchAttributes, (CFTypeRef *)&result);
(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
if (pwStr) {
SecKeychainItemFreeContent(NULL, pwStr);
if (status == errSecSuccess && result != NULL) {
NSDictionary *attributes = (__bridge_transfer NSDictionary *)result;
NSData *passwordData = attributes[(__bridge id)kSecValueData];
NSUInteger pwLen = [passwordData length];
const void *pwBytes = [passwordData bytes];
jbyteArray password = (*env)->NewByteArray(env, (int)pwLen);
(*env)->SetByteArrayRegion(env, password, 0, (int)pwLen, (jbyte *)pwBytes);
return password;
} else if (status != errSecItemNotFound) {
NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
}
return result;
return NULL; // empty
// Create a dictionary of search attributes to find the item in the keychain
NSDictionary *searchAttributes = @{
(__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)searchAttributes, (CFTypeRef *)&result);
(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
if (status == errSecSuccess && result != NULL) {
NSDictionary *attributes = (__bridge_transfer NSDictionary *)result;
NSData *passwordData = attributes[(__bridge id)kSecValueData];
NSUInteger pwLen = [passwordData length];
const void *pwBytes = [passwordData bytes];
jbyteArray password = (*env)->NewByteArray(env, (int)pwLen);
(*env)->SetByteArrayRegion(env, password, 0, (int)pwLen, (jbyte *)pwBytes);
return password;
} else {
NSLog(@"Error retrieving item from keychain. Status code: %d", (int)status);
}
return NULL; // empty

}

JNIEXPORT jint JNICALL Java_org_cryptomator_macos_keychain_MacKeychain_00024Native_deletePassword(JNIEnv *env, jobject thisObj, jbyteArray service, jbyteArray key) {
const int serviceLen = (*env)->GetArrayLength(env, service);
jbyte *serviceStr = (*env)->GetByteArrayElements(env, service, NULL);
const int keyLen = (*env)->GetArrayLength(env, key);
jbyte *keyStr = (*env)->GetByteArrayElements(env, key, NULL);
SecKeychainItemRef itemRef = NULL;
OSStatus status = SecKeychainFindGenericPassword(
NULL, // default keychain
serviceLen, // length of service name
(char *)serviceStr, // service name
keyLen, // length of account name
(char *)keyStr, // account name
NULL, // length of password
NULL, // pointer to password data
&itemRef // the item reference
);

if (status == errSecSuccess) {
status = SecKeychainItemDelete(
itemRef // the item reference
);

// find existing:
NSDictionary *searchAttributes = @{
(__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
};

CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)searchAttributes, (CFTypeRef *)&result);

if (status == errSecSuccess && result != NULL) {

status = SecItemDelete((__bridge CFDictionaryRef)searchAttributes);

} else if (status != errSecItemNotFound) {
NSLog(@"Error deleting item from keychain. Status code: %d", (int)status);
}

(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
if (itemRef) {
CFRelease(itemRef);
}

Copy link

Choose a reason for hiding this comment

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

Enhance error messages to provide more context in deletion operations.

-    NSLog(@"Error deleting item from keychain. Status code: %d", (int)status);
+    NSLog(@"Error deleting item from keychain. Status code: %d. Ensure the item exists and is accessible.", (int)status);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// find existing:
NSDictionary *searchAttributes = @{
(__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
};
CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)searchAttributes, (CFTypeRef *)&result);
if (status == errSecSuccess && result != NULL) {
status = SecItemDelete((__bridge CFDictionaryRef)searchAttributes);
} else if (status != errSecItemNotFound) {
NSLog(@"Error deleting item from keychain. Status code: %d", (int)status);
}
(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);
if (itemRef) {
CFRelease(itemRef);
}
// find existing:
NSDictionary *searchAttributes = @{
(__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
};
CFDictionaryRef result = NULL;
OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)searchAttributes, (CFTypeRef *)&result);
if (status == errSecSuccess && result != NULL) {
status = SecItemDelete((__bridge CFDictionaryRef)searchAttributes);
} else if (status != errSecItemNotFound) {
NSLog(@"Error deleting item from keychain. Status code: %d. Ensure the item exists and is accessible.", (int)status);
}
(*env)->ReleaseByteArrayElements(env, service, serviceStr, JNI_ABORT);
(*env)->ReleaseByteArrayElements(env, key, keyStr, JNI_ABORT);

return status;
}
Loading