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

javax.net.ssl.SSLPeerUnverifiedException: Hostname not verified #26

Closed
niklashigi opened this issue Dec 2, 2020 · 14 comments
Closed

javax.net.ssl.SSLPeerUnverifiedException: Hostname not verified #26

niklashigi opened this issue Dec 2, 2020 · 14 comments
Labels
bug Something isn't working certificate pinning Issues related to certificate pinning

Comments

@niklashigi
Copy link
Owner

  1. Patching succeeded, showing warning about Android App Bundle
    2.I exported via SAI, patched resulting .apks, installed resulting .apks, and it app itself works, however MITM does not, seemingly there is some pinning that was missed by the apk-mitm (I noticed that briefly it has shown "no pinning detected").

How can I export log/debug that? So far I identified in logcat:

 Root cause (1 of 1)
 javax.net.ssl.SSLPeerUnverifiedException: Hostname foo.com not verified:
     certificate: sha256/[...]
     DN: CN=foo.com,OU=UNTRUSTED SandroProxy,O=UNTRUSTED SandroProxy
     subjectAltNames: []
 	at okhttp3.internal.connection.RealConnection.b(SourceFile:22)
 	at okhttp3.internal.connection.RealConnection.f(SourceFile:9)
 	at okhttp3.internal.connection.RealConnection.connect(SourceFile:15)
 	at okhttp3.internal.connection.ExchangeFinder.c(SourceFile:32)
 	at okhttp3.internal.connection.ExchangeFinder.d(SourceFile:1)
 	at okhttp3.internal.connection.ExchangeFinder.b(SourceFile:6)
 	at okhttp3.internal.connection.Transmitter.e(SourceFile:5)
 	at okhttp3.internal.connection.ConnectInterceptor.intercept(SourceFile:5)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:1)
 	at okhttp3.internal.cache.CacheInterceptor.intercept(SourceFile:22)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:1)
 	at okhttp3.internal.http.BridgeInterceptor.intercept(SourceFile:22)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(SourceFile:6)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:1)
 	at com.myapp.base.network.interceptor.ImageProfilingNetworkInterceptor.intercept(SourceFile:5)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:1)
 	at com.myapp.base.network.interceptor.ImageCacheInterceptor.intercept(SourceFile:3)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:10)
 	at okhttp3.internal.http.RealInterceptorChain.proceed(SourceFile:1)
 	at okhttp3.RealCall.e(SourceFile:13)
 	at okhttp3.RealCall$AsyncCall.execute(SourceFile:2)
 	at okhttp3.internal.NamedRunnable.run(SourceFile:3)
 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
 	at java.lang.Thread.run(Thread.java:764)

Originally posted by @anilatx in #23 (comment)

@niklashigi
Copy link
Owner Author

niklashigi commented Dec 2, 2020

@anilatx

seemingly there is some pinning that was missed by the apk-mitm (I noticed that briefly it has shown "no pinning detected")

That could very well be the case. apk-mitm currently only patches pinning logic that's using the javax.net.ssl.X509TrustManager interface by searching for the three relevant methods in all *.smali files.

javax.net.ssl.SSLPeerUnverifiedException: Hostname foo.com not verified:

This exception also comes from the javax.net.ssl package, but there's no mention of the X509TrustManager interface anywhere, so that doesn't seem to be involved here at all.

at okhttp3.internal.connection.RealConnection.b(SourceFile:22)

The app is using the OkHttp library, which has its own way of setting up certificate pinning. I found this guide on circumventing it, so maybe you could give that a try. The --wait flag could come in helpful for that.

@niklashigi niklashigi added bug Something isn't working certificate pinning Issues related to certificate pinning labels Dec 2, 2020
@anilatx
Copy link

anilatx commented Dec 2, 2020

Thanks! I started with simple ignoring exception in okhttp3.Handshake class, changing
.catch Ljavax/net/ssl/SSLPeerUnverifiedException; {:try_start_0 .. :try_end_0} ::catch_0
into
.catch Ljavax/net/ssl/SSLPeerUnverifiedException; {:try_start_0 .. :try_end_0} :goto_0
But I get
zygote : Rejecting re-init on previously-failed class java.lang.Class<okhttp3.Handshake>: java.lang.VerifyError: Verifier rejected class okhttp3.Handshake: okhttp3.Handshake okhttp3.Handshake.get(javax.net.ssl.SSLSession) failed to verify: okhttp3.Handshake okhttp3.Handshake.get(javax.net.ssl.SSLSession): [0x2A] type Conflict unexpected as arg to if-eqz/if-nez (declaration of 'okhttp3.Handshake' appears in /data/app/com.myapp-fxq0DA7v3ZpYEHcw8McmXg==/base.apk:classes27.dex)
This is the only Handshake class in the sourcecode. Am I doing something wrong? Or could they have some tough protection that stores duplicates of key classes in binary data or downloads them from a server?

@niklashigi
Copy link
Owner Author

Verifier rejected class

This probably means that your changes to the Smali code were invalid (see relevant SO answer). Smali is a representation of Java bytecode, so it can be difficult to make changes without breaking things. (That's why apk-mitm is replacing entire methods instead of making more "precise" changes.)

Could you create a secret Gist with the relevant Smali files (Handshake and CertificatePinner if it exists) and post it here?

@anilatx
Copy link

anilatx commented Dec 2, 2020

@niklashigi
Copy link
Owner Author

The check method on the CertificatePinner class doesn't return anything when the verification is successful, so maybe you could try replacing the entire method with this (the equivalent of return; in Java):

.method public check(Ljava/lang/String;Ljava/util/List;)V
    .locals 0
    return-void
.end method

@anilatx
Copy link

anilatx commented Dec 2, 2020

Apparently this is not the only place, https://github.com/1184893257/okhttp/blob/master/okhttp/src/main/java/com/squareup/okhttp/internal/io/RealConnection.java can also throw this SSLPeerUnverifiedException (and it does, when I fixed CertificatePinner). I managed to get the MITM to work replacing two verify() in connectTls: https://gist.github.com/anilatx/23ac1c41a2ad8301087d78ce51ae00b9
I suspect that to prevent random disabled fuctionalities, it might be necessary to disarm similarly other instances of javax.net.ssl.SSLPeerUnverifiedException, there are plenty https://gist.github.com/anilatx/9ab259f89b7d12a555319f69dbc59380

@niklashigi
Copy link
Owner Author

I managed to get the MITM to work replacing two verify() in connectTls

That's great news! OkHttp is pretty popular, so if we manage to find a way to apply these changes automatically we'd be able to make many more apps patchable using apk-mitm.

I suspect that to prevent random disabled fuctionalities, it might be necessary to disarm similarly other instances of javax.net.ssl.SSLPeerUnverifiedException

Good point. It would be best if we could remove all "throws" of that exception using a single rule regardless of the library, but I doubt there's a way to do that reliably. Putting together a list of function signatures for OkHttp (similar to what I've already put together for X509TrustManager) would probably be good enough though.

To do that I would need an APK to investigate. I see you've censored the name of the app you're trying to patch in your comment, but could you privately share that APK (or a link to it on a site like APKPure) with me via Telegram or email (see my GitHub profile)? If that's not possible, could you find another APK that also uses OkHttp and has similar protections enabled?

@niklashigi
Copy link
Owner Author

I've received your email, so I can now take a look at all the Smali sources.

I managed to get the MITM to work replacing two verify() in connectTls

Can you clarify what you mean by that? What did you replace these calls with? Also, did you make these changes in the Smali code or in Java (using something like jadx or smali2java)?

@anilatx
Copy link

anilatx commented Dec 7, 2020

The email included modified two already modified smali (I can upload them/diff somewhere public in the evening), I basically replaced calls to verify with setting unused variable to 0, directly in smali (I read Java code only as roadmap )

@niklashigi
Copy link
Owner Author

The email included modified two already modified smali

Whoops, totally missed the attachments. 😅 I'll take a look at your changes!

@niklashigi
Copy link
Owner Author

I've taken a look at your changes and I even found a slightly more robust way to disable host name verification (which involves patching the verify method on implementations of the javax.net.ssl.HostnameVerifier interface). The question is though: Is this something apk-mitm should even do? I was testing the APK using the Charles and even without the host name verification changes I was able to look at all the traffic.

I suspect that SandroProxy, which you seem to be using, might not correctly generate its certificates to include the "Common Name" and "Alternative Names" the verification logic is looking for. You could see if this is the case by opening a website in Chrome on Android (Firefox would probably work too) and viewing the certificate information before and after you've enabled the proxy. The domains in the two fields I mentioned should be the same in both cases.

@niklashigi
Copy link
Owner Author

@anilatx Any updates on this? I'd be happy to implement a fix to disable host name verification, but I want to make sure I actually understand the issue and its cause first.

@niklashigi
Copy link
Owner Author

This was fixed in 0f85c10.

@anilatx
Copy link

anilatx commented Jan 31, 2021

@shroudedcode I can confirm that v0.11.1 works out-of-the-box for same app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working certificate pinning Issues related to certificate pinning
Projects
None yet
Development

No branches or pull requests

2 participants