approov / react-native-cert-pinner

Strengthens TLS in React Native through Certificate Pinning
https://blog.approov.io/tag/reactnative
Apache License 2.0
97 stars 41 forks source link

Not working with proguard #9

Open frncs-eu opened 5 years ago

frncs-eu commented 5 years ago

Hi, first of all kudos for the great work. I'd like to discuss about an issue I'm having with this package. In order to make it work with proguard I had to exclude the package from obfuscation with:

-keep class com.criticalblue.reactnative.** {
*;
}

Since the package is using reflection to access the certPinner:

try {
            Class noparams[] = {};
            Class clazz = Class.forName("com.criticalblue.reactnative.GeneratedCertificatePinner");
            Method method = clazz.getDeclaredMethod("instance", noparams);
            certificatePinner = (CertificatePinner) method.invoke(null);
            Log.i(TAG, "Generated Certficate Pinner in use");
        } catch(Exception e){
            Log.e(TAG, "No Generated Certficate Pinner found - likely a pinset configuration error");
            Log.w(TAG, "CERTIFICATE PINNING NOT BEING USED");
        }

But excluding this package from obfuscation makes it extremely trivial for an attacker to decompile and bypass the pinning feature. Wouldn't it be better to manually import the com.criticalblue.reactnative.GeneratedCertificatePinner package inside of CertPinnerPackage.java and invoke the static instance method instead of relying on reflection? This would improve Proguard obfuscation without breaking the functionality. Thank you,

Francesco

Pankaj-Hss commented 5 years ago

@kde3kko you have raised a good point. But if you go through the code at CertPinnerPackage.java line no 24 developer already commented: "create custom certificate pinner.needs to use reflection so that class can be generated outside the package library". This means the developer is already aware of the reflection at this point. So @kde3kko, if we implement your suggested solution so it can impact the functionality.