Feature: TouchID Security support for the Screen Security Privacy #663
Conversation
… document switch statements
…erty declarations. [Does not change interface]
fbartho
changed the title
|
Nice work, although I would prefer a solution using kSecAccessControlUserPresence on a keychain item which contains the key needed to decrypt my authentication token / stored messages, rather than LocalAuthentication framework. LocalAuthentication can be bypassed on a jailbroken device and does nothing for data stored at rest. |
|
@hubert3 this pull request is purely a privacy feature akin to the existing "Screen Security" privacy setting. This PR doesn't change any data model involved, else that would be a much more complex feature. Additionally, jail broken devices can't be presumed to have any security whatsoever. Key loggers or keychain, all are insecure. This PR isn't attempting to address or debate that question in any way. In a non-jail broken context, the only "trusted" party is Apple. Is the PR as given useful to you? Changing the whole security model seems out of scope for my available time. |
|
I haven't tried it yet, but it sounds like a nice addition. I agree my suggestion would be a larger change. I'm just wary of people using LocalAuthentication inappropriately or promising more security benefit from it than it really delivers, see e.g. this analysis of how Dropbox uses it: http://www.andreas-kurtz.de/2014/10/ios-8-touch-id-local-authentication-caveats.html |
|
Hey @fbartho, Thanks for this great contribution. I will fully review your pull request tomorrow. I'm focusing on some critical bug fixes today. I think that we do want it to be part of the keychain authentication in the future. This could be a good temporary fix for people desiring local authenticating if we make it clear about how it works and under what threat model it's useful. |
|
@FredericJacobs I think LocalAuthentication would be a nice extra security measure for screen lock once the app is already running and has opened the database (i.e. dbPassword is in memory) Longer term I think it would be great if dbPassword could (optionally) have the kSecAccessControlUserPresence flag applied to it. This means you'd have to provide a fingerprint or re-enter the device passcode at app launch to be able to decrypt the database, which offers better security than kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly which is used at the moment (see line 145 TSStorageManager.m). iOS will transparently pop up the fingerprint dialog when you try to access a keychain item with that flag applied, so you don't have to explicitly do anything to check the fingerprint. Device passcode re-entry instead of fingerprint is always possible as a fallback and can't be disabled. |
|
@fbartho: I tested it on a iPhone 5S where TouchID isn't enabled and it allowed me to toggle the TouchID setting. Then when the user opens the app, it stays stuck on the launch screen. |
FredericJacobs
force-pushed
the
master
branch
2 times, most recently
from
407641c
to
82a9029
Compare
|
Looks like this is stalled. Closing. I'm happy to review another rebased PR with the mentioned issues addressed. |
|
Yeah, @FredericJacobs I had limited contributor availability. Sorry I wasn't able to further advance the changes on this. Cheers, and good luck. |
|
Hi! Would people have time to review this PR if I rebased it first? Also, how'd y'all feel about including a link to something like Police Can Force You to Use Your Fingerprint to Unlock Your Phone (or an ACLU equivalent), maybe under the toggle switch, as a risk reminder to users? Edit: I think the link suggestion falls under the "Please do not use this issue tracker as a discussion forum" section of the contributing guide. Sorry about that. |
Yeah, do it. |
|
Great, I'll get started. (PS, I think merging the string localization PR might make rebasing a bit easier here, but I can work around it for now!) |
|
+1 for this feature |
Sometimes people hand their device to others, or have them stolen while unlocked. This feature allows a user to lock access to the Signal App to only be used via verified biometric fingerprint.
A future improvement could enable the "Fallback Password" feature of LocalAuthentication, but to do that would require a more complex UI state-machine, and the addition of either a pod or a viewcontroller to handle that. This would require more complexity in settings, and keychain management.
Some Localizable strings were added, following existing project conventions, but they were only added to the english strings file, please let me know if you need a separate listing.