github / SoftU2F

Software U2F authenticator for macOS
MIT License
2.21k stars 160 forks source link

Optionally store keys in SEP #29

Closed btoews closed 7 years ago

btoews commented 7 years ago

By popular demand, this PR adds the option to store keys in the SEP. This setting can be toggled by invoking the binary with --[en|dis]able-sep flags.

For those who are interested in this feature, I'm curious for feedback on the ACL options people would find most useful. The current setup is:

SecAccessControlCreateWithFlags(
  nil, 
  kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly,
  [.privateKeyUsage, .touchIDCurrentSet],
  &err
)

This configuration is quite strict. If you temporarily remove your password or add any new fingerprints to your Touch ID enrollment, all your U2F keys will be deleted. This means that even if someone steals your laptop and gets your password, they still wont be able to access the U2F keys... It also increases risk though of you accidentally deleting your U2F registrations.

You can find docs for kSecAttrAccessible values in SecItem.h and docs for SecAccessControlCreateFlags values in SecAccessControl.h.

I don't have one of the new MacBooks handy at the moment, so this code probably doesn't work. I should be able to test this branch out in the next couple weeks though.

btoews commented 7 years ago

I got a TouchID equipped computer and got this branch working. I decided that it was confusing and unnecessary to have options for both using SEP-backed keys as well as Keychain-backed keys with TouchID for user presence checks. This PR now removes the option to use TouchID unless keys are stored with the SEP.

I still want to do a bit of testing to make sure that the ACLs I chose actually work the way they're supposed to. I'll try to get this merged sometime next week.

lukegb commented 7 years ago

I'm not 100% convinced that wiping the U2F keys on changing touch-ID adds any useful extra security for most people, and is potentially dangerous. Would you mind justifying why you chose that ACL over e.g. touchIDAny?

The ACL you're proposing is possibly quite surprising, particularly if people have only registered SoftU2F - I would find losing all my 2FA access after changing the registered set of fingerprints somewhat unexpected.

In addition, the threat model this extra security is defending against seems fairly narrow - someone who has both physical access to your device and knows your password could do a lot of harm just by making use of the sessions you already have open. I do wish that Apple required using an already registered fingerprint to add more fingers to Touch ID though.

I would argue that a lesser security mode which is roughly equivalent to leaving a small U2F key inside the device at all times is probably perfectly adequate for most people, since most of the risk can be mitigated by choosing a strong, unique password for your Mac anyway.

But then, I'm not a security professional (and don't actually own a Mac either) so that's just my 2c. Take it or leave it :)

btoews commented 7 years ago

Thanks for your input @lukegb. My initial thought was also to use touchIDAny for the reasons you mention. I'm not planning on using this feature though, mostly because I spend my days working on an external monitor with my laptop closed and no easy access to the TouchID element. Also, I feel like the usability constraints around SEP-backed keys are pretty harsh, regardless of the ACL. For example, it's unclear if there's any way to backup SEP-backed keys, even if the backup will later be used on the same device (eg. when reinstalling the OS).

I see this as somewhat of a fringe feature for security enthusiasts or those who face a different set of threats than I do. For those groups, the extra assurance of touchIDCurrentSet might make sense. It mitigates against a (for me) super fringe scenario where an attacker has physical access to your device as well as your device password. With touchIDAny, such an attacker could unlock the device, add a fingerprint, use a SEP-backed U2F key, and remove the fingerprint, covering their tracks. Is this attack worth mitigating? I don't know.

lukegb commented 7 years ago

I don't think macOS is designed to be secure against "evil maid"-style attacks where you lose physical control of the device, so I'm not sure that it adds any meaningful level of security for this use case. If an attacker has that level of access to your device, they could also install software on your Mac which snarfs up your cookies every time you authorise a login using U2F and sends them back to the attacker.

I do appreciate that it's potentially a bit of a fringe feature, and it's entirely possible I've overlooked an attack vector that touchIDCurrentSet would mitigate over touchIDAny.

