From 66873783ec05f82fd853a2ca57dc4e772a91001c Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Wed, 18 Oct 2023 19:13:15 +0200 Subject: [PATCH] Remove unnecessary lax Conscrypt hooks This is covered more effectively by our injection of our system certificate into the default trust store for all Conscrypt implementations via the index. --- android-certificate-unpinning.js | 26 -------------- android-system-certificate-injection.js | 45 +++++++++++++++++-------- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/android-certificate-unpinning.js b/android-certificate-unpinning.js index 2905537..e1aef04 100644 --- a/android-certificate-unpinning.js +++ b/android-certificate-unpinning.js @@ -66,23 +66,6 @@ const PINNING_FIXES = { } ], - // --- Native Conscrypt OpenSSLSocketImpl - - 'com.android.org.conscrypt.OpenSSLSocketImpl': [ - { - methodName: 'verifyCertificateChain', - replacement: () => NO_OP - } - ], - - 'com.android.org.conscrypt.OpenSSLEngineSocketImpl': [ - { - methodName: 'verifyCertificateChain', - overload: ['[Ljava.lang.Long;', 'java.lang.String'], - replacement: () => NO_OP - } - ], - // --- Native Conscrypt CertPinManager 'com.android.org.conscrypt.CertPinManager': [ @@ -233,15 +216,6 @@ const PINNING_FIXES = { } ], - // --- Apache Harmony version of OpenSSLSocketImpl (v similar to Conscrypt above) - - 'org.apache.harmony.xnet.provider.jsse.OpenSSLSocketImpl': [ - { - methodName: 'verifyCertificateChain', - replacement: () => NO_OP - } - ], - // --- PhoneGap sslCertificateChecker (https://github.com/EddyVerbruggen/SSLCertificateChecker-PhoneGap-Plugin) 'nl.xservices.plugins.sslCertificateChecker': [ diff --git a/android-system-certificate-injection.js b/android-system-certificate-injection.js index c372416..c86801f 100644 --- a/android-system-certificate-injection.js +++ b/android-system-certificate-injection.js @@ -27,22 +27,39 @@ Java.perform(() => { // by prepopulating all instances, we ensure that all TrustManagerImpls (and potentially other // things) automatically trust our certificate specifically (without disabling validation entirely). // This should apply to Android v7+ - previous versions used SSLContext & X509TrustManager. - const TrustedCertificateIndex = Java.use('com.android.org.conscrypt.TrustedCertificateIndex'); - TrustedCertificateIndex.$init.overloads.forEach((overload) => { - overload.implementation = function () { - this.$init(...arguments); - // Index our cert as already trusted, right from the start: - this.index(cert); + [ + 'com.android.org.conscrypt.TrustedCertificateIndex', + 'org.conscrypt.TrustedCertificateIndex', // Might be used (com.android is synthetic) - unclear + 'org.apache.harmony.xnet.provider.jsse.TrustedCertificateIndex' // Used in Apache Harmony version of Conscrypt + ].forEach((TrustedCertificateIndexClassname, i) => { + let TrustedCertificateIndex; + try { + TrustedCertificateIndex = Java.use(TrustedCertificateIndexClassname); + } catch (e) { + if (i === 0) { + throw new Error(`${TrustedCertificateIndexClassname} not found - could not inject system certificate`); + } else { + // Other classnames are optional fallbacks + return; + } } - }); - TrustedCertificateIndex.reset.overloads.forEach((overload) => { - overload.implementation = function () { - const result = this.reset(...arguments); - // Index our cert in here again, since the reset removes it: - this.index(cert); - return result; - }; + TrustedCertificateIndex.$init.overloads.forEach((overload) => { + overload.implementation = function () { + this.$init(...arguments); + // Index our cert as already trusted, right from the start: + this.index(cert); + } + }); + + TrustedCertificateIndex.reset.overloads.forEach((overload) => { + overload.implementation = function () { + const result = this.reset(...arguments); + // Index our cert in here again, since the reset removes it: + this.index(cert); + return result; + }; + }); }); // This effectively adds us to the system certs, and also defeats quite a bit of basic certificate