Sorry for the top post, I'm wring this on an iPad on a plane.
It's perfectly acceptable for a provider to prefer its version of stuff over even something early in the provider list. Usually, that has more to do with key material ( for example pkcs11 instantiations), but there's no reason why it shouldn't apply to certificates. Mike Sent from my iPad > On Oct 14, 2015, at 18:35, Langer, Christoph <christoph.lan...@sap.com> wrote: > > 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 >
