[xmltooling] 12/24: CPPXT-110 OpenSSL 1.1 makes X509_STORE_CTX and X509_CRL opaque
Ferenc Wágner
wferi at moszumanska.debian.org
Fri Dec 16 11:56:12 UTC 2016
This is an automated email from the git hooks/post-receive script.
wferi pushed a commit to branch master
in repository xmltooling.
commit fb43000e099683aae9188f0197978437e62163c7
Author: Rod Widdowson <rdw at steadingsoftware.com>
Date: Thu Jul 14 14:10:50 2016 +0100
CPPXT-110 OpenSSL 1.1 makes X509_STORE_CTX and X509_CRL opaque
https://issues.shibboleth.net/jira/browse/CPPXT-110
Where possible we use the setters and getters.
In some cases the setters and getters have had their names changed
and so we hide this inside the new header file
security/impl/OpenSSLSupport.h
We are keeping the header file inside impl for now.
In PKIXPathValidator we used to have a class on the stack. Instead
security/impl/OpenSSLSupport (.h and .cpp) invent a new RAII class to
abstract the allocation away. The class also has a couple of methods
which hide the changes to setter/getter names
(X509_STORE_CTX_set0_trusted_stack and X509_STORE_CTX_get0_chain)
Also in PKIXPathValidator a series of macros around X509_CRL have
become a function without a const parameter so we conditionalize
a local variable's 'const' nesss.
(reapplied - previously backed out in bae0dd53)
---
Projects/vc10/xmltooling/xmltooling.vcxproj | 6 ++-
.../vc10/xmltooling/xmltooling.vcxproj.filters | 13 ++++-
.../security/impl/ExplicitKeyTrustEngine.cpp | 1 +
xmltooling/security/impl/OpenSSLSupport.cpp | 62 +---------------------
xmltooling/security/impl/OpenSSLSupport.h | 32 +----------
xmltooling/security/impl/PKIXPathValidator.cpp | 54 +++++++++++--------
xmltooling/soap/impl/CURLSOAPTransport.cpp | 11 ++--
7 files changed, 59 insertions(+), 120 deletions(-)
diff --git a/Projects/vc10/xmltooling/xmltooling.vcxproj b/Projects/vc10/xmltooling/xmltooling.vcxproj
index 2320132..48e2cf3 100644
--- a/Projects/vc10/xmltooling/xmltooling.vcxproj
+++ b/Projects/vc10/xmltooling/xmltooling.vcxproj
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup Label="ProjectConfigurations">
<ProjectConfiguration Include="Debug|Win32">
@@ -194,6 +194,7 @@
<ClCompile Include="..\..\..\XMLTooling\Lockable.cpp" />
<ClCompile Include="..\..\..\XMLTooling\Namespace.cpp" />
<ClCompile Include="..\..\..\XMLTooling\QName.cpp" />
+ <ClCompile Include="..\..\..\xmltooling\security\impl\OpenSSLSupport.cpp" />
<ClCompile Include="..\..\..\XMLTooling\security\impl\PKIXPathValidator.cpp" />
<ClCompile Include="..\..\..\XMLTooling\unicode.cpp" />
<ClCompile Include="..\..\..\XMLTooling\util\CloneInputStream.cpp" />
@@ -270,6 +271,7 @@
<ClInclude Include="..\..\..\XMLTooling\Namespace.h" />
<ClInclude Include="..\..\..\XMLTooling\PluginManager.h" />
<ClInclude Include="..\..\..\XMLTooling\QName.h" />
+ <ClInclude Include="..\..\..\xmltooling\security\impl\OpenSSLSupport.h" />
<ClInclude Include="..\..\..\XMLTooling\security\OpenSSLPathValidator.h" />
<ClInclude Include="..\..\..\XMLTooling\security\PathValidator.h" />
<ClInclude Include="..\..\..\XMLTooling\security\PKIXPathValidatorParams.h" />
@@ -346,4 +348,4 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
</ImportGroup>
-</Project>
+</Project>
\ No newline at end of file
diff --git a/Projects/vc10/xmltooling/xmltooling.vcxproj.filters b/Projects/vc10/xmltooling/xmltooling.vcxproj.filters
index bee07e2..a3e2882 100644
--- a/Projects/vc10/xmltooling/xmltooling.vcxproj.filters
+++ b/Projects/vc10/xmltooling/xmltooling.vcxproj.filters
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup>
<Filter Include="Source Files">
@@ -73,6 +73,9 @@
<UniqueIdentifier>{67DA6AB6-F800-4c08-8B7A-83BB121AAD01}</UniqueIdentifier>
<Extensions>rc;ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe;resx;tiff;tif;png;wav</Extensions>
</Filter>
+ <Filter Include="Header Files\security\impl">
+ <UniqueIdentifier>{8ce132be-735f-49f0-899a-cc0e7cb8e775}</UniqueIdentifier>
+ </Filter>
</ItemGroup>
<ItemGroup>
<ClCompile Include="..\..\..\XMLTooling\AbstractAttributeExtensibleXMLObject.cpp">
@@ -270,6 +273,9 @@
<ClCompile Include="..\..\..\XMLTooling\util\CloneInputStream.cpp">
<Filter>Source Files\util</Filter>
</ClCompile>
+ <ClCompile Include="..\..\..\xmltooling\security\impl\OpenSSLSupport.cpp">
+ <Filter>Source Files\security\impl</Filter>
+ </ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\..\..\XMLTooling\AbstractAttributeExtensibleXMLObject.h">
@@ -521,6 +527,9 @@
<ClInclude Include="..\..\..\XMLTooling\util\CloneInputStream.h">
<Filter>Header Files\util</Filter>
</ClInclude>
+ <ClInclude Include="..\..\..\xmltooling\security\impl\OpenSSLSupport.h">
+ <Filter>Header Files\security\impl</Filter>
+ </ClInclude>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\..\..\XMLTooling\xmltooling.rc">
@@ -531,4 +540,4 @@
<None Include="..\..\..\XMLTooling\config_pub.h.in" />
<None Include="..\..\..\XMLTooling\Makefile.am" />
</ItemGroup>
-</Project>
+</Project>
\ No newline at end of file
diff --git a/xmltooling/security/impl/ExplicitKeyTrustEngine.cpp b/xmltooling/security/impl/ExplicitKeyTrustEngine.cpp
index 6ad420f..5f70a70 100644
--- a/xmltooling/security/impl/ExplicitKeyTrustEngine.cpp
+++ b/xmltooling/security/impl/ExplicitKeyTrustEngine.cpp
@@ -45,6 +45,7 @@ using namespace xmltooling::logging;
using namespace xmltooling;
using namespace std;
+
using xercesc::DOMElement;
namespace xmltooling {
diff --git a/xmltooling/security/impl/OpenSSLSupport.cpp b/xmltooling/security/impl/OpenSSLSupport.cpp
index a936173..bedf872 100644
--- a/xmltooling/security/impl/OpenSSLSupport.cpp
+++ b/xmltooling/security/impl/OpenSSLSupport.cpp
@@ -27,9 +27,7 @@
#include "internal.h"
#include <openssl/x509_vfy.h>
-#include <security\impl\OpenSSLSupport.h>
-
-using namespace xmltooling;
+#include <security/impl/OpenSSLSupport.h>
X509StoreCtxRAII::X509StoreCtxRAII() : m_context(X509_STORE_CTX_new()) {
}
@@ -58,8 +56,7 @@ STACK_OF(X509) *X509StoreCtxRAII::get0Chain() {
}
// the API to set the trusted stack changed in OpenSSL1.1
-void X509StoreCtxRAII::set0TrustedStack(STACK_OF(X509) *sk)
-{
+void X509StoreCtxRAII::set0TrustedStack(STACK_OF(X509) *sk) {
if (m_context) {
#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
X509_STORE_CTX_trusted_stack(m_context, sk);
@@ -68,58 +65,3 @@ void X509StoreCtxRAII::set0TrustedStack(STACK_OF(X509) *sk)
#endif
}
}
-
-BIGNUM *DSA_get0_pubkey(const DSA *dsa)
-{
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
- return dsa->pub_key;
-#else
- BIGNUM *result;
- DSA_get0_key(dsa, &result, NULL);
- return result;
-#endif
-}
-
-BIGNUM *DSA_get0_privkey(const DSA *dsa)
-{
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
- return dsa->priv_key;
-#else
- BIGNUM *result;
- DSA_get0_key(dsa, NULL, &result);
- return result;
-#endif
-}
-
-BIGNUM *RSA_get0_n(const RSA *rsa)
-{
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
- return rsa->n;
-#else
- BIGNUM *result;
- RSA_get0_key(rsa, &result, NULL, NULL);
- return result;
-#endif
-}
-
-BIGNUM *RSA_get0_e(const RSA *rsa)
-{
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
- return rsa->e;
-#else
- BIGNUM *result;
- RSA_get0_key(rsa, NULL, &result, NULL);
- return result;
-#endif
-}
-
-BIGNUM *RSA_get0_d(const RSA *rsa)
-{
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
- return rsa->d;
-#else
- BIGNUM *result;
- RSA_get0_key(rsa, NULL, NULL, &result);
- return result;
-#endif
-}
diff --git a/xmltooling/security/impl/OpenSSLSupport.h b/xmltooling/security/impl/OpenSSLSupport.h
index 2d267be..ffaa390 100644
--- a/xmltooling/security/impl/OpenSSLSupport.h
+++ b/xmltooling/security/impl/OpenSSLSupport.h
@@ -29,28 +29,10 @@
// X509_STORE_CTX becomes opaque
#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
-# define X509_STORE_CTX_get0_cert(_ctx_) ((_ctx_)->cert)
-# define X509_STORE_CTX_get0_untrusted(_ctx_) ((_ctx_)->untrusted)
-
-# define EVP_PKEY_get0_DSA(_pkey_) ((_pkey_)->pkey.dsa)
-# define EVP_PKEY_get0_RSA(_pkey_) ((_pkey_)->pkey.rsa)
-#endif
-
-#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
-# define EVP_PKEY_id(_evp_) ((_evp_)->type)
-#endif
-
-// BIO_s_file and BIO_s_file_internal
-// in 0.9.8 #define BIO_s_file BIO_s_file_internal, uses both
-// in 1.0.0 #define BIO_s_file_internal BIO_s_file, uses both
-// in 1.0.1 #define BIO_s_file_internal BIO_s_file, uses both
-// in 1.0.1 #define BIO_s_file_internal BIO_s_file, uses both
-// in 1.1 no BIO_s_file_internal
-#if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
-# define BIO_s_file_internal BIO_s_file
+# define X509_STORE_CTX_get0_cert(ctx) (ctx->cert)
+# define X509_STORE_CTX_get0_untrusted(ctx) (ctx->untrusted)
#endif
-namespace xmltooling {
// RAII for the now opaque X509_STORE_CTX
class X509StoreCtxRAII
{
@@ -69,13 +51,3 @@ namespace xmltooling {
private:
X509_STORE_CTX *m_context;
};
-
-
- BIGNUM *DSA_get0_pubkey(const DSA *dsa);
- BIGNUM *DSA_get0_privkey(const DSA *dsa);
-
- BIGNUM *RSA_get0_n(const RSA *rsa);
- BIGNUM *RSA_get0_d(const RSA *rsa);
- BIGNUM *RSA_get0_e(const RSA *rsa);
-
-}
diff --git a/xmltooling/security/impl/PKIXPathValidator.cpp b/xmltooling/security/impl/PKIXPathValidator.cpp
index 3ac8308..90cee59 100644
--- a/xmltooling/security/impl/PKIXPathValidator.cpp
+++ b/xmltooling/security/impl/PKIXPathValidator.cpp
@@ -30,6 +30,7 @@
#include "security/OpenSSLCryptoX509CRL.h"
#include "security/PKIXPathValidatorParams.h"
#include "security/SecurityHelper.h"
+#include "security/impl/OpenSSLSupport.h"
#include "util/NDC.h"
#include "util/PathResolver.h"
#include "util/Threads.h"
@@ -54,7 +55,9 @@ namespace {
{
if (!ok) {
Category::getInstance("OpenSSL").error(
- "path validation failure at depth(%d): %s", ctx->error_depth, X509_verify_cert_error_string(ctx->error)
+ "path validation failure at depth(%d): %s",
+ X509_STORE_CTX_get_error_depth(ctx),
+ X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx))
);
}
return ok;
@@ -291,18 +294,24 @@ bool PKIXPathValidator::validate(X509* EE, STACK_OF(X509)* untrusted, const Path
// This contains the state of the validate operation.
int count=0;
- X509_STORE_CTX ctx;
+ X509StoreCtxRAII ctxContainer;
+
+ if (!ctxContainer.of()) {
+ log_openssl();
+ X509_STORE_free(store);
+ return false;
+ }
// AFAICT, EE and untrusted are passed in but not owned by the ctx.
#if (OPENSSL_VERSION_NUMBER >= 0x00907000L)
- if (X509_STORE_CTX_init(&ctx,store,EE,untrusted) != 1) {
+ if (X509_STORE_CTX_init(ctxContainer.of(),store,EE,untrusted) != 1) {
log_openssl();
m_log.error("unable to initialize X509_STORE_CTX");
X509_STORE_free(store);
return false;
}
#else
- X509_STORE_CTX_init(&ctx,store,EE,untrusted);
+ X509_STORE_CTX_init(ctxContainer.of(),store,EE,untrusted);
#endif
STACK_OF(X509)* CAstack = sk_X509_new_null();
@@ -316,15 +325,15 @@ bool PKIXPathValidator::validate(X509* EE, STACK_OF(X509)* untrusted, const Path
m_log.debug("supplied (%d) CA certificate(s)", count);
// Seems to be most efficient to just pass in the CA stack.
- X509_STORE_CTX_trusted_stack(&ctx,CAstack);
- X509_STORE_CTX_set_depth(&ctx,100); // we check the depth down below
- X509_STORE_CTX_set_verify_cb(&ctx,error_callback);
+ ctxContainer.set0TrustedStack(CAstack);
+ X509_STORE_CTX_set_depth(ctxContainer.of(),100); // we check the depth down below
+ X509_STORE_CTX_set_verify_cb(ctxContainer.of(),error_callback);
// Do a first pass verify. If CRLs aren't used, this is the only pass.
- int ret = X509_verify_cert(&ctx);
+ int ret = X509_verify_cert(ctxContainer.of());
if (ret == 1) {
// Now see if the depth was acceptable by counting the number of intermediates.
- int depth=sk_X509_num(ctx.chain)-2;
+ int depth=sk_X509_num(ctxContainer.get0Chain())-2;
if (pkixParams->getVerificationDepth() < depth) {
m_log.error(
"certificate chain was too long (%d intermediates, only %d allowed)",
@@ -340,7 +349,7 @@ bool PKIXPathValidator::validate(X509* EE, STACK_OF(X509)* untrusted, const Path
#if (OPENSSL_VERSION_NUMBER >= 0x00907000L)
// After the first X509_verify_cert call, the ctx can no longer be used
// (subsequent calls will fail with OpenSSL 1.0.1p / 1.0.2d or later).
- X509_STORE_CTX_cleanup(&ctx);
+ X509_STORE_CTX_cleanup(ctxContainer.of());
// When we add CRLs, we have to be sure the nextUpdate hasn't passed, because OpenSSL won't accept
// the CRL in that case. If we end up not adding a CRL for a particular link in the chain, the
@@ -403,23 +412,23 @@ bool PKIXPathValidator::validate(X509* EE, STACK_OF(X509)* untrusted, const Path
// Do a second pass verify with CRLs in place. Reinitialize ctx, see
// https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=aae41f8c54257d9fa6904d3a9aa09c5db6cefd0d
#if (OPENSSL_VERSION_NUMBER >= 0x00907000L)
- if (X509_STORE_CTX_init(&ctx,store,EE,untrusted) != 1) {
+ if (X509_STORE_CTX_init(ctxContainer.of(),store,EE,untrusted) != 1) {
log_openssl();
m_log.error("unable to initialize X509_STORE_CTX");
ret = 0;
}
#else
- X509_STORE_CTX_init(&ctx,store,EE,untrusted);
+ X509_STORE_CTX_init(ctxContainer.of(),store,EE,untrusted);
#endif
if (ret != 0) {
- X509_STORE_CTX_trusted_stack(&ctx,CAstack);
- X509_STORE_CTX_set_depth(&ctx,100); // already checked above
- X509_STORE_CTX_set_verify_cb(&ctx,error_callback);
+ ctxContainer.set0TrustedStack(CAstack);
+ X509_STORE_CTX_set_depth(ctxContainer.of(),100); // already checked above
+ X509_STORE_CTX_set_verify_cb(ctxContainer.of(),error_callback);
if (pkixParams->getRevocationChecking() == PKIXPathValidatorParams::REVOCATION_FULLCHAIN)
- X509_STORE_CTX_set_flags(&ctx, X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL);
+ X509_STORE_CTX_set_flags(ctxContainer.of(), X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL);
else
- X509_STORE_CTX_set_flags(&ctx, X509_V_FLAG_CRL_CHECK);
- ret = X509_verify_cert(&ctx);
+ X509_STORE_CTX_set_flags(ctxContainer.of(), X509_V_FLAG_CRL_CHECK);
+ ret = X509_verify_cert(ctxContainer.of());
}
#else
m_log.warn("CRL checking is enabled, but OpenSSL version is too old");
@@ -431,13 +440,13 @@ bool PKIXPathValidator::validate(X509* EE, STACK_OF(X509)* untrusted, const Path
m_log.debug("successfully validated certificate chain");
}
#if defined(X509_V_ERR_NO_EXPLICIT_POLICY) && (OPENSSL_VERSION_NUMBER < 0x10000000L)
- else if (X509_STORE_CTX_get_error(&ctx) == X509_V_ERR_NO_EXPLICIT_POLICY && !pkixParams->isPolicyMappingInhibited()) {
+ else if (X509_STORE_CTX_get_error(ctxContainer.of()) == X509_V_ERR_NO_EXPLICIT_POLICY && !pkixParams->isPolicyMappingInhibited()) {
m_log.warn("policy mapping requires OpenSSL 1.0.0 or later");
}
#endif
// Clean up...
- X509_STORE_CTX_cleanup(&ctx);
+ X509_STORE_CTX_cleanup(ctxContainer.of());
X509_STORE_free(store);
sk_X509_free(CAstack);
@@ -546,7 +555,10 @@ XSECCryptoX509CRL* PKIXPathValidator::getRemoteCRLs(const char* cdpuri) const
bool PKIXPathValidator::isFreshCRL(XSECCryptoX509CRL *c, Category* log) const
{
if (c) {
- const X509_CRL* crl = static_cast<OpenSSLCryptoX509CRL*>(c)->getOpenSSLX509CRL();
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+ const
+#endif
+ X509_CRL* crl = static_cast<OpenSSLCryptoX509CRL*>(c)->getOpenSSLX509CRL();
time_t thisUpdate = getCRLTime(X509_CRL_get_lastUpdate(crl));
time_t nextUpdate = getCRLTime(X509_CRL_get_nextUpdate(crl));
time_t now = time(nullptr);
diff --git a/xmltooling/soap/impl/CURLSOAPTransport.cpp b/xmltooling/soap/impl/CURLSOAPTransport.cpp
index 38e9271..b7ebe25 100644
--- a/xmltooling/soap/impl/CURLSOAPTransport.cpp
+++ b/xmltooling/soap/impl/CURLSOAPTransport.cpp
@@ -30,6 +30,7 @@
#include "security/CredentialCriteria.h"
#include "security/OpenSSLTrustEngine.h"
#include "security/OpenSSLCredential.h"
+#include "security/impl/OpenSSLSupport.h"
#include "soap/HTTPSOAPTransport.h"
#include "soap/OpenSSLSOAPTransport.h"
#include "util/NDC.h"
@@ -711,20 +712,20 @@ int xmltooling::verify_callback(X509_STORE_CTX* x509_ctx, void* arg)
ctx->m_criteria->setUsage(Credential::TLS_CREDENTIAL);
// Bypass name check (handled for us by curl).
ctx->m_criteria->setPeerName(nullptr);
- success = ctx->m_trustEngine->validate(x509_ctx->cert,x509_ctx->untrusted,*(ctx->m_peerResolver),ctx->m_criteria);
+ success = ctx->m_trustEngine->validate(X509_STORE_CTX_get0_cert(x509_ctx),X509_STORE_CTX_get0_untrusted(x509_ctx),*(ctx->m_peerResolver),ctx->m_criteria);
}
else {
// Bypass name check (handled for us by curl).
CredentialCriteria cc;
cc.setUsage(Credential::TLS_CREDENTIAL);
- success = ctx->m_trustEngine->validate(x509_ctx->cert,x509_ctx->untrusted,*(ctx->m_peerResolver),&cc);
+ success = ctx->m_trustEngine->validate(X509_STORE_CTX_get0_cert(x509_ctx),X509_STORE_CTX_get0_untrusted(x509_ctx),*(ctx->m_peerResolver),&cc);
}
if (!success) {
log.error("supplied TrustEngine failed to validate SSL/TLS server certificate");
- if (x509_ctx->cert) {
+ if (X509_STORE_CTX_get0_cert(x509_ctx)) {
BIO* b = BIO_new(BIO_s_mem());
- X509_print(b, x509_ctx->cert);
+ X509_print(b, X509_STORE_CTX_get0_cert(x509_ctx));
BUF_MEM* bptr = nullptr;
BIO_get_mem_ptr(b, &bptr);
if (bptr && bptr->length > 0) {
@@ -736,7 +737,7 @@ int xmltooling::verify_callback(X509_STORE_CTX* x509_ctx, void* arg)
}
BIO_free(b);
}
- x509_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION; // generic error, check log for plugin specifics
+ X509_STORE_CTX_set_error(x509_ctx, X509_V_ERR_APPLICATION_VERIFICATION); // generic error, check log for plugin specifics
ctx->setAuthenticated(false);
return ctx->m_mandatory ? 0 : 1;
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-shibboleth/xmltooling.git
More information about the Pkg-shibboleth-devel
mailing list