Modify sslpinning bypass for okhttp3 library

[leonjza]

Thanks for the PR. In thinking about it, I think we need to explicitly also hook certificatePinner.check$okhttp in addition to certificatePinner.check$okhttp.overload("java.lang.String", "u15"). Could you update the code with that?

[kiven7299]

Yes I could do that. However, I wonder whether we must use overload or not, since there's only 1 check$okhttp method in Okhttp3.CertificatePinner.

• From Okhttp3 source code, link: https://github1s.com/square/okhttp/blob/HEAD/okhttp/src/jvmMain/kotlin/okhttp3/CertificatePinner.kt
• From result of android hooking list class_methods okhttp3.CertificatePin:

Besides, the overload("java.lang.String", "u15") seems not match with arguments of check$okhttp method (internal fun check(hostname: String, cleanedPeerCertificatesFn: () -> List<X509Certificate>)), refers to the initial issue.

[leonjza]

Right, but in older versions of okhttp the function signatures are different.

[kiven7299]

I've added hook to certificatePinner.check$okhttp.overload("java.lang.String", "u15"). Please review the new commit.

[CDuPlooy]

I only had a quick look at this, but since the hook is just logging (and thus not throwing the exception); It should be easyish to use the information that the Java bridge already provides to hook all overloads, and then make them all do the same thing. I can take a stab at this sometime next week @kiven7299 if you don't want to.

I'm on a quick lunch break, but I think something like the below should work.

const overloads = Java.use("target_class")["target_method"].overloads
ovearloads.forEach(overload => {
     overload.implementation = function(arguments) {
          // do some logging or whatever
     }
})

This is not the only hook where something like this would be useful.

[kiven7299]

Hi @CDuPlooy, Thanks for your advice, I've changed the code based on that

const okHttp3CertificatePinnerCheckOkHttp = (ident: string): any | undefined => {
  return wrapJavaPerform(() => {
    try {
      const certificatePinner = Java.use("okhttp3.CertificatePinner");
      certificatePinner["check$okhttp"].overloads.forEach((m) => {
          // get the argument types for this overload
          var calleeArgTypes = m.argumentTypes.map((arg) => arg.className);
          send(color_1.colors.blackBright(`Found okhttp3.CertificatePinner.check$okhttp(${calleeArgTypes.join(", ")}), overriding ...`));

          m.implementation = function () {
              helpers_1.qsend(quiet, color_1.colors.blackBright(`[${ident}] `) + `Called check$okhttp ` +
                  color_1.colors.green(`OkHTTP 3.x CertificatePinner.check$okhttp()`) +
                  `, not throwing an exception.`);
          }
      });
      return CertificatePinnerCheckOkHttp;

    } catch (err) {
      if ((err as Error).message.indexOf("ClassNotFoundException") === 0) {
        throw err;
      }
    }
  });
};

Run code:

[kiven7299]

Hi @CDuPlooy, I know the problems in my latest committed code; I type helpers_1.qsend instead of qsend, and color_1.colors.blackBright instead of c.blackBright. Your changes are great and it might be useful in some cases. However, normally when we kill bypass sslpinning job, it is expected to kill all the hooks to all libraries. In my opinion, there is barely a need to kill just one override of CertificatePinner.check$okhttp method.

[kiven7299]

I think I've made much more error in the latest commit. My git skill is suck.

[CDuPlooy]

I think there may be some confusion here. In the example I show, okHttp3CertificatePinnerCheckOkHttp returns an array of Java methods. The job kill routine takes the job object, and then performs a series of actions on each Interceptor, implementation and whatever else. Where previously this hook only operated on one overload, it was okay to return a single element - but now that it operates on multiple elements, it needs to return all of those elements.

With the previous change, if you only pushed one of several overloads and run the kill routine, only that overload would be stopped and the remaining hooks would still be in place with no way to kill them.