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

Latest build does not compile #8

Closed
wuf810 opened this issue Nov 10, 2017 · 15 comments
Closed

Latest build does not compile #8

wuf810 opened this issue Nov 10, 2017 · 15 comments

Comments

@wuf810
Copy link

wuf810 commented Nov 10, 2017

Compilation error.

I guess you mean to be returning "x9_62HeaderECHeader"

screen shot 2017-11-10 at 09 05 26

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

Yep. Sorry about that. Fixed now.

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

That was the fastest bug report ever :D Literally minutes after I pushed. Thanks for reporting this issue. 🎉

@hfossli hfossli closed this as completed Nov 10, 2017
@wuf810
Copy link
Author

wuf810 commented Nov 10, 2017

No problem. Great code btw. Been working with ECC keys the last few weeks and your project has been very helpful!

However I'm still seeing issues with using LAContext and setting touchIDAuthenticationAllowableReuseDuration not working the way I expect it to. Have you done anything with this?

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

Hey. Thanks for the feedback. That's very valuable to me.

I haven't tried using touchIDAuthenticationAllowableReuseDuration with this library. What's the current behavior and what are you expecting?

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

(latest changes is published as 1.0.1 btw)

@wuf810
Copy link
Author

wuf810 commented Nov 10, 2017

Basically I want to be able to create a LAContext, with touchIDAuthenticationAllowableReuseDuration set to LATouchIDAuthenticationMaximumAllowableReuseDuration that I can pass into any queries (kSecUseAuthenticationContext) where I have locked down the keys with .userPresence.

This means the user can authenticate just once for multiple queries during this period.

But what I am seeing is that the context never "expires" as such. So despite for example setting the LATouchIDAuthenticationMaximumAllowableReuseDuration to 30 seconds, after that time the TouchID authentication is not displayed.

I guess I was expecting it fail, at which point I could re-initialise the context for another period.

To be honest I'm not 100% clear how the context is supposed to work. Whether the context is set to nil after the expired period. There seems no way to get back whether it is still valid or not (although there is a way to invalid a context).

Maybe LAContext is not supposed to be used this way with .UserPresence. Not sure. If you have any thoughts/idea I would be love to hear them.

@wuf810
Copy link
Author

wuf810 commented Nov 10, 2017

BTW My report may be have been the fastest ever but so was your fix :-)

@wuf810
Copy link
Author

wuf810 commented Nov 10, 2017

Just done some more tests. Seems regardless of what I set the touchIDAuthenticationAllowableReuseDuration to, it only "times" out after 10 minutes! Once authenticated again, a period of 10 minutes becomes active again.

If at some point you have a chance to test this yourself, it would to know if you are seeing this too.

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

Thanks for waiting. I have been testing quite extensively myself. I haven't checked after 10 minutes, but I'm glad to hear there's a limit.

I guess I was expecting it fail, at which point I could re-initialise the context for another period.

That would be my assumption as well. Local Authentication + Security framework seems really buggy...

Also, when reading the documentation on touchIDAuthenticationAllowableReuseDuration it states The default value is 0, meaning that no previous TouchID unlock can be reused., but I am experiencing that it can in fact be reused as many times I want.

This is superweird. I have no explanation. If you file a bugreport to Apple, please dupe in on openradar and paste a link here.

I have fixed some bug I introduced after refactoring. I don't know if you encountered more problems, but master and 1.0.2 should be fine now. Sorry for any inconvenience.

@hfossli
Copy link
Contributor

hfossli commented Nov 10, 2017

Also, I recommend opening a TSI to Apple. They might provide some valuable insight.

If you need to fix this now I would recommend to just code your timeout logic yourself as it is better to have some measures than no measures.

@wuf810
Copy link
Author

wuf810 commented Nov 13, 2017

I’m still doing some testing (which isn’t easy when having to wait 10 minutes for the session to time out).

I’ll raise a bug report once I’m sure of exactly what’s happening. But I think you’re right I that super buggy is the best description.

@wuf810
Copy link
Author

wuf810 commented Nov 23, 2017

FYI. I finally got round to filing that bug report.

http://www.openradar.me/radar?id=6060524114542592

@hfossli
Copy link
Contributor

hfossli commented Nov 23, 2017

Awesome. I like that very much.

I found something in the documentation. It states If the specified context has been previously authenticated, the operation will succeed without asking user for authentication.

    @constant kSecUseAuthenticationContext Specifies a dictionary key whose value
        is LAContext to be used for keychain item authentication.
        * If the item requires authentication and this key is omitted, a new context
          will be created just for the purpose of the single call.
        * If the specified context has been previously authenticated, the operation
          will succeed without asking user for authentication.
        * If the specified context has not been previously authenticated, the new
          authentication will be started on this context, allowing caller to
          eventually reuse the sucessfully authenticated context in subsequent
          keychain operations.

That constant is the one I use when I pass the LAContext.

@wuf810
Copy link
Author

wuf810 commented Nov 23, 2017

I agree but the context is an LAContext and elsewhere in the documentation it implies that if the touchIDAuthenticationAllowableReuseDuration is set then the context should respect it and require a dialog after the duration has "expired".

I'm guessing that in general use LAContext does work like this but when used with the SecItemCopyMatching query it is just applying the rules as documented above i.e. if authenticated then remain authenticated. This is the behaviour that would have been implemented prior to iOS 9 and prior to touchIDAuthenticationAllowableReuseDuration being added.

I guess the best we can hope is that either it is an oversight/bug and they fix it or they at least update the documentation to be clear.

@hfossli
Copy link
Contributor

hfossli commented Nov 23, 2017

Yep

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

No branches or pull requests

2 participants