[cpp-sp] branch main updated: Hide header transformations.

Scott Cantor cantor.2 at osu.edu
Wed Apr 23 18:51:53 UTC 2025


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

scantor 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=05662b2ccd6a9d066eba7f4bc9b9a24bc6282a3d

The following commit(s) were added to refs/heads/main by this push:
     new 05662b2c Hide header transformations.
05662b2c is described below

commit 05662b2ccd6a9d066eba7f4bc9b9a24bc6282a3d
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Apr 23 14:51:49 2025 -0400

    Hide header transformations.
---
 apache/mod_shib_24.cpp                             | 23 +++++++----------
 fastcgi/shibauthorizer.cpp                         |  2 +-
 fastcgi/shibresponder.cpp                          |  2 +-
 iis7_shib/IIS7Request.cpp                          | 26 +++++++++----------
 iis7_shib/headers/IIS7Request.hpp                  |  2 +-
 shibsp/AbstractSPRequest.cpp                       | 11 ++++++++
 shibsp/AbstractSPRequest.h                         |  9 +++++++
 shibsp/SPRequest.h                                 | 12 ++++++---
 .../impl/DefaultAttributeConfiguration.cpp         | 29 +++++++---------------
 9 files changed, 62 insertions(+), 54 deletions(-)

diff --git a/apache/mod_shib_24.cpp b/apache/mod_shib_24.cpp
index 1faf27b5..42fb67a6 100644
--- a/apache/mod_shib_24.cpp
+++ b/apache/mod_shib_24.cpp
@@ -459,32 +459,27 @@ public:
   vector<const char*>::size_type getParameters(const char* name, vector<const char*>& values) const {
       return AbstractSPRequest::getParameters(name, values);
   }
-  void clearHeader(const char* rawname, const char* cginame) {
+  void clearHeader(const char* name) {
     if (isUseHeaders()) {
        // ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO,0, m_req, "shib_clear_header: hdr\n");
         if (g_checkSpoofing && m_firsttime) {
             if (m_allhttp.empty()) {
-                // First time, so populate set with "CGI" versions of client-supplied headers.
+                // First time, so populate cached guard set with "CGI" versions of client-supplied headers.
                 const apr_array_header_t *hdrs_arr = apr_table_elts(m_req->headers_in);
                 const apr_table_entry_t *hdrs = (const apr_table_entry_t *) hdrs_arr->elts;
                 for (int i = 0; i < hdrs_arr->nelts; ++i) {
-                    if (!hdrs[i].key)
-                        continue;
-                    string cgiversion("HTTP_");
-                    const char* pch = hdrs[i].key;
-                    while (*pch) {
-                        cgiversion += (isalnum(*pch) ? toupper(*pch) : '_');
-                        pch++;
+                    if (hdrs[i].key) {
+                        m_allhttp.insert(getCGINameForHeader(hdrs[i].key));
                     }
-                    m_allhttp.insert(cgiversion);
                 }
             }
 
-            if (m_allhttp.count(cginame) > 0)
-                throw SessionException(string("Attempt to spoof header ") + rawname + " was detected.");
+            if (m_allhttp.count(getCGINameForHeader(name)) > 0) {
+                throw SessionException(string("Attempt to spoof header ") + name + " was detected.");
+            }
         }
-        apr_table_unset(m_req->headers_in, rawname);
-        apr_table_set(m_req->headers_in, rawname, g_unsetHeaderValue.c_str());
+        apr_table_unset(m_req->headers_in, name);
+        apr_table_set(m_req->headers_in, name, g_unsetHeaderValue.c_str());
     }
   }
   void setHeader(const char* name, const char* value) {
diff --git a/fastcgi/shibauthorizer.cpp b/fastcgi/shibauthorizer.cpp
index 03b52621..b9a04ca4 100644
--- a/fastcgi/shibauthorizer.cpp
+++ b/fastcgi/shibauthorizer.cpp
@@ -114,7 +114,7 @@ public:
         const char* s = FCGX_GetParam("REMOTE_ADDR", m_req->envp);
         return s ? s : "";
     }
-    void clearHeader(const char* rawname, const char* cginame) {
+    void clearHeader(const char* name) {
         // No need, since we use environment variables.
     }
     void setHeader(const char* name, const char* value) {
diff --git a/fastcgi/shibresponder.cpp b/fastcgi/shibresponder.cpp
index d5a1bd36..cefb9666 100644
--- a/fastcgi/shibresponder.cpp
+++ b/fastcgi/shibresponder.cpp
@@ -206,7 +206,7 @@ public:
 
     // Not used in the extension.
 
-    virtual void clearHeader(const char* rawname, const char* cginame) {
+    virtual void clearHeader(const char* name) {
         throw runtime_error("clearHeader not implemented by FastCGI responder.");
     }
 
diff --git a/iis7_shib/IIS7Request.cpp b/iis7_shib/IIS7Request.cpp
index f81c95b5..56569678 100644
--- a/iis7_shib/IIS7Request.cpp
+++ b/iis7_shib/IIS7Request.cpp
@@ -205,7 +205,7 @@ const char* IIS7Request::getMethod() const
     return m_request->GetHttpMethod();
 }
 
-void IIS7Request::clearHeader(const char* rawname, const char* cginame)
+void IIS7Request::clearHeader(const char* name)
 {
     if (isUseHeaders()) {
         if (g_checkSpoofing && m_firsttime) {
@@ -219,14 +219,14 @@ void IIS7Request::clearHeader(const char* rawname, const char* cginame)
                 m_allhttp =  (nullptr == val) ? "" : val;
             }
             if (!m_allhttp.empty()) {
-                string hdr = (m_safeHeaderNames ? ("HTTP_" + makeSafeHeader(cginame + 5)) : string(cginame)) + ':';
+                string hdr = (m_safeHeaderNames ? ("HTTP_" + makeSafeHeader(getCGINameForHeader(name).c_str() + 5)) : getCGINameForHeader(name)) + ':';
                 if (strstr(m_allhttp.c_str(), hdr.c_str())) {
                     throw SessionException(string("Attempt to spoof header (") + hdr + ") was detected.");
                 }
             }
         }
         string unsetHeaderValue(g_Config->getAgent().getString(Agent::UNSET_HEADER_VALUE_PROP_NAME, ""));
-        HRESULT hr = m_request->SetHeader(m_safeHeaderNames ? makeSafeHeader(rawname).c_str() : rawname,
+        HRESULT hr = m_request->SetHeader(m_safeHeaderNames ? makeSafeHeader(name).c_str() : name,
             unsetHeaderValue.c_str(), static_cast<USHORT>(unsetHeaderValue.length()), TRUE);
         if (FAILED(hr)) {
             throwError("clearHeader", hr);
@@ -234,6 +234,16 @@ void IIS7Request::clearHeader(const char* rawname, const char* cginame)
     }
 }
 
+string IIS7Request::makeSafeHeader(const char* rawname) const
+{
+    string hdr;
+    for (; *rawname; ++rawname) {
+        if (isalnum(*rawname))
+            hdr += *rawname;
+    }
+    return hdr;
+}
+
 long IIS7Request::returnDecline()
 {
     return RQ_NOTIFICATION_CONTINUE;
@@ -440,16 +450,6 @@ long IIS7Request::sendRedirect(const char* url)
     return RQ_NOTIFICATION_FINISH_REQUEST;
 }
 
-string IIS7Request::makeSafeHeader(const char* rawname) const
-{
-    string hdr;
-    for (; *rawname; ++rawname) {
-        if (isalnum(*rawname))
-            hdr += *rawname;
-    }
-    return hdr;
-}
-
 void IIS7Request::logFatal(const string& operation, HRESULT hr) const
 {
     string msg(operation + " failed: " + boost::lexical_cast<string>(hr));
diff --git a/iis7_shib/headers/IIS7Request.hpp b/iis7_shib/headers/IIS7Request.hpp
index 905f242f..ef975dc1 100644
--- a/iis7_shib/headers/IIS7Request.hpp
+++ b/iis7_shib/headers/IIS7Request.hpp
@@ -63,7 +63,7 @@ protected:
     void setRemoteUser(const char* user);
     const vector<string>& getClientCertificates() const;
     const char* getMethod() const;
-    void  clearHeader(const char* rawname, const char* cginame);
+    void  clearHeader(const char* name);
     long  returnDecline();
     long  returnOK();
     string getRemoteAddr() const;
diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 30e0e97f..f2340145 100644
--- a/shibsp/AbstractSPRequest.cpp
+++ b/shibsp/AbstractSPRequest.cpp
@@ -506,3 +506,14 @@ bool AbstractSPRequest::isPriorityEnabled(Priority::Value level) const
 {
     return m_log.isPriorityEnabled(level);
 }
+
+string AbstractSPRequest::getCGINameForHeader(const char* name) const
+{
+    string cgiversion("HTTP_");
+    const char* pch = name;
+    while (*pch) {
+        cgiversion += (isalnum(*pch) ? toupper(*pch) : '_');
+        pch++;
+    }
+    return cgiversion;
+}
diff --git a/shibsp/AbstractSPRequest.h b/shibsp/AbstractSPRequest.h
index 58ba818c..dd12adf2 100644
--- a/shibsp/AbstractSPRequest.h
+++ b/shibsp/AbstractSPRequest.h
@@ -81,6 +81,15 @@ namespace shibsp {
         void log(Priority::Value level, const std::string& msg) const;
         bool isPriorityEnabled(Priority::Value level) const;
 
+    protected:
+        /**
+         * Gets the transformed header name constructed from a raw input name by transforming
+         * punctuation into underscores and prefixing with "HTTP_".
+         * 
+         * @return CGI name for input header name
+         */ 
+        std::string getCGINameForHeader(const char* name) const;
+
     private:
         Category& m_log;
         Agent& m_agent;
diff --git a/shibsp/SPRequest.h b/shibsp/SPRequest.h
index 40a62ea4..781e5041 100644
--- a/shibsp/SPRequest.h
+++ b/shibsp/SPRequest.h
@@ -148,12 +148,16 @@ namespace shibsp {
         virtual std::string getSecureHeader(const char* name) const=0;
 
         /**
-         * Ensures no value exists for a request header.
+         * Ensures no value exists for a request header by installing an empty or hardcoded
+         * value.
+         * 
+         * <p>The input parameter must be the undecorated/transformed version of the header rather
+         * than the one actually populated by a web server's CGI interface, i.e., this lacks the
+         * HTTP_ prefix and punctuation conversion.</p>
          *
-         * @param rawname  raw name of header to clear
-         * @param cginame  CGI-equivalent name of header
+         * @param name  raw name of header to clear
          */
-        virtual void clearHeader(const char* rawname, const char* cginame)=0;
+        virtual void clearHeader(const char* name)=0;
 
         /**
          * Sets a value for a request header.
diff --git a/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp b/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
index 40b5cfda..0d4e5c2a 100644
--- a/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
+++ b/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
@@ -74,8 +74,7 @@ namespace {
         Category& m_log;
         ptree m_pt;
         bool m_urlEncoding,m_exportDuplicates;
-        // headers are tracked as Raw name, CGI name
-        map<string,pair<string,string>> m_mappings;
+        map<string,string> m_mappings;
         set<string> m_caseSensitiveIds;
     };
 
@@ -95,13 +94,10 @@ AttributeConfiguration::~AttributeConfiguration() {}
 DefaultAttributeConfiguration::DefaultAttributeConfiguration(const char* pathname)
     : m_log(Category::getInstance(SHIBSP_LOGCAT ".AttributeConfiguration")), m_urlEncoding(false), m_exportDuplicates(true)
 {
-
-    m_mappings["Shib-Application-ID"] = pair<string,string>("Shib-Application-ID", "HTTP_SHIB_APPLICATION_ID");
-    m_mappings["Shib-Session-ID"] = pair<string,string>("Shib-Session-ID", "HTTP_SHIB_SESSION_ID");
-    m_mappings["Shib-Session-Expires"] = pair<string,string>("Shib-Session-Expires", "HTTP_SHIB_SESSION_EXPIRES");
-    m_mappings["Shib-Session-Inactivity"] = pair<string,string>("Shib-Session-Inactivity", "HTTP_SHIB_SESSION_INACTIVITY");
-    m_mappings["Shib-Cookie-Name"] = pair<string,string>("Shib-Cookie-Name", "HTTP_SHIB_COOKIE_NAME");
-    m_mappings["REMOTE_USER"] = pair<string,string>("REMOTE_USER", "HTTP_REMOTE_USER");
+    // Populate "built-in" mappings.
+    for (const string& name : {"Shib-Application-ID", "Shib-Session-ID", "Shib-Session-Expires", "Shib-Session-Inactivity", "Shib-Cookie-Name", "REMOTE_USER"}) {
+        m_mappings[name] = name;
+    }
 
     if (!pathname) {
         return;
@@ -127,14 +123,7 @@ DefaultAttributeConfiguration::DefaultAttributeConfiguration(const char* pathnam
                 continue;
             }
 
-            string transformed("HTTP_");
-            const char* pch = alias.c_str();
-            while (*pch) {
-                transformed += (isalnum(*pch) ? toupper(*pch) : '_');
-                pch++;
-            }
-
-            m_mappings[child.first] = make_pair(alias, transformed);
+            m_mappings[child.first] = alias;
         }
     }
 }
@@ -230,7 +219,7 @@ void DefaultAttributeConfiguration::clearHeaders(SPRequest& request) const
 {
     if (request.isUseHeaders()) {
         for (const auto& names : m_mappings) {
-            request.clearHeader(names.second.first.c_str(), names.second.second.c_str());
+            request.clearHeader(names.second.c_str());
         }
     }
 }
@@ -254,7 +243,7 @@ void DefaultAttributeConfiguration::exportAttributes(SPRequest& request, const S
                 continue;
             }
 
-            string header(request.getSecureHeader(headerMapping->second.first.c_str()));
+            string header(request.getSecureHeader(headerMapping->second.c_str()));
 
             DDF vals = a.second; // cheap copy drops const qualifier
             DDF v = vals.first();
@@ -281,7 +270,7 @@ void DefaultAttributeConfiguration::exportAttributes(SPRequest& request, const S
 
                 v = vals.next();
             }
-            request.setHeader(headerMapping->second.first.c_str(), header.c_str());
+            request.setHeader(headerMapping->second.c_str(), header.c_str());
         }
     }
     else {

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


More information about the commits mailing list