moxie0 / AndroidPinning

A standalone library project for certificate pinning on Android.
http://www.thoughtcrime.org/blog/authenticity-is-broken-in-ssl-but-your-app-ha/
GNU General Public License v3.0
618 stars 114 forks source link

Check for trustedChain in CertificateChainCleaner #18

Open kbsriram opened 10 years ago

kbsriram commented 10 years ago

One of the goals of the certificate chain cleaner is to ensure there's at least one trust anchor in the cleaned chain, so I'm wondering at the order of checks at

https://github.com/moxie0/AndroidPinning/blob/9098125806174444937315a421f3fec63e702aad/src/org/thoughtcrime/ssl/pinning/CertificateChainCleaner.java#L50

Would it be better to put the isTrustRoot(chain[i]) check inside the if (isValidLink(chain[i], chain[i - 1])) block, rather than before it? Otherwise, it seems like trustedChain could be set to true without a verified trust anchor in the chain.

Apologies if this is a non-issue, because I have a feeling there's probably something I've overlooked :smile: and thanks for the library!