alexrainman / ModernHttpClient

ModernHttpClient
MIT License
126 stars 27 forks source link

VerifyPins() fails for SHA1 and MD5 pins #58

Closed tele-bird closed 4 years ago

tele-bird commented 4 years ago

When I pass in a valid sha1 or md5 base64 string, I get the FormatException (The input is not a valid Base-64 string.) . After analyzing the problem, I found a bug in Utility.cs:

Currently, line 85 in VerifyPins() method assumes the prefix is 7 characters in length:

byte[] bytes = Convert.FromBase64String(pin.Remove(0, 7));

This is completely legit for "sha256/", but not "sha1/" or "md5/".

To fix this bug, you could change it like this:

byte[] bytes = Convert.FromBase64String(pin.Remove(0, pin.IndexOf('/') + 1));

tele-bird commented 4 years ago

I made this change I described and tested in my iOS/Android projects.

The iOS app works nicely.

However, the Android app throws the following Exception. It appears that the Square.OkHttp3.CertificatePinner.Builder() method won't accept adding pins that start with "md5/". It's strange because the iOS implementation does accept pins that start with "md5/". Probably this is a bug in that package.

FullName: Java.Lang.IllegalArgumentException Message: pins must start with 'sha256/' or 'sha1/': md5/mymd5fingerprint StackTrace: StackTrace: at Java.Interop.JniEnvironment+InstanceMethods.CallObjectMethod (Java.Interop.JniObjectReference instance, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue args) [0x00069] in :0 at Android.Runtime.JNIEnv.CallObjectMethod (System.IntPtr jobject, System.IntPtr jmethod, Android.Runtime.JValue parms) [0x0000e] in <266395aeef1642bab2007ca31686faf9>:0 at Square.OkHttp3.CertificatePinner+Builder.Add (System.String pattern, System.String[] pins) [0x00069] in :0 at ModernHttpClient.CertificatePinner.AddPins (System.String hostname, System.String[] pins) [0x00016] in /Users/phadley/Projects/ModernHttpClient/ModernHttpClient.Android/CertificatePinner.cs:41 at ModernHttpClient.NativeMessageHandler..ctor (System.Boolean throwOnCaptiveNetwork, ModernHttpClient.TLSConfig tLSConfig, ModernHttpClient.NativeCookieHandler cookieHandler, System.Net.IWebProxy proxy) [0x00194] in /Users/phadley/Projects/ModernHttpClient/ModernHttpClient.Android/OkHttpNetworkHandler.cs:83 at IQ.Droid.Services.Implementations.HttpMessageHandlerProvider.GetMessageHandler (System.String host, System.String sha1HashBase64String, System.String md5HashBase64String) [0x00001] in /Users/phadley/Projects/K-Connect/IQ.Droid/Services/Implementations/HttpMessageHandlerProvider.cs:13 at IQ.Core.Services.DuoServices.DuoCommunicator.Initialize () [0x00001] in /Users/phadley/Projects/K-Connect/IQ.Core/Services/DuoServices/DuoCommunicator.cs:43 at IQ.Core.ViewModels.Account.GateScreenViewModel.ProvisionKDuoAsync (System.String ssid, System.String password) [0x0005b] in /Users/phadley/Projects/K-Connect/IQ.Core/ViewModels/Account/GateScreenViewModel.cs:233 at IQ.Core.ViewModels.Account.GateScreenViewModel.ConnectToKDuoAccessPointAsync () [0x00127] in /Users/phadley/Projects/K-Connect/IQ.Core/ViewModels/Account/GateScreenViewModel.cs:211 --- End of managed Java.Lang.IllegalArgumentException stack trace --- java.lang.IllegalArgumentException: pins must start with 'sha256/' or 'sha1/': md5/mymd5fingerprint at okhttp3.CertificatePinner$Pin.(CertificatePinner.java:279) at okhttp3.CertificatePinner$Builder.add(CertificatePinner.java:332) at android.support.v7.app.AlertDialog_IDialogInterfaceOnClickListenerImplementor.n_onClick(Native Method) at android.support.v7.app.AlertDialog_IDialogInterfaceOnClickListenerImplementor.onClick(AlertDialog_IDialogInterfaceOnClickListenerImplementor.java:30) at android.support.v7.app.AlertController$ButtonHandler.handleMessage(AlertController.java:167) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loop(Looper.java:193) at android.app.ActivityThread.main(ActivityThread.java:6718) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

alexrainman commented 4 years ago

Sadly, OkHttp doesn't support MD5 so, i will remove it from the plugin next release.

tele-bird commented 4 years ago

Thanks. And probably the bug I noted in VerifyPins() should be fixed as well.