keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.39k stars 1.48k forks source link

Device Password fallback when Touch-ID devices are unavailable #11410

Closed findus closed 1 week ago

findus commented 4 weeks ago

Device Password fallback when Touch-ID devices are unavailable

I often use my macbook docked with the lid closed, so I cannot use the fingerprint sensor, and I got rather tired reentering my whole database password every time after unlocking my session.

This change adds the "kSecAccessControlDevicePasscode" flag to the AccessControlFlags, so it is possible to use the device password to unlock the database again.

~This feature is disabled by default and can be enabled in the settings menu.~

Screenshots

https://github.com/user-attachments/assets/529425a1-18a9-4c84-83ad-9ba958eddac7

CleanShot 2024-10-25 at 11 52 24@2x

Type of change

droidmonkey commented 4 weeks ago

Oh neat, didn't even know that was an option. I don't think we need a special setting to toggle it though. Recommend removing that part and just making this part of the default security control options.

findus commented 4 weeks ago

@droidmonkey ok then I'll remove the settings menu entry again.

Having this default fallback, that is available all the time, we could also get rid of the isWatchIDAvailable/isTouchIdAvailable checks altogether.

Doing this TouchID/WatchID unlock would always be possbile, even when you open the database for the first time with no biometric/apple watch hardware present and open the lid later.

findus commented 4 weeks ago

Oh, its probably saver to keep all checks in place and add another one with https://developer.apple.com/documentation/localauthentication/klapolicydeviceownerauthentication, I'll do that later this day

findus commented 3 weeks ago

It was not as easy as I thought, the kSecAccessControlBiometryCurrentSet flag is only allowed when TouchID is enrolled, adding the flag while its not enrolled results in an error when trying to save the secret to the keychain.

But it is possible to catch the specific "is not enrolled" error, and only skip adding the flag when this one occurs.

The kSecAccessControlWatch seems to not have this constraint, but I need to test it a little more, same as my current implementation for TouchID.

findus commented 3 weeks ago

I tested i a little more and it seems to be pretty reliable, the only dangerous path is trying to save the key to the enclave with kSecAccessControlBiometryCurrentSet or kSecAccessControlTouchIDCurrentSet as the saving logic is throwing an error when having no fingers scanned, but that path is covered with the new touchidEnrolled check.

With this new logic it is now possible to:

But I dont have an iMac or Mac Mini to test what happens on devices with no integrated TouchID, I tested this constellation in a qemu vm and this case also got handled by the kLAErrorBiometryNotEnrolled error, but I would feel saver if it could get tested with actual imac/mac mini hardware once.

I also could get my hands on a magic keyboard on monday to test that aswell at least once.

droidmonkey commented 3 weeks ago

Awesome thank you @findus, this will greatly improve usability of quick unlock. Also helps when I finish implementing "remembered" quick unlock.

findus commented 3 weeks ago

I've set up a Hackintosh yesterday to check what happens on devices with no TouchID present at all, there the "isEnrolled" check does not work because the API returns LAErrorBiometryNotAvailable, which kinda sucks because now this error is used for two states:

To "fix" this I just call setKey a second time without the TouchID/Biometrics flag when the first call fails, that makes it a little more robust.

findus commented 2 weeks ago

I got my hand on an iMac and I tested the behavior if there is no TouchID hardware present, and what happens if you connect a magic keyboard with sensor to it.

It kinda behaves like on a Hackintosh, only the error message is different, here "kLAErrorBiometryNotPaired" is returned the first time setKey is called, the 2nd fallback call also enables us to use WatchId/DeviceCode.

After you pair the keyboard MacOS guides you through the fingerprint setup process, after that its the same behavior as with a macbook.

After you Unpair the keyboard again and delete your Fingerprints, the API returns "kLAErrorBiometryNotEnrolled" instead of "kLAErrorBiometryNotPaired" 🤔.

But in general its pretty stable now, I used this build for about a week now and haven't encountered anything weird.

droidmonkey commented 2 weeks ago

Awesome and thank you for the comprehensive testing! Will merge this shortly.

droidmonkey commented 1 week ago

Thank you for the contribution!