OpenSSLX509CRLEntry.hasUnsupportedCriticalExtension shouldn't call X509_supported_extensions

[davidben]

Noticed this idly... hasUnsupportedCriticalExtension on OpenSSLX509CRLEntry is implemented with X509_supported_extensions, but X509_supported_extensions only lists extensions that we implement in a certificate context.

For X509_REVOKED, BoringSSL just doesn't implement any critical extensions, so the replacement should be pretty straightforward. Although this does beg the question whose implementation these functions are meant to query, since Conscrypt isn't even using BoringSSL's verifier... :-)

[prbprbprb]

For X509_REVOKED, BoringSSL just doesn't implement any critical extensions, so the replacement should be pretty straightforward

\o/

Although this does beg the question whose implementation these functions are meant to query

Yeah, which applies to certificates too. For most purposes if you process an OpenSSLX509Certificate, you're going to get BoringSSL's notion of which extensions are supported. But if you pass it to any of the CertPath APIs then it's going to get re-parsed and you'll get the Sun notion of which extensions are supported. Which is...... Not ideal.

For the Android platform, I think it's ok because (a) we don't (intentionally) use CRLs for TLS trust decisions and (b) we're fairly confident the CertPath APIs work correctly for our purposes.

The same holds for apps using the CertPath APIs for their own purposes.

Apps building their own verifiers (eek!) won't have access to the internal Sun classes, so they'll be using the OpenSSLX509Certificate methods.

So it's mostly self consistent as long as apps don't mix and match own code with CertPath code.

Although one wrinkle I did spot while down this rabbit hole - OpenJDK 8 (and so current Android) CertPathValidator uses the algorithm from RFC 3280, but OpenJDK 21 uses RFC 5280. The differences look relatively minor (to me) but I wonder if this will cause app compat issues when Android updates to OpenJDK 21 code.

[davidben]

Oh I was less worried about the verifier logic is self-consistent (I certainly hope the verifier uses its own notion of critical!), and more what OpenSSLX509Certificate's API is even expected to return in the first place.

If you all don't ever call the BoringSSL verifier, what does it mean for OpenSSLX509Certificate to report on that verifier's capabilities? I.e. suppose we had a bug where we thought zero critical extensions were supported. Is that any less correct than the current behavior? 🙃