Allow remoting of unsafe strings, and protect encoding of RelayState URLs.

diff --git a/adfs/adfs.cpp b/adfs/adfs.cpp
index 4795b4c..ffbb7f7 100644 (file)
--- a/adfs/adfs.cpp
+++ b/adfs/adfs.cpp
@@ -409,7 +409,7 @@ pair<bool,long> ADFSSessionInitiator::run(SPRequest& request, string& entityID,
     in.addmember("entity_id").string(entityID.c_str());
     in.addmember("acsLocation").string(ACSloc.c_str());
     if (!target.empty())
-        in.addmember("RelayState").string(target.c_str());
+        in.addmember("RelayState").unsafe_string(target.c_str());
     if (acClass.first)
         in.addmember("authnContextClassRef").string(acClass.second);
 
@@ -458,7 +458,7 @@ void ADFSSessionInitiator::receive(DDF& in, ostream& out)
     doRequest(*app, NULL, *http.get(), entityID, acsLocation, in["authnContextClassRef"].string(), relayState);
     if (!ret.isstruct())
         ret.structure();
-    ret.addmember("RelayState").string(relayState.c_str());
+    ret.addmember("RelayState").unsafe_string(relayState.c_str());
     out << ret;
 }
 

diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp
index 0fe6aeb..1045fc8 100644 (file)
--- a/shibsp/handler/impl/AbstractHandler.cpp
+++ b/shibsp/handler/impl/AbstractHandler.cpp
@@ -316,7 +316,7 @@ void AbstractHandler::preserveRelayState(const Application& application, HTTPRes
                 else if (SPConfig::getConfig().isEnabled(SPConfig::InProcess)) {
                     DDF out,in = DDF("set::RelayState").structure();
                     in.addmember("id").string(mech.second);
-                    in.addmember("value").string(relayState.c_str());
+                    in.addmember("value").unsafe_string(relayState.c_str());
                     DDFJanitor jin(in),jout(out);
                     out = application.getServiceProvider().getListenerService()->send(in);
                     if (!out.isstring())
@@ -624,7 +624,7 @@ DDF AbstractHandler::getPostData(const Application& application, const HTTPReque
             DDF ret = DDF("parameters").list();
             for (; params.first != params.second; ++params.first) {
                 if (!params.first->first.empty()) {
-                    child = DDF(params.first->first.c_str()).string(params.first->second);
+                    child = DDF(params.first->first.c_str()).unsafe_string(params.first->second);
                     ret.add(child);
                 }
             }

diff --git a/shibsp/handler/impl/RemotedHandler.cpp b/shibsp/handler/impl/RemotedHandler.cpp
index b8580e2..2ee35a1 100644 (file)
--- a/shibsp/handler/impl/RemotedHandler.cpp
+++ b/shibsp/handler/impl/RemotedHandler.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2007 Internet2
+ *  Copyright 2001-2009 Internet2
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -206,7 +206,7 @@ long RemotedResponse::sendRedirect(const char* url)
 {
     if (!m_output.isstruct())
         m_output.structure();
-    m_output.addmember("redirect").string(url);
+    m_output.addmember("redirect").unsafe_string(url);
     return HTTPResponse::XMLTOOLING_HTTP_STATUS_MOVED;
 }
 
@@ -240,7 +240,7 @@ DDF RemotedHandler::wrap(const SPRequest& request, const vector<string>* headers
     DDF in = DDF(m_address.c_str()).structure();
     in.addmember("application_id").string(request.getApplication().getId());
     in.addmember("scheme").string(request.getScheme());
-    in.addmember("hostname").string(request.getHostname());
+    in.addmember("hostname").unsafe_string(request.getHostname());
     in.addmember("port").integer(request.getPort());
     in.addmember("content_type").string(request.getContentType().c_str());
     in.addmember("content_length").integer(request.getContentLength());
@@ -248,8 +248,8 @@ DDF RemotedHandler::wrap(const SPRequest& request, const vector<string>* headers
     in.addmember("remote_user").string(request.getRemoteUser().c_str());
     in.addmember("client_addr").string(request.getRemoteAddr().c_str());
     in.addmember("method").string(request.getMethod());
-    in.addmember("uri").string(request.getRequestURI());
-    in.addmember("url").string(request.getRequestURL());
+    in.addmember("uri").unsafe_string(request.getRequestURI());
+    in.addmember("url").unsafe_string(request.getRequestURL());
     in.addmember("query").string(request.getQueryString());
 
     if (headers) {

diff --git a/shibsp/handler/impl/SAML2SessionInitiator.cpp b/shibsp/handler/impl/SAML2SessionInitiator.cpp
index 3bfcf36..452bb31 100644 (file)
--- a/shibsp/handler/impl/SAML2SessionInitiator.cpp
+++ b/shibsp/handler/impl/SAML2SessionInitiator.cpp
@@ -457,7 +457,7 @@ pair<bool,long> SAML2SessionInitiator::run(SPRequest& request, string& entityID,
             target = option;
     }
     if (!target.empty())
-        in.addmember("RelayState").string(target.c_str());
+        in.addmember("RelayState").unsafe_string(target.c_str());
 
     // Remote the processing.
     out = request.getServiceProvider().getListenerService()->send(in);
@@ -511,7 +511,7 @@ void SAML2SessionInitiator::receive(DDF& in, ostream& out)
         );
     if (!ret.isstruct())
         ret.structure();
-    ret.addmember("RelayState").string(relayState.c_str());
+    ret.addmember("RelayState").unsafe_string(relayState.c_str());
     out << ret;
 }
 

diff --git a/shibsp/handler/impl/Shib1SessionInitiator.cpp b/shibsp/handler/impl/Shib1SessionInitiator.cpp
index 417cbcd..5903e4b 100644 (file)
--- a/shibsp/handler/impl/Shib1SessionInitiator.cpp
+++ b/shibsp/handler/impl/Shib1SessionInitiator.cpp
@@ -188,7 +188,7 @@ pair<bool,long> Shib1SessionInitiator::run(SPRequest& request, string& entityID,
     if (artifactInbound)
         in.addmember("artifact").integer(1);
     if (!target.empty())
-        in.addmember("RelayState").string(target.c_str());
+        in.addmember("RelayState").unsafe_string(target.c_str());
 
     // Remote the processing. Our unwrap method will handle POST data if necessary.
     out = request.getServiceProvider().getListenerService()->send(in);
@@ -235,7 +235,7 @@ void Shib1SessionInitiator::receive(DDF& in, ostream& out)
     doRequest(*app, NULL, *http.get(), entityID, acsLocation, (in["artifact"].integer() != 0), relayState);
     if (!ret.isstruct())
         ret.structure();
-    ret.addmember("RelayState").string(relayState.c_str());
+    ret.addmember("RelayState").unsafe_string(relayState.c_str());
     out << ret;
 }
 

diff --git a/shibsp/impl/XMLServiceProvider.cpp b/shibsp/impl/XMLServiceProvider.cpp
index cc412c6..4e61127 100644 (file)
--- a/shibsp/impl/XMLServiceProvider.cpp
+++ b/shibsp/impl/XMLServiceProvider.cpp
@@ -1616,7 +1616,7 @@ void XMLConfig::receive(DDF& in, ostream& out)
         }
 
         // Repack for return to caller.
-        DDF ret=DDF(NULL).string(relayState.c_str());
+        DDF ret=DDF(NULL).unsafe_string(relayState.c_str());
         DDFJanitor jret(ret);
         out << ret;
     }

diff --git a/shibsp/remoting/ddf.h b/shibsp/remoting/ddf.h
index 041f344..b361ec0 100644 (file)
--- a/shibsp/remoting/ddf.h
+++ b/shibsp/remoting/ddf.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2007 Internet2
+ *  Copyright 2001-2009 Internet2
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -44,7 +44,7 @@ namespace shibsp {
         // constructors
         DDF() : m_handle(NULL) {}
         DDF(const char* n);
-        DDF(const char* n, const char* val);
+        DDF(const char* n, const char* val, bool safe=true);
         DDF(const char* n, long val);
         DDF(const char* n, double val);
         DDF(const char* n, void* val);
@@ -80,7 +80,10 @@ namespace shibsp {
         DDF& string(const char* val) {
             return string(const_cast<char*>(val), true);
         }
-        DDF& string(char* val, bool copyit=true);
+        DDF& unsafe_string(const char* val) {
+            return string(const_cast<char*>(val), true, false);
+        }
+        DDF& string(char* val, bool copyit=true, bool safe=true);
         DDF& string(long val);
         DDF& string(double val);
         DDF& integer(long val);

diff --git a/shibsp/remoting/impl/ddf.cpp b/shibsp/remoting/impl/ddf.cpp
index 463bb8c..c11a5e5 100644 (file)
--- a/shibsp/remoting/impl/ddf.cpp
+++ b/shibsp/remoting/impl/ddf.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2007 Internet2
+ *  Copyright 2001-2009 Internet2
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,6 +32,7 @@
 #include <xercesc/util/XMLUniDefs.hpp>
 #include <xmltooling/XMLToolingConfig.h>
 #include <xmltooling/util/ParserPool.h>
+#include <xmltooling/util/URLEncoder.h>
 #include <xmltooling/util/XMLHelper.h>
 
 using namespace shibsp;
@@ -101,7 +102,8 @@ struct shibsp::ddf_body_t {
         DDF_FLOAT,
            DDF_STRUCT,
         DDF_LIST,
-           DDF_POINTER
+           DDF_POINTER,
+        DDF_STRING_UNSAFE
     } type;                         // data type of node
 
     union {
@@ -126,11 +128,11 @@ DDF::DDF(const char* n)
     name(n);
 }
 
-DDF::DDF(const char* n, const char* val)
+DDF::DDF(const char* n, const char* val, bool safe)
 {
     m_handle=new(nothrow) ddf_body_t;
     name(n);
-    string(val);
+    string(const_cast<char*>(val), true, safe);
 }
 
 DDF::DDF(const char* n, long val)
@@ -171,7 +173,8 @@ DDF DDF::copy() const
         case ddf_body_t::DDF_EMPTY:
             return DDF(m_handle->name);
         case ddf_body_t::DDF_STRING:
-            return DDF(m_handle->name,m_handle->value.string);
+        case ddf_body_t::DDF_STRING_UNSAFE:
+            return DDF(m_handle->name,m_handle->value.string,(m_handle->type==ddf_body_t::DDF_STRING));
         case ddf_body_t::DDF_INT:
             return DDF(m_handle->name,m_handle->value.integer);
         case ddf_body_t::DDF_FLOAT:
@@ -240,7 +243,7 @@ bool DDF::isempty() const
 
 bool DDF::isstring() const
 {
-    return m_handle ? (m_handle->type==ddf_body_t::DDF_STRING) : false;
+    return m_handle ? (m_handle->type==ddf_body_t::DDF_STRING || m_handle->type==ddf_body_t::DDF_STRING_UNSAFE) : false;
 }
 
 bool DDF::isint() const
@@ -351,13 +354,13 @@ DDF& DDF::empty()
     return *this;
 }
 
-DDF& DDF::string(char* val, bool copyit)
+DDF& DDF::string(char* val, bool copyit, bool safe)
 {
     if (empty().m_handle) {
         m_handle->value.string = copyit ? ddf_strdup(val) : val;
         if (!m_handle->value.string && val && *val)
             return destroy();
-        m_handle->type=ddf_body_t::DDF_STRING;
+        m_handle->type=(safe ? ddf_body_t::DDF_STRING : ddf_body_t::DDF_STRING_UNSAFE);
     }
     return *this;
 }
@@ -680,6 +683,7 @@ void DDF::dump(FILE* f, int indent) const
                 break;
 
             case ddf_body_t::DDF_STRING:
+            case ddf_body_t::DDF_STRING_UNSAFE:
                 if (m_handle->name)
                     fprintf(f,"char* %s = ",m_handle->name);
                 else
@@ -798,6 +802,7 @@ void serialize(ddf_body_t* p, ostream& os, bool name_attr=true)
         switch (p->type) {
             
             case ddf_body_t::DDF_STRING:
+            case ddf_body_t::DDF_STRING_UNSAFE:
                 os << "<string";
                 if (name_attr && p->name) {
                     os << " name=\"";
@@ -805,8 +810,14 @@ void serialize(ddf_body_t* p, ostream& os, bool name_attr=true)
                     os << '"';
                 }
                 if (p->value.string) {
-                    os << '>';
-                    xml_encode(os,p->value.string);
+                    if (p->type == ddf_body_t::DDF_STRING) {
+                        os << '>';
+                        xml_encode(os,p->value.string);
+                    }
+                    else {
+                        os << " unsafe=\"1\">";
+                        xml_encode(os,XMLToolingConfig::getConfig().getURLEncoder()->encode(p->value.string).c_str());
+                    }
                     os << "</string>";
                 }
                 else
@@ -941,11 +952,12 @@ static const XMLCh _number[] =  UNICODE_LITERAL_6(n,u,m,b,e,r);
 static const XMLCh _array[] =   UNICODE_LITERAL_5(a,r,r,a,y);
 static const XMLCh _struct[] =  UNICODE_LITERAL_6(s,t,r,u,c,t);
 static const XMLCh _lowercase[] = UNICODE_LITERAL_9(l,o,w,e,r,c,a,s,e);
+static const XMLCh _unsafe[] =  UNICODE_LITERAL_6(u,n,s,a,f,e);
 
 DDF deserialize(DOMElement* root, bool lowercase)
 {
     DDF obj(NULL);
-    auto_ptr_char name_val(root->getAttribute(_name));
+    auto_ptr_char name_val(root->getAttributeNS(NULL, _name));
     if (name_val.get() && *name_val.get()) {
         if (lowercase)
             for (char* pch=const_cast<char*>(name_val.get()); *pch=tolower(*pch); pch++);
@@ -961,9 +973,19 @@ DDF deserialize(DOMElement* root, bool lowercase)
     if (XMLString::equals(tag,_string)) {
         DOMNode* child=root->getFirstChild();
         if (child && child->getNodeType()==DOMNode::TEXT_NODE) {
-            char* val = toUTF8(child->getNodeValue(), true);    // use malloc
-            if (val)
-                obj.string(val, false); // don't re-copy the string
+            const XMLCh* unsafe = root->getAttributeNS(NULL, _unsafe);
+            if (unsafe && *unsafe==chDigit_1) {
+                // If it's unsafe, it's not UTF-8 data, so we have to convert to ASCII and decode it.
+                char* encoded = XMLString::transcode(child->getNodeValue());
+                XMLToolingConfig::getConfig().getURLEncoder()->decode(encoded);
+                obj.string(encoded, true, false); // re-copy into free-able buffer, plus mark unsafe
+                XMLString::release(&encoded);
+            }
+            else {
+                char* val = toUTF8(child->getNodeValue(), true);    // use malloc
+                if (val)
+                    obj.string(val, false); // don't re-copy the string
+            }
         }
     }
     else if (XMLString::equals(tag,_number)) {