alexrainman / ModernHttpClient

ModernHttpClient
MIT License
126 stars 28 forks source link

Certificate pinning failure issues #20

Closed patchandthat closed 5 years ago

patchandthat commented 5 years ago

Hi, I'm using this library and I have an issue with respect to certificate pinning in my app.

I was wanting to fix my issue (and send a pull request if you want to take the changes) but it looks like as of 2.7.0 you've deleted a lot of the projects - I'm unsure how I'm supposed to build and run this code now.

Is this project still actively maintained?

alexrainman commented 5 years ago

@patchandthat the answer is yes, this repo is maintained. I migrated the project to a multi-target style when I took over.

It runs like any other Visual Studio project. At build time it compiles for the platform you are targeting.

What kind of fix you did for certificate pinning?

I am planing a release that will include and update to the way certificate pinning is done. That and some other minor things.

patchandthat commented 5 years ago

Ok. I saw there was only one project file with only the shared code and was confused, I expected to see separate Droid/iOS/etc projects as well. My mistake, sorry.

So for cert pinning, I'm using the ServicePointManager callback. On iOS when the cert changes and the callback returns false, I see an OperationCancelledException, With an inner NSErrorException, like below.

System.OperationCanceledException: cancelled ---> Foundation.NSErrorException: Error Domain=NSURLErrorDomain Code=-999 "cancelled" UserInfo={NSErrorFailingURLStringKey=https://xxxxxxxxxxxxx/xxxxxxxx/xx, NSErrorFailingURLKey=https://xxxxxxxxxxxxx/xxxxxxxx/xx, _NSURLErrorRelatedURLSessionTaskErrorKey=( "LocalDataTask <7289AE5D-604B-4B76-B6E3-651665D6C8EE>.<22>" ), _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <7289AE5D-604B-4B76-B6E3-651665D6C8EE>.<22>, NSLocalizedDescription=cancelled} --- End of inner exception stack trace --- at ModernHttpClient.NativeMessageHandler+d24.MoveNext () [0x00421] in :0 --- End of stack trace from previous location where exception was thrown --- at System.Net.Http.HttpClient+d48.MoveNext () [0x00080] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:276

The problem being that this looks quite ambiguous. I need to take action in the event of a certificate trust failure, so I need to be able to see the reason for cancellation in there somewhere.

For comparison, on Android, if you see an untrusted cert for example, you see an exception such as the one below, which makes it clear why the request failed.

System.Net.Http.HttpRequestException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found. ---> Javax.Net.Ssl.SSLHandshakeException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found. ---> Java.Security.Cert.CertificateException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found. ---> Java.Security.Cert.CertPathValidatorException: Trust anchor for certification path not found.

alexrainman commented 5 years ago

Using the ServicePointManager callback is not adviced for performance issues.

Instead, i am going to implement one-way and two-way certificate pinning in the next release.

patchandthat commented 5 years ago

Can you elaborate on why you would avoid using ServicePointManager?

In the context of my app, I need to be able to dynamically change the validation function while the app is running. The app goes through some trusted setup to capture a certificate to pin against, rather than have a static certificate compiled into the app. Hopefully the changes you have planned will work in this case as well.

Do you have a rough idea of when the next release will be?