jglobus / JGlobus

jGlobus is a collection of Java client libraries for Globus® Toolkit security, GRAM, and GridFTP.
http://www.globus.org/toolkit/jglobus/
Apache License 2.0
24 stars 44 forks source link

ssl-proxies: Improve RFC 3820 compliance of key usage check #90

Closed gbehrmann closed 11 years ago

gbehrmann commented 11 years ago

Each certificate in a certificate chain can have a key usage field. The existing JGlobus code checks that each proxy certificate does not ave more key usages than the issuing certificate. This seems like a sensible check, however it conflicts with the algorithm described in RFC 3820.

The RFC states that all key usage (and extended key usage) extensions of all proxy certificates and the EEC must be collected in a list and the effective key usage computed by the intersection of all extensions:

Since JGlobus does not actually utilize the key usage, this patch simply removes the faulty check. The effective key usage is not computed.

We have observed real-world proxy certificates that failed with the existing check in JGlobus 2, even though they are valid proxies according to the RFC. These proxies also work with JGlobus 1.

bbockelm commented 11 years ago

Hi,

Looks like this is breaking unit tests - can you please review those?

Brian

gbehrmann commented 11 years ago

You have to be more specific that that - since unit tests in general fail in JGlobus due to it having external dependencies.

bbockelm commented 11 years ago

Here are the tests that now fail after the improved key usage work:

Failed tests: testCrlsChecks(org.globus.gsi.proxy.ProxyPathValidatorTest)

Tests in error: testResctrictedWithOtherExt(org.globus.gsi.bc.BouncyCastleCertProcessingFactoryTest) testExtensions(org.globus.gsi.bc.BouncyCastleCertProcessingFactoryTest) testCreateProxyCertInfo2(org.globus.gsi.proxy.ext.ProxyCertInfoTest) testParseProxyCertInfo(org.globus.gsi.proxy.ext.ProxyCertInfoTest) testGetCertificateType(org.globus.gsi.bc.BouncyCastleUtilTest) testGetCertificateType2(org.globus.gsi.bc.BouncyCastleUtilTest) testGetCertificateType3(org.globus.gsi.bc.BouncyCastleUtilTest) testGetGsi2IdentityCertificate(org.globus.gsi.bc.BouncyCastleUtilTest) testValidateGsi3PathGood(org.globus.gsi.bc.BouncyCastleUtilTest) testCrlsChecks(org.globus.gsi.provider.TestTrustManager) testCrlsChecks(org.globus.gsi.provider.TestProxyPathValidator)

I think the underlying cause is this one:

Caused by: java.security.cert.CertPathValidatorException: Certificate O=test CA1,OU=simple ca,CN=Globus Test violated key usage policy. at org.globus.gsi.trustmanager.X509ProxyCertPathValidator.checkKeyUsage(X509ProxyCertPathValidator.java:428) at org.globus.gsi.trustmanager.X509ProxyCertPathValidator.validate(X509ProxyCertPathValidator.java:194) at org.globus.gsi.trustmanager.X509ProxyCertPathValidator.engineValidate(X509ProxyCertPathValidator.java:111) at org.globus.gsi.proxy.ProxyPathValidator.validate(ProxyPathValidator.java:321)

Perhaps it doesn't like the test CA now?

FWIW, I normally have zero failures in either ssl-proxies or gss.

gbehrmann commented 11 years ago

Thanks. I usually skip all the unit tests in JGlobus as at least some of them had dependencies on external services. Maybe they have been removed now.

Anyway, I am a little handicapped at the moment as my grid cert expired - at JGlobus tests seem to depend on the users grid cert too.... (bad style, as it makes it difficult to run stuff on a CI server).

bbockelm commented 11 years ago

Hi Gerd,

Luckily, the test failures are the completely self-contained ones (no user proxy needed). I think if you focus on this one, you probably will get the rest:

mvn test -Dtest=org.globus.gsi.provider.TestTrustManager

Brian

gbehrmann commented 11 years ago

Funny thing is that at least ProxyPathValidatorTest seems to pass for me (2.0 branch):

Running org.globus.gsi.proxy.ProxyPathValidatorTest [main] WARN gsi.TrustedCertificates - no signing policy for ca cert O=Grid,CN=dCache Test CA - Thu May 10 09:37:53 CEST 2012 [main] WARN gsi.TrustedCertificates - no signing policy for ca cert O=Grid,CN=dCache Test CA - Fri May 11 15:18:51 CEST 2012 Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.172 sec

gbehrmann commented 11 years ago

Same for TestTrustManager:

Running org.globus.gsi.provider.TestTrustManager Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.209 sec

Results :

Tests run: 13, Failures: 0, Errors: 0, Skipped: 0

gbehrmann commented 11 years ago

Also, from what you are writing above, it is now failing a cert path in validation. However my patch removed parts of the check - the check is now more loose than before. I don't understand how it should now reject something that was accepted before.

gbehrmann commented 11 years ago

Hang on - I didn't have the latest changes from the upstream repo. Let me retest.

gbehrmann commented 11 years ago

Confirmed. The 2.0 branch is fine - the fault was introduced by the enum change. Will fix.

gbehrmann commented 11 years ago

See #92.

bbockelm commented 11 years ago

Ok, some tests have resolved themselves.

Unfortunately, I now get the following errors:

java.lang.NoSuchMethodError: org.bouncycastle.asn1.DERSequence.<init>(Lorg/bouncycastle/asn1/DEREncodableVector;)V at org.globus.gsi.proxy.ext.ProxyPolicy.getDERObject(ProxyPolicy.java:146) at org.globus.gsi.proxy.ext.ProxyCertInfo.getDERObject(ProxyCertInfo.java:139) at org.globus.gsi.bc.BouncyCastleX509Extension.setValue(BouncyCastleX509Extension.java:54) at org.globus.gsi.proxy.ext.ProxyCertInfoExtension.<init>(ProxyCertInfoExtension.java:34) at org.globus.gsi.bc.BouncyCastleCertProcessingFactoryTest.testResctrictedWithOtherExt(BouncyCastleCertProcessingFactoryTest.java:98)

It's not clear where this is coming from (this commit or something else). I'll have to bisect.

gbehrmann commented 11 years ago

Works for me:

Running org.globus.gsi.bc.BouncyCastleCertProcessingFactoryTest Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.546 sec

Results :

Tests run: 3, Failures: 0, Errors: 0, Skipped: 0

bbockelm commented 11 years ago

Yeah - I suspect it's a bouncycastle issue. I see it with 1.46. What version is working for you?

paulmillar commented 11 years ago

I've seen this error before. It is a bouncycastle issue, in particular, between 1.45 and 1.46; as 'master' uses 1.46 and 2.0 uses 1.45, it's relatively easy to get confused.

Try doing a 'mvn clean'

bbockelm commented 11 years ago

Ugh. Stupid maven.

All tests work now. Thanks!