tarcieri commented 7 years ago

To me the goal of U2F is to have an inextricable device-specific key, with the idea being if we think a particular device is compromised, it can be removed from an account.

In that regard, I would definitely prioritize UX over further hardening. If someone steals your laptop with a Yubikey Nano permanently in its USB slot, they can certainly still use it, and to me the proper response is to revoke stolen tokens.

I would not look at the SEP as any sort of "Tinfoil Hat" feature, and would suggest using touchIDAny. Biometric auth is somewhat weak to begin with, so I'm not sure it makes any sense to harm UX in an attempt to defeat an attacker who has physical control of a TouchID device. -- Tony Arcieri

btoews commented 7 years ago

Biometric auth is somewhat weak to begin with, so I'm not sure it makes any sense to harm UX in an attempt to defeat an attacker who has physical control of a TouchID device.

That reasoning makes sense to me.

btoews commented 7 years ago

I cut a beta-release with these changes. After installing this release, SEP storage will still be disabled by default (that default may change eventually). There are new command line flags for enabling/disabling SEP storage, seeing the status of the setting, and listing registrations from the keychain.

$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F --show-sep
$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F --enable-sep
$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F --disable-sep
$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F --list
ejholmes commented 6 years ago

FWIW, the above commands didn't initially work for me, since I was running into https://github.com/github/SoftU2F/issues/39, and getting an error when it tried to connect to the kext.

$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F 
Error connecting to SoftU2F.kext: -536870203
Error starting authenticator

The following worked:

$ launchctl unload ~/Library/LaunchAgents/com.github.SoftU2F.plist
$ /Applications/SoftU2F.app/Contents/MacOS/SoftU2F --enable-sep
$ launchctl load ~/Library/LaunchAgents/com.github.SoftU2F.plist

After that, Touch ID U2F works great.

exabrial commented 6 years ago

I had this working on another mac, but it seems to have been broken :/

btoews commented 6 years ago

@exabrial Can you provide more details on what isn't working please?

andymartin-sch commented 5 years ago

I did not extensively test this, but in addition to the instructions in this comment, I had to restart my computer before the SoftU2F installation would work again. (May be the cause of the issue mentioned by @exabrial.)

dconnolly commented 4 years ago

Because this bit me, I'm commenting to add that SEP will not enable unless you have at least one fingerprint registered with Touch ID, even if it's disabled for all macOS capabilities, and the 'use-SEP' flag will un-set as soon as you remove the fingerprint.

zofrex commented 4 years ago

@mastahyeti you asked for feedback on which ACLs would be useful. I think it would be handy to allow users to enter the device passcode instead of TouchID if they want – it's typical to allow users to use one or the other and I can't think of a reason not to allow passcode, plus it would provide a way for people to still use the SEP when the Macbook is docked & closed. It would also address what @dconnolly ran into above.

I think the ACL for that would be: [.privateKeyUsage, .touchIDAny, .or, .devicePasscode].

On the same note of using while the Macbook is docked, these days it's also possible to allow an authorised Apple Watch to unlock the SEP. I've been trying this out as an option as the TouchID sensor is useless for me, as my Macbook is docked 99% of the time. I think it would be great to allow that as well (as I understand the feature it'll only be an option if the user has enabled it in OS settings, so people who don't want it won't get it).

The option for that is .watch, although in my testing that seems to come along with the ride along with touchID and passcode if you use: [.privateKeyUsage, .userPresence]

If there's a good reason to not allow passcode but it's still ok to enable the Apple Watch, that would be: [.privateKeyUsage, .touchIDAny, .or, .watch], I believe.

† (The Apple documentation for userPresence says it's equivalent to "biometryAny, or, and devicePasscode" but in my testing it seems to also include "watch")

If any of these options sound good to you I'm happy writing up a PR for the change. Thanks!