File 0001-CPPXT-110-OpenSSL-1.1-makes-X509_STORE_CTX... of Package xmltooling

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-CPPXT-110-OpenSSL-1.1-makes-X509_STORE_CTX-and-X509_.patch of Package xmltooling (Revision 812ea2d9516c7958e47ca64a2cd99f94)

Currently displaying revision 812ea2d9516c7958e47ca64a2cd99f94 , Show latest

From fcdcf871c32bf5eb8a37bf874d1ad27851984dc3 Mon Sep 17 00:00:00 2001
From: Rod Widdowson <rdw@steadingsoftware.com>
Date: Thu, 14 Jul 2016 14:10:50 +0100
Subject: [PATCH 01/31] 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 had their names changed and so
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 one 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 variables 'const' nesss.
---
 Projects/vc10/xmltooling/xmltooling.vcxproj        |  6 +-
 .../vc10/xmltooling/xmltooling.vcxproj.filters     | 13 ++++-
 .../security/impl/ExplicitKeyTrustEngine.cpp       |  1 +
 xmltooling/security/impl/OpenSSLSupport.cpp        | 67 ++++++++++++++++++++++
 xmltooling/security/impl/OpenSSLSupport.h          | 53 +++++++++++++++++
 xmltooling/security/impl/PKIXPathValidator.cpp     | 54 ++++++++++-------
 xmltooling/soap/impl/CURLSOAPTransport.cpp         | 11 ++--
 7 files changed, 175 insertions(+), 30 deletions(-)
 create mode 100644 xmltooling/security/impl/OpenSSLSupport.cpp
 create mode 100644 xmltooling/security/impl/OpenSSLSupport.h

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
new file mode 100644
index 0000000..581cc9f
--- /dev/null
+++ b/xmltooling/security/impl/OpenSSLSupport.cpp
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the University Corporation for Advanced Internet
+ * Development, Inc. (UCAID) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ *
+ * UCAID licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the
+ * License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied. See the License for the specific
+ * language governing permissions and limitations under the License.
+ */
+
+/**
+ * @file xmltooling/impl/OpenSSLSupport.h
+ * 
+ * Various functions and classes to abstract away some of the
+ * differences introduced by (at least) OpenSSL 1.1
+ */
+
+#include "internal.h"
+#include <openssl/x509_vfy.h> 
+#include <security\impl\OpenSSLSupport.h>
+
+X509StoreCtxRAII::X509StoreCtxRAII() : m_context(X509_STORE_CTX_new()) {
+}
+
+X509StoreCtxRAII::~X509StoreCtxRAII() {
+    if (m_context) {
+        X509_STORE_CTX_free(m_context);
+    }
+}
+
+X509_STORE_CTX *X509StoreCtxRAII::of(void) {
+    return m_context;
+}
+
+// the API to get the chain changed in OpenSSL1.1
+
+STACK_OF(X509) *X509StoreCtxRAII::get0Chain() {
+    if (!m_context) {
+        return nullptr;
+    }
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+    return X509_STORE_CTX_get_chain(m_context);
+#else
+    return X509_STORE_CTX_get0_chain(m_context);
+#endif
+}
+
+// the API to set the trusted stack changed in OpenSSL1.1
+void X509StoreCtxRAII::set0TrustedStack(STACK_OF(X509) *sk) {
+    if (m_context) {
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+        X509_STORE_CTX_trusted_stack(m_context, sk);
+#else
+        X509_STORE_CTX_set0_trusted_stack(m_context, sk);
+#endif
+    }
+}
diff --git a/xmltooling/security/impl/OpenSSLSupport.h b/xmltooling/security/impl/OpenSSLSupport.h
new file mode 100644
index 0000000..ffaa390
--- /dev/null
+++ b/xmltooling/security/impl/OpenSSLSupport.h
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the University Corporation for Advanced Internet
+ * Development, Inc. (UCAID) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ *
+ * UCAID licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the
+ * License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied. See the License for the specific
+ * language governing permissions and limitations under the License.
+ */
+
+/**
+ * @file xmltooling/impl/OpenSSLSupport.h
+ *
+ * Various functions and classes to abstract away some of the
+ * differences introduced by (at least) OpenSSL 1.1
+ */
+#include <openssl/x509_vfy.h>
+
+// 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)
+#endif
+
+    // RAII for the now opaque X509_STORE_CTX
+    class X509StoreCtxRAII
+    {
+    public:
+        X509StoreCtxRAII();
+
+        ~X509StoreCtxRAII();
+
+        X509_STORE_CTX *of(void);
+
+        // the API to get the chain changed in OpenSSL1.1
+        STACK_OF(X509) *get0Chain();
+
+        void set0TrustedStack(STACK_OF(X509) *sk);
+
+    private:
+        X509_STORE_CTX *m_context;
+    };
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;
     }
-- 
2.13.6

Places

File 0001-CPPXT-110-OpenSSL-1.1-makes-X509_STORE_CTX-and-X509_.patch of Package xmltooling (Revision 812ea2d9516c7958e47ca64a2cd99f94)

Places