Report keychain load errors and keep Touch ID entries protected

CHANGELOG.md

@@ -8,8 +8,14 @@ The changelog starts with version 1.4.1.
 Changes to prior versions can be found on the [Github release page](https://github.com/cryptomator/integrations-mac/releases).
 
 ## [Unreleased](https://github.com/cryptomator/integrations-mac/compare/1.5.1...HEAD)
+### Changed
+* Touch ID keychain falls back to the device password when biometrics are unavailable
+* Loading a stored password reports errors, e.g. cancelled authentication, instead of reporting no stored password
+* Require macOS 11
 
-No changes  yet.
+### Fixed
+* Touch ID keychain entries that were stored without authentication are now protected when loaded
+* Possible out-of-bounds read when converting keychain service and account names
 
 
 ## [1.5.1](https://github.com/cryptomator/integrations-mac/releases/tag/1.5.1) - 2026-10-06

src/main/java/org/cryptomator/macos/keychain/MacKeychain.java

@@ -20,8 +20,8 @@ class MacKeychain {
 	 * @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>
+	 * @param requireOsAuthentication Defines, whether the user needs to authenticate to load the stored passphrase. If <code>false</code>, an existing entry requiring authentication keeps its protection, and updating it requires the user to authenticate.
+	 * @see <a href="https://developer.apple.com/documentation/security/secitemadd(_:_:)">SecItemAdd</a>
 	 */
 	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 <code>null</code> if no such keychain entry could be loaded from the keychain.
-	 * @see <a href="https://developer.apple.com/documentation/security/1397301-seckeychainfindgenericpassword">SecKeychainFindGenericPassword</a>
+	 * @return password or <code>null</code> 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 <a href="https://developer.apple.com/documentation/security/secitemcopymatching(_:_:)">SecItemCopyMatching</a>
 	 */
-	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 <a href="https://github.com/cryptomator/integrations-mac/issues/13">Issue 13</a>.
-	 * Attempts to find the account with the old hard-coded service name 'Cryptomator\0'
-	 *
-	 * @param account Unique account identifier
-	 * @return <code>true</code> 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 <code>true</code> if the passwords has been deleted, <code>false</code> if no entry for the given key exists.
-	 * @see <a href="https://developer.apple.com/documentation/security/1395547-secitemdelete">SecKeychainItemDelete</a>
+	 * @see <a href="https://developer.apple.com/documentation/security/secitemdelete(_:)">SecItemDelete</a>
 	 */
 	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 <code>true</code> if biometric authentication is available, <code>false</code> otherwise
+	 * @return <code>true</code> if passwords requiring user authentication are supported, <code>false</code> 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();
 	}
 
 }

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

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.
  * <p>
- * Items are stored in the default keychain with the service name <code>Cryptomator</code>, unless configured otherwise
+ * Items are stored in the data protection keychain with the service name <code>Cryptomator</code>, unless configured otherwise
  * using the system property <code>cryptomator.integrationsMac.keychainServiceName</code>.
  */
 @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

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;