Allow to disable pinning for subdomains

[omerlh]

Hey, I want to enable pinning for all our domain, but disable it for a specific subdomain. So I tried it with something like that:

<network-security-config>
  <!-- Pin the domain www.datatheorem.com -->
  <!-- Official Android N API -->
  <domain-config>
    <domain includeSubdomains="true">www.mydomain.com</domain>
    <pin-set>
      <pin digest="SHA-256">....</pin> <!-- production -->
      <pin digest="SHA-256">...</pin> <!-- backup -->
    </pin-set>
    <!-- TrustKit Android API -->
    <!-- Do not enforce pinning validation -->
    <trustkit-config enforcePinning="true" disableDefaultReportUri="true">
      <!-- Add a reporting URL for pin validation reports-->
      <report-uri>https://report-uri</report-uri>
    </trustkit-config>
    <domain-config>
      <domain>unsecure.mydomain.com</domain>
      <pin-set></pin-set>
      <trustkit-config enforcePinning="false" disableDefaultReportUri="true">
        <!-- Add a reporting URL for pin validation reports-->
        <report-uri>https://report-uri</report-uri>
      </trustkit-config>
    </domain-config>
  </domain-config>
</network-security-config>

Notice the empty pin set in the child domain configuration - this is required, otherwise Android will use the pin from the parent domain cofing. The problem is - TrustKit not allow declaring empty pinset. Maybe this is something possible to change? I have similar issue with ios - datatheorem/TrustKit#88 - and I suggested similar change there too. Thanks, Omer

[nabla-c0d3]

Hey, Can you try this config on Android N without initializing TrustKit (you can comment the line out) ? I'd like to know what is Android N's behavior in this case. TrustKit should 100% mirror it. Thanks!

[omerlh]

@nabla-c0d3, I checked that by debugging Android N TrustManager - it seems that if the pin list is empty, it does not do certificate pinning (from NetworkSecurityTrustManager):

PinSet pinSet = mNetworkSecurityConfig.getPins();
        if (pinSet.pins.isEmpty()
                || System.currentTimeMillis() > pinSet.expirationTime
                || !isPinningEnforced(chain)) {
            return;
        }

private boolean isPinningEnforced(List<X509Certificate> chain) throws CertificateException {
        if (chain.isEmpty()) {
            return false;
        }
        X509Certificate anchorCert = chain.get(chain.size() - 1);
        TrustAnchor chainAnchor =
                mNetworkSecurityConfig.findTrustAnchorBySubjectAndPublicKey(anchorCert);
        if (chainAnchor == null) {
            throw new CertificateException("Trusted chain does not end in a TrustAnchor");
        }
        return !chainAnchor.overridesPins;
    }

[nabla-c0d3]

Thanks for checking - then this is definitely a bug in TrustKit. Will fix it for the next release.

[omerlh]

Ok, thank you! If that help, I can create a PR.

[nabla-c0d3]

Sure, that would definitely help! Two things I would ask tho:

• Please make sure that an empty < pin-set > will be accepted as a valid policy by Android N (as in your previous reply you said "it seems that if"). For a quick check, you can use the TrustKit Demo App, remove the line of code that initializes TrustKit, add an empty and make sure that it is accepted.
• In your implementation, we will need an extra check which is that if for the domain's configuration enforcePinning is set to true and the < pin-set > is empty, TrustKit should crash on initialization (because this config wouldn't make sense).

So yeah feel free to create a PR, or otherwise we will eventually fix this for the next release. Thanks!

[omerlh]

I opened the issue only after we had issue (e.g. subdomain that should be excluded does not exclude) on Android 7. I just looked in the code to make sure I am not missing something.