Code review request: 7113275, compatibility issue with MD2 trust anchor and old X509TrustManager
Sean Mullan
sean.mullan at oracle.com
Mon Nov 21 21:59:33 UTC 2011
On 11/20/2011 10:06 PM, Xuelei Fan wrote:
> webrev: http://cr.openjdk.java.net/~xuelei/7113275/webrev.00/
> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7113275
>
> Test MD2InTrustAnchor.java is used to test that MD2 in trust anchor is
> able to work with the default trust manager (X509ExtendedTrustManager).
>
> Test TrustTrustedCert.java is used to test that MD2 in trust anchor is
> able to work with the un-extended trust manager (X509TrustManager).
>
> Some customized trust manages developed in JDK 6 did not know the
> features in JDK 7, and may not check algorithm constraints. I think we
> need the addition algorithm constraint check for un-extended trust
> manager in order to ensure that the TM comply to security constraints
> defined by security property, jdk.certpath.disabledAlgorithms.
>
> The algorithm check of certification chain is light weight, so even the
> customized trust manager has already managed to check the algorithm
> constraints during certification path validation, the performance hurt
> is very limited.
I believe you could add an optimization that if the TrustManager is an
instance of sun.security.ssl.X509TrustManagerImpl, then there is no need
to perform the additional certpath constraint checks because the PKIX
CertPathValidator will have already checked them.
I think the checkAlgorithmConstraints method can be marked static.
A nit on line 922, change to:
checkedLength--;
Cycling through every root certificate each time to find a match in the
chain could be costly. Another potential optimization is to copy the
root certs into a Map, where the key is simply the Certificate. The
value can be null. Then you can just check if the last certificate is in
the Map.
But even better would be to add a method to TrustManager (or something
like that) that returned the trusted Certificate that was used to anchor
the chain that was just validated. This would be the most optimal
solution. That might be a bit challenging given the API, but something
to keep in mind if you enhance the API later on.
--Sean
More information about the security-dev
mailing list