PKIX revocation checking calls OpenSSL's X509_verify_cert in an unsupported way (breaks with OpenSSL 1.0.1p/1.0.2d and later)

Description

CRL checking for the PKIX trust engine (as implemented with the improvements in https://shibboleth.atlassian.net/browse/CPPXT-64#icft=CPPXT-64) apparently calls X509_verify_cert in an unsupported way. As the OpenSSL documentation now states:

X509_STORE_CTX_init() sets up ctx for a subsequent verification
operation. It must be called before each call to X509_verify_cert(),
i.e. a ctx is only good for one call to X509_verify_cert(); if you want
to verify a second certificate with the same ctx then you must call
X509_XTORE_CTX_cleanup() and then X509_STORE_CTX_init() again before the
second call to X509_verify_cert().

This behavior is now enforced in current OpenSSL releases (Reject calls to X509_verify_cert that have not been reinitialised was applied to the 1.0.1 and 1.0.2 branches and is present in 1.0.1p/1.0.2d and later), so a trust engine which enables revocation checking fails to validate the respective signature.

The attached patch addresses this problem. It should be noted that this is not only a question of compatibility with the latest releases – according to the commit message, — at best, the results would be undefined in the case of a reused X509_STORE_CTX.

Environment

None

Attachments

1
  • 04 Aug 2015, 08:14 AM

Activity

Scott Cantor August 4, 2015 at 6:57 PM

Tagged, tarballs are posted to the opensaml 2.5.5 directory.

Windows build of service release is done, testing TBD.

Macport and RPMs coming.

Scott Cantor August 4, 2015 at 3:16 PM

Thx, I'll push out a patch today.

Former user August 4, 2015 at 2:44 PM

I have successfully tested the ed8a88a560d5076dcdf70f01c4d5c8581bfb75bc patch against 1.5.5 and OpenSSL 1.0.1p, yes. With 1.5.5, I'm getting "DEBUG XMLTooling.TrustEngine.PKIX : failed to validate certificate chain using supplied PKIX information", and with 1.5.5 and the patch, it's the expected "DEBUG XMLTooling.PathValidator.PKIX : successfully validated certificate chain".

Scott Cantor August 4, 2015 at 1:58 PM

Cleaned up patch here:

http://git.shibboleth.net/view/?p=cpp-xmltooling.git;a=commit;h=ed8a88a560d5076dcdf70f01c4d5c8581bfb75bc

This adds additional error checking, avoids a double free of the ctx, and skips the CRL pass when the first pass failed.

Kaspar, do you have the ability to test this? If not I'll have to find some testing time and that will probably a few days out. If you can test, I can push 1.5.6 for everything but Windows today and then I should be able to get an installer update done for Windows in a day or two.

Scott Cantor August 4, 2015 at 1:35 PM

Never mind, I see it's doing two passes. I guess that was to prevent the network call for a remote CRL, but it doesn't seem to be doing that because the first pass result doesn't prevent the second pass.

Seems like the conditional that's looking at the revocation setting should also check if the result was 0 already and just go straight to "failure" at the end of the function. It isn't exactly broken now, but it's a waste. Adding in the CRLs can't possibly make a failed check succeed.

Fixed

Details

Assignee

Reporter

Components

Fix versions

Affects versions

Created August 4, 2015 at 8:17 AM
Updated August 5, 2015 at 1:28 AM
Resolved August 5, 2015 at 1:28 AM