maxgoedjen / secretive

Store SSH keys in the Secure Enclave
MIT License
7.23k stars 159 forks source link

Remove password option and require biometric auth #412

Open webtoolbox opened 2 years ago

webtoolbox commented 2 years ago

It would be nice if there was a way to only allow the SSH key to be used instead of the "Use password" button being shown as an alternative method.

zachriggle commented 2 years ago

I think what @webtoolbox means is to always require TouchID, and disable password based auth?

webtoolbox commented 2 years ago

Yes, that's correct. To remove the "Use Password" option from this dialog.

Screen Shot 2022-10-18 at 10 58 57 AM
maxgoedjen commented 2 years ago

@zachriggle @webtoolbox thanks for clarifying :)

That UI is entirely provided by the system, I don't THINK there's anyway to do that. Just out of curiosity, what's your use case?

webtoolbox commented 2 years ago

Just would rather not allow a password alternative. It's less secure.

maxgoedjen commented 2 years ago

So this is technically possible by passing .biometryAny here https://github.com/maxgoedjen/secretive/blob/a1009d0dacb63d19c522ba1cb555f0399081c017/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift#L38

Screenshot 2022-10-23 at 2 18 50 PM

I'll consider adding this as a creation option in the future.

maxgoedjen commented 2 years ago

Basically my reservation is this: Users can use their password to remove/add Touch ID sets, which kinda makes the protection moot. There is an option called .biometryCurrentSet which would prevent this, but if my reading of that is correct, adding or removing a Touch ID finger would delete the item, which is probably not a behavior I'm okay with.

webtoolbox commented 2 years ago

Thanks for looking into this. That's great that it can be removed. For me and probably others, I would be fine with it removing the item if a Touch ID finger is added or removed. It probably depends on the user's use case. A visible warning when the feature is enabled would be good. It would be very useful to me if this could be added as an option for existing items as well though.

maxgoedjen commented 2 years ago

I would be fine with it removing the item if a Touch ID finger is added or removed.

Main concern is that it's not really something I could adequately warn people of – like the flow is basically macOS System Preferences -> Security -> Touch ID -> Add Finger -> poof, your items are gone.

Yeah, you could warn people about that when creating the key, but realistically people will not read that or remember it when they're modifying their security settings outside the app.

maxgoedjen commented 2 years ago

It would be very useful to me if this could be added as an option for existing items as well though.

That is almost definitely not possible, it's an option you pass at creation so it would only be applicable to new items if this is implemented.

webtoolbox commented 2 years ago

Ok, no problem. Thanks for considering it.

maxgoedjen commented 2 years ago

I'll leave the ticket open for now while I give it some more thought. I could certainly see legitimate use cases for it, but need to weigh that against risks.

webtoolbox commented 2 years ago

Yea. Typically a single user account on a Mac will be used for a single person. I added my fingerprint when setting up the mac and haven't added or removed the fingerprint in more than 10 years through multiple different macs. It seems like a rare case, and wording the option cautiously could help avoid it.

wssheldon commented 1 year ago

I wanted to pop in here and show support for adding this functionality. Could this be considered an "advanced feature" that would be disabled by default, configurable by the user, and in this flow the warning of data loss would be promptly shown? I'll echo that I have never set my fingerprint twice and the security benefit here is nothing to scoff at.

sandstrom commented 1 year ago

I'd like to chime in with an example of where biometryCurrentSet would be great.

For a company with many machines, a SSH key is easy to replace. We can just recreate and ask an admin to whitelist the new public key. Basically no problem with wiping keys when a finger is added (which never happens after setup in my experience, btw).

Enforcing that only fingerprint access is accepted would be great.

Maybe if a CLI tool was added, this could be a configurable flag in that CLI tool. That would make it more of an advanced feature.

KizzyCode commented 1 year ago

I'd also like to see this feature, because biometry is a much better indicator of user presence than a password, since gaining access to a password usually is a lot easier than creating a fake-finger or chopping it off.

As for UX, I'd recommend to use the usual macOS-flow: A normal click on the plus shows the current menu, whereas an option-click would show an advanced menu. This is quite common in other applications and macOS itself.

Regarding the risk of accidental date/key loss: Since you cannot backup SEP keys, you must consider them ephemeral at any time, and you always need a plan B. If you a) forget about the very prominent warning and b) change your enrolled fingerprints (people usually never do this anyway), and c) this results in a problem for you (loss of access etc.), something went very wrong in the first place anyway. (However I fully agree that this should be an "expert" or hidden feature and not a default or something recommended.)

lduck commented 1 year ago

Hello, I came across this ticket while searching for more information about the possibility of exploiting SEP keys in the event that my MacBook is compromised remotely (for example, through a security vulnerability), without requiring my physical presence at my macbook by using Touch ID for authentication.

My understanding of the flag userPresence is, that it only validates if someone is there and will confirm (with password or biometric) use of ssh key. It can be and attacker with some kind of remote access. Is that correct?

I have trouble to find out, how will the access to the SEP keys behave with with flag biometryAny and without biometryCurrentSet if all the fingers are removed. Can someone point me to documentation about this? Or did someone test that?

I'm ok with the possibility, that the users/attacker can use password to add Touch ID, as my main concern is to be protected against remote attack. But even using biometryCurrentSet is acceptable for most scenarios I can imagine. By my opinion, using SEP keys for authentication is really good and convenient for fast and secure everyday use, but everyone should have backup way of authentication if something happens to those SEP keys (Anything that can go wrong will go wrong, even macbook 😰 )

KizzyCode commented 1 year ago

@lduck Not the maintainer, but to answer a few common questions. There are in fact three different flags to require user presence (SecAccessControlCreateFlags):

  1. .userPresence: The user must interactively confirm the key usage. This might happen via password entry or valid biometry. Since the password entry happens via macOS, it is possible to exploit this authentication level remotely (provided the attacker has sufficient access to macOS itself and knows or has phished the password).

  2. .biometryAny: This requires interactive authentication via biometry. The interesting part is that the authentication itself bypasses macOS completely: The TouchID/FaceID-sensor communicates with the secure enclave via an encrypted channel, so a remote attacker cannot exploit it until it also exploits either the TouchID/FaceID sensors or the secure enclave itself.

    An attacker with sufficient access to macOS and who knows or has phished the password could trigger a biometry unenroll, but this is not useful: If all the fingers are removed, you simply cannot use the key until you re-add at least one finger. An attacker could enroll it's own biometry and use it to authenticate, but there is no way to bypass physical presence (unless you have a remote secure enclave or sensor exploit).

    This brings us to the last scenario: An attacker with sufficient access to macOS and who knows or has phished the password could trigger an additional biometry enroll; but this would also require human interaction so it's not useful as a remote exploit. However if a malicious actor is physically present and enrolls additional biometry, it can use it to authenticate and use the key.

  3. .biometryCurrentSet: This also requires interactive authentication via a biometry. It is mostly similar two 2, but with an additional twist: You cannot use the key with newly enrolled biometry. This means, even if someone knows your password and uses it to enroll new biometry, it cannot authenticate via this new biometry to use the key. This is especially useful to protect against people who might know or gain access to your password and device (e.g. colleagues, significant others, children, ...). Without this setting, such an attacker could register it's biometry, perform some actions and unregister it again, leaving no trace to what happened.

By my opinion, using SEP keys for authentication is really good and convenient for fast and secure everyday use, but everyone should have backup way of authentication if something happens to those SEP keys (Anything that can go wrong will go wrong, even macbook 😰 )

Full ack; it's important to notice that the key is always tied to some user state and your hardware. If you cannot use your hardware anymore for any reason (defective, superseded, stolen, ...), you need a plan b (e.g. a second device, a YubiKey, a paperkey, physical access, your IT department, ...).

0xmachos commented 6 months ago

@KizzyCode do you have a source for the following

The interesting part is that the authentication itself bypasses macOS completely: The TouchID/FaceID-sensor communicates with the secure enclave via an encrypted channel, so a remote attacker cannot exploit it until it also exploits either the TouchID/FaceID sensors or the secure enclave itself.

Cheers

KizzyCode commented 6 months ago

@KizzyCode do you have a source for the following

@0xmachos see e.g. https://support.apple.com/guide/security/face-id-and-touch-id-security-sec067eb0c9e/1/web/1, last paragraph 😊