[cpp-sp] 02/02: CPPSP-8 WinHTTPRemotingService Development

Rod Widdowson rdw at steadingsoftware.com
Thu May 29 15:34:22 UTC 2025


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=bd5884a99d738c4b174b144718faa4022b0edb45

commit bd5884a99d738c4b174b144718faa4022b0edb45
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu May 29 16:28:51 2025 +0100

    CPPSP-8 WinHTTPRemotingService Development
    
    https://shibboleth.atlassian.net/browse/CPPSP-8?focusedCommentId=45909
    
    Add revocation checking to a base class and try to wire it in to
    the WinHttpRemotingService
---
 .../remoting/impl/AbstractHTTPRemotingService.cpp   |  8 ++++++++
 shibsp/remoting/impl/AbstractHTTPRemotingService.h  |  4 ++++
 shibsp/remoting/impl/WinHTTPRemotingService.cpp     | 21 +++++++++++++--------
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp b/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
index a9513de7..08381c2a 100644
--- a/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
+++ b/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
@@ -44,6 +44,7 @@ const char AbstractHTTPRemotingService::AUTH_CACHING_COOKIE_PROP_NAME[] = "authC
 const char AbstractHTTPRemotingService::CONNECT_TIMEOUT_PROP_NAME[] = "connectTimeout";
 const char AbstractHTTPRemotingService::TIMEOUT_PROP_NAME[] = "timeout";
 const char AbstractHTTPRemotingService::CA_FILE_PROP_NAME[] = "tlsCAFile";
+const char AbstractHTTPRemotingService::REVOCATION_CHECK_PROP_NAME[] = "revocationCheck";
 
 const char AbstractHTTPRemotingService::SECRET_SOURCE_TYPE_PROP_DEFAULT[] = "File";
 const char AbstractHTTPRemotingService::BASE_URL_PROP_DEFAULT[] = "http://localhost/idp/profile/sp";
@@ -51,6 +52,7 @@ const char AbstractHTTPRemotingService::AUTH_METHOD_PROP_DEFAULT[] = "basic";
 const char AbstractHTTPRemotingService::AUTH_CACHING_COOKIE_PROP_DEFAULT[] = "__Host-JSESSIONID";
 unsigned int AbstractHTTPRemotingService::CONNECT_TIMEOUT_PROP_DEFAULT = 3;
 unsigned int AbstractHTTPRemotingService::TIMEOUT_PROP_DEFAULT = 10;
+const bool AbstractHTTPRemotingService::REVOCATION_CHECK_DEFAULT = false;
 const char AbstractHTTPRemotingService::CA_FILE_PROP_DEFAULT[] = "trustlist.pem";
 
 AbstractHTTPRemotingService::AbstractHTTPRemotingService(ptree& pt)
@@ -68,6 +70,7 @@ AbstractHTTPRemotingService::AbstractHTTPRemotingService(ptree& pt)
     m_authMethod = getAuthMethod(props.getString(AUTH_METHOD_PROP_NAME, AUTH_METHOD_PROP_DEFAULT));
     m_connectTimeout = props.getUnsignedInt(CONNECT_TIMEOUT_PROP_NAME, CONNECT_TIMEOUT_PROP_DEFAULT);
     m_timeout = props.getUnsignedInt(TIMEOUT_PROP_NAME, TIMEOUT_PROP_DEFAULT);
+    m_revocationCheck = props.getBool(REVOCATION_CHECK_PROP_NAME, REVOCATION_CHECK_DEFAULT);
 
     m_caFile = props.getString(CA_FILE_PROP_NAME, CA_FILE_PROP_DEFAULT);
     if (!m_caFile.empty()) {
@@ -178,6 +181,11 @@ unsigned int AbstractHTTPRemotingService::getTimeout() const
     return m_timeout;
 }
 
+bool AbstractHTTPRemotingService::isRevocationCheck() const
+{
+    return m_revocationCheck;
+}
+
 const char* AbstractHTTPRemotingService::getCAFile() const
 {
     return m_caFile.c_str();
diff --git a/shibsp/remoting/impl/AbstractHTTPRemotingService.h b/shibsp/remoting/impl/AbstractHTTPRemotingService.h
index 01eb17fd..b569ecc7 100644
--- a/shibsp/remoting/impl/AbstractHTTPRemotingService.h
+++ b/shibsp/remoting/impl/AbstractHTTPRemotingService.h
@@ -69,6 +69,7 @@ namespace shibsp {
         std::string getAuthCachingCookieValue() const;
         unsigned int getConnectTimeout() const;
         unsigned int getTimeout() const;
+        bool isRevocationCheck() const;
         const char* getCAFile() const;
 
         // Property names and defaults.
@@ -80,6 +81,7 @@ namespace shibsp {
         static const char CONNECT_TIMEOUT_PROP_NAME[];
         static const char TIMEOUT_PROP_NAME[];
         static const char CA_FILE_PROP_NAME[];
+        static const char REVOCATION_CHECK_PROP_NAME[];
 
         static const char SECRET_SOURCE_TYPE_PROP_DEFAULT[];
         static const char BASE_URL_PROP_DEFAULT[];
@@ -88,6 +90,7 @@ namespace shibsp {
         static unsigned int CONNECT_TIMEOUT_PROP_DEFAULT;
         static unsigned int TIMEOUT_PROP_DEFAULT;
         static const char CA_FILE_PROP_DEFAULT[];
+        static const bool REVOCATION_CHECK_DEFAULT;
 
     protected:
         AbstractHTTPRemotingService(boost::property_tree::ptree& pt);
@@ -104,6 +107,7 @@ namespace shibsp {
         auth_t m_authMethod;
         unsigned int m_connectTimeout;
         unsigned int m_timeout;
+        bool m_revocationCheck;
         /** Shared lock for guarding auth cache value. */
 #if defined(HAVE_CXX17)
         std::unique_ptr<std::shared_mutex> m_authcachelock;
diff --git a/shibsp/remoting/impl/WinHTTPRemotingService.cpp b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
index bc0943fc..867cd1da 100644
--- a/shibsp/remoting/impl/WinHTTPRemotingService.cpp
+++ b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
@@ -171,7 +171,7 @@ void WinHTTPRemotingService::setupCaChecking() {
                           NULL,
                           NULL,
                           (void const**)&certContext)) {
-        m_log.crit("CryptQueryObject failure: %d", GetLastError());
+        m_log.crit("CryptQueryObject failure on file '%s': %d", caFile.c_str(), GetLastError());
         throw runtime_error("WinHHHTP failed to initialize: failed to open tlsCAFile");
     }
 
@@ -382,10 +382,6 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
     //      SECURITY_FLAG_IGNORE_CERT_CN_INVALID
     //                   Ignore errors associated with a certificate that contains a common name that is not valid.
     //
-    // We (will) selectively set
-    //      SECURITY_FLAG_IGNORE_REVOCATION
-    //                    Ignore errors associated with a revoked certificate.
-    //
     DWORD securityFlags = SECURITY_FLAG_IGNORE_UNKNOWN_CA;
 
     if (!WinHttpSetOption(request, WINHTTP_OPTION_SECURITY_FLAGS, &securityFlags, sizeof(securityFlags))) {
@@ -393,6 +389,14 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
         throw RemotingException("Send failed");
     }
 
+    if (isRevocationCheck()) {
+        DWORD enableFeature = WINHTTP_ENABLE_SSL_REVOCATION;
+        if (!WinHttpSetOption(request, WINHTTP_OPTION_ENABLE_FEATURE, &enableFeature, sizeof(enableFeature))) {
+            m_log.crit("Send.  Failed to set feature : %d", GetLastError());
+            throw RemotingException("Send failed");
+        }
+    }
+
     if (m_authScheme) {
         bool sendPass = (m_authScheme == WINHTTP_AUTH_SCHEME_BASIC || m_authScheme == WINHTTP_AUTH_SCHEME_DIGEST);
         if (!WinHttpSetCredentials(request,
@@ -587,9 +591,10 @@ void WinHTTPRemotingService::handleCert(HINTERNET handle) const
     chainPara.cbSize = sizeof(chainPara);
 
     DWORD flags = CERT_CHAIN_CACHE_END_CERT;
-    // REVOCATION!
-    //flags |= isCheckRecovation() ? CERT_CHAIN_REVOCATION_CHECK_CHAIN : 0;
-    // flags != getAdditionalGlags();
+
+    if (isRevocationCheck()) {
+        flags |= CERT_CHAIN_REVOCATION_CHECK_CHAIN;
+    }
 
     BOOL gotCertChain = CertGetCertificateChain(m_caChainEngine,
                                                 certCtx,

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list