Hi Mike,


thanks for your comments on this.



I agree that the change is quite critical and my approach might not be good. 
However, as for your suggestion to specify the sun provider for the certificate 
factory, I'd say this is not quite right, too. Because, if you have loaded a 
different provider on top of the providers list, you'll intend to use its 
facilities by default. Maybe the idea would be to use the sun provider as 
fallback in case of an exception?



The other suggestion to check for certChain.length in KeyStore.getCertificate 
and return null would be an improvement compared to throwing an 
ArrayIndexOutOfBoundsException - however, it would still be difficult to find 
the root cause why a certificate could not be loaded.



What do you think?



@all: Are there other opinions?



Thanks and best regards

Christoph



From: security-dev [mailto:security-dev-boun...@openjdk.java.net] On Behalf Of 
Mike StJohns
Sent: Mittwoch, 14. Oktober 2015 02:17
To: security-dev@openjdk.java.net
Subject: Re: RFR 8139436: sun.security.mscapi.KeyStore might load incomplete 
data



Hi -

I took a look and this probably isn't the correct way to fix this.

A simpler change might be to specify the sun provider when requesting the 
certificate factory.    I hesitate to say that definitively as modularization 
guidance may restrict that approach?

The belt and suspenders approach is to catch the bad certificate exception and 
return null.  That appears to be the correct contract for 
KeyStore.getCertificate(String alias).   (e.g. "if (certChain.length == 0) 
return null;")

Mike


On 10/12/2015 5:04 PM, Langer, Christoph wrote:

   Hi,



   please review a change proposal regarding an issue in the Microsoft Security 
API (mscapi).



   Bug: https://bugs.openjdk.java.net/browse/JDK-8139436

   Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8139436.0/



   I stumbled over the issue when using an old IAIK security provider. It would 
throw java.security.cert.CertificateException upon parsing ECC based 
certificates when generating Certificate objects. The way it is right now, such 
exceptions are silently caught and the Windows Keystore is created with 
incomplete data. Upon accessing such ECC certificates from the Keystore object, 
e.g. when iterating over it, you'll get exceptions like:

   Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 0

           at 
sun.security.mscapi.KeyStore.engineGetCertificate(KeyStore.java:313)

           at 
sun.security.mscapi.KeyStore$ROOT.engineGetCertificate(KeyStore.java:60)

           at java.security.KeyStore.getCertificate(KeyStore.java:1095)

   At that point it is not obvious what the real root cause for that is.



   With my change, loading of the keystore would already throw like this:

   java.io.IOException: java.security.KeyStoreException: Exception occurred 
generating certificate object for alias DigiCert Assured ID Root G3

           at sun.security.mscapi.KeyStore.engineLoad(KeyStore.java:780)

           at sun.security.mscapi.KeyStore$ROOT.engineLoad(KeyStore.java:60)

           at java.security.KeyStore.load(KeyStore.java:1459)

           at 
WindowsCertificateReaderTest.main(WindowsCertificateReaderTest.java:18)

   Caused by: java.security.KeyStoreException: Exception occurred generating 
certificate object for alias DigiCert Assured ID Root G3

           at sun.security.mscapi.KeyStore.loadKeysOrCertificateChains(Native 
Method)

           at sun.security.mscapi.KeyStore.engineLoad(KeyStore.java:777)

           ... 3 more

   Caused by: java.security.cert.CertificateException: Error parsing 
certificates! iaik.asn1.DerInputException: Next ASN.1 object is no OBJECT 
IDENTIFIER!

           at iaik.x509.CertificateFactory.engineGenerateCertificates(Unknown 
Source)

           at 
java.security.cert.CertificateFactory.generateCertificates(CertificateFactory.java:462)

           at 
sun.security.mscapi.KeyStore.generateCertificate(KeyStore.java:869)

           ... 5 more

   This is more obvious when it comes to analyzing such an issue.



   Also, I added a property "sun.security.mscapi.ignoreFailingCertificates" 
which, when set to true, will cause skipping of certificates that failed with 
Exception. That might be a nice workaround option if one is not particularly 
interested in a failing certificate.



   You can reproduce all this with the test coding in the OpenJDK Bug, the IAIK 
provider 3.15 which is downloadable here: 
http://jcewww.iaik.tu-graz.ac.at/sic/Download (educational/research version, 
needs registration) and ECC certificates in the Windows Root certificate store.



   Would you think this change is reasonable and worthwile?



   Thanks & Best regards

   Christoph



Reply via email to