On 11/10/2015 4:12 PM, Langer, Christoph wrote:
Hi folks,
is there any feedback/review for this change?
I missed the 4 Nov message when it came past. No further objections.
But still unclear if modularization will permit this.
Mike
Thanks
Christoph
*From:*Langer, Christoph
*Sent:* Mittwoch, 4. November 2015 12:11
*To:* 'Michael StJohns' <mstjo...@comcast.net>
*Cc:* security-dev@openjdk.java.net
*Subject:* RE: RFR 8139436: sun.security.mscapi.KeyStore might load
incomplete data
Hi Mike,
eventually I’ve made a new webrev implementing your suggestion for a
simpler patch:
http://cr.openjdk.java.net/~clanger/webrevs/8139436.1/
<http://cr.openjdk.java.net/%7Eclanger/webrevs/8139436.1/>
@All: Is that a valid fix for this issue? Would anyone mind to
review/sponsor this?
Thanks
Christoph
*From:*Michael StJohns [mailto:mstjo...@comcast.net]
*Sent:* Donnerstag, 15. Oktober 2015 02:09
*To:* Langer, Christoph <christoph.lan...@sap.com
<mailto:christoph.lan...@sap.com>>
*Cc:* security-dev@openjdk.java.net <mailto:security-dev@openjdk.java.net>
*Subject:* Re: RFR 8139436: sun.security.mscapi.KeyStore might load
incomplete data
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
<mailto: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
<mailto: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
<https://bugs.openjdk.java.net/browse/JDK-8139436>
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8139436.0/
<http://cr.openjdk.java.net/%7Eclanger/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
