Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecAccessControlCreateFlags missing touchIDAny/biometryAny flag ? #1489

Open
udf2457 opened this issue Jun 20, 2022 · 2 comments
Open

SecAccessControlCreateFlags missing touchIDAny/biometryAny flag ? #1489

udf2457 opened this issue Jun 20, 2022 · 2 comments
Labels
feature Feature requested by users

Comments

@udf2457
Copy link

udf2457 commented Jun 20, 2022

I'm not sure if you would classify this as a feature request or a bug, but I've put it down as a bug as its (potentially) quite an important security flag missing from SecAccessControlCreateFlags.

I'm no iOS developer, but I was interested to see blinksh implements secure enclave storage of keys, and so I wanted to check how this was actually implemented.

My understanding of the code is that the magic happens on Line 65 of SEKey.swift.

I also note this from the Apple dev docs (my highlighting of the second phrase of the paragraph):

You could also combine the privateKeyUsage flag with other flags, using a bitwise OR to obtain additional protection for your key. For example, if you include the touchIDAny flag, you instruct the system to make the key available only when the system can authenticate the user with Touch ID (or a fallback passcode). See SecAccessControlCreateFlags for the complete list of available flags.

However, searching the blinksh code for touchIDAny (or indeed biometryAny since the touchIDAny form seems to be deprecated) yields no results.

I would have thought it was somewhat important that blinksh required touchID/biometry before using a key ? (Or at the very least give the user the option to mandate it).

@udf2457 udf2457 added the support Help needed from the maintainers of the repository label Jun 20, 2022
@udf2457 udf2457 changed the title SecAccessControlCreateFlags missing flag ? SecAccessControlCreateFlags missing touchIDAny/biometryAny flag ? Jun 20, 2022
@carloscabanero carloscabanero added feature Feature requested by users and removed support Help needed from the maintainers of the repository labels Jun 20, 2022
@carloscabanero
Copy link
Member

First of all, don't think it is a security bug, more like a feature. The key is still protected by the SE and it cannot be read, Blink is still the only app which can send "signature" requests to that key. Enabling biometryAny is about using what is considered "proof of user presence", but it is not a requirement of Hardware keys themselves (Yubikeys for example are ok with the user just "tapping a button", so definitely on the very low end).

We should definitely add this as a feature though. It is one extra step for security, as if your device is left unattended AND unlocked, and you also don't have Blink protected, someone could use it to log in to one of your servers and possibly install a key.

@carloscabanero
Copy link
Member

Also, we should create a "Security" tag in here as well, to give a path to request like yours. Thanks a lot for taking a look at our code! That's the beauty of having an open source shell :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature requested by users
Projects
None yet
Development

No branches or pull requests

2 participants