Can't check multiple certs at the same time

[jacobweber]

If you try to call the iOS plugin twice, to check two different servers, it overwrites its _allowedFingerprint and _allowedFingerprintAlt properties the second time. So if the first check hasn't completed by the time the second starts, it will fail.

Haven't tried the Android plugin to see if it has the same problem.

[EddyVerbruggen]

First off: well spotted, I didn't consider this case (checking two different servers). This should be fixed.

For a temp fix: you could delay checking the second server by calling the plugin in the success/error callback of the first call.

[jacobweber]

I tried doing them in sequence, but discovered some other bugs.

• Sometimes the second request times out (it calls didFailWithError with "The request timed out"). I'm not sure why this happens, but I think it's because willSendRequestForAuthenticationChallenge never calls any of the required methods, like performDefaultHandlingForAuthenticationChallenge. The timeout doesn't happen if I add those calls. Maybe it's blocking the second request until the first one deals with the authentication challenge.
• Even if I fix that, sometimes willSendRequestForAuthenticationChallenge isn't called. It seems like iOS may cache the result in certain cases. So if I test it twice with the same URL, it doesn't always call this method the second time. It still calls connectionDidFinishLoading, though. So I think you need to implement that method, and make it return something to the JS layer. I don't know of any way to clear the cache; you might just have to assume that the certificate is valid, or return some "unknown" message.

I might try to fix these myself if I have time.

[EddyVerbruggen]

That would be awesome as I'm not sure when I'll be able to take a look.
Thanks for the pointers anyway!

[jacobweber]

I attempted to fix these issues -- see this pull request. But please take a careful look at them -- my knowledge of Objective C is next to nil. I'm sure I'm leaking memory somewhere.

[jacobweber]

BTW, there's some discussion of the caching issue here. When I see this behavior, the cache does seem to be cleared after 10 minutes, as they mention.

[EddyVerbruggen]

Hi Jacob, I've merged your PR and decided to not break current usage, so when the request was cached by iOS (found no way around it) the connection is considered secure (like it was before).

The Android implementation already supported consecutive certificate checks, so I think we're done.

Thanks you very much, I've added you to the credentials in the readme for your efforts.

[jacobweber]

Great, thanks!

[EddyVerbruggen]

Hi Jacob, just checked in a little change for two reasons:

• When an invalid fingerprint was used the result was always 'connection_failed' instead of 'connection_unsecure' due to this line:
  [[challenge sender] cancelAuthenticationChallenge:challenge];
• The caching issue seems to be solved by removing this line:
  [[challenge sender] performDefaultHandlingForAuthenticationChallenge:challenge];

can you perhaps let me know what the intended use was and if we can do without it? It was never in the original code.

Thanks,
Eddy

[jacobweber]

Hi. See my comment above beginning "Sometimes the second request times out". Adding them seemed to fix the timeout issue, although it could have been just a coincidence. But according to Apple's documentation, you have to call one of them from connection:willSendRequestForAuthenticationChallenge.

[EddyVerbruggen]

Hi Jacob, thanks for the pointer to the docs. You're right, Apple says you must call one of the methods in the delegate, but if you do part of the purpose of this plugin is defeated. I think in a normal workflow it makes sense to call one of the methods, but deciding on still calling the server is up to the programmer and is decided in the callback of the plugin. So it doesn't really fit a normal app workflow I guess.

So after testing all of the 5 listed methods of which you must invoke one (which all result in the same problems as described before) I decided to not invoke any of the methods.

I haven't seen the timeout myself, I do think it was a coincidence.

Thanks for your help and ideas!

[jacobweber]

Is the workflow really different, though? You're making a request in the Objective C layer to examine the SSL cert. It should be safe to always cancel that request, since you don't care about the result. If you decide to make an HTTP request in the JavaScript layer, that's going to be a completely different call -- you're not continuing the one you started in the Objective C layer.

It seems strange that you'd get an error by using the method Apple recommends. Are you sure there's not something else going on?

[EddyVerbruggen]

If I invoke the cancelAuthenticationChallenge method the didFailWithError method will be invoked immediately. Not good.

I don't see a problem with not calling the methods. The plugin has never done it and Apple doesn't complain about it. I can stand upsidedown all day and don't like the situation or just accept it as it is.

[jacobweber]

I see. I think that was why I added the sentResponse variable...if we call the perform/cancel methods, it also calls the other delegate methods. But that's probably what it's supposed to do. Maybe you just need to add a if (![self sentResponse]) { condition to didFailWithError, like I did to connectionDidFinishLoading. Since we're already sending the response in willSendRequestForAuthenticationChallenge, we don't want to send it twice.

I'm only bringing it up because I think that timeout thing may have been a real consequence of not calling the expected methods. I could be wrong -- if I have time one of these days, I'll try to re-test.