Security update for shibboleth-sp in lenny
Russ Allbery
rra at debian.org
Wed Dec 2 02:12:16 UTC 2009
I'm very sorry about how long it's taken me to prepare these patches.
This should address CVE 2009-3300 in the shibboleth-sp (not
shibboleth-sp2) packages in Debian lenny. I will also work on a backport
of these patches to the version that released with Debian etch.
Note that the upstream source contains Windows line endings in some
places, which my mailer doesn't want to send without encoding, so this
patch may require the ignore whitespace flag to apply as-is.
Please let me know if these are good for upload to the stable-security
queue.
diff -u shibboleth-sp-1.3.1.dfsg1/debian/changelog shibboleth-sp-1.3.1.dfsg1/debian/changelog
--- shibboleth-sp-1.3.1.dfsg1/debian/changelog
+++ shibboleth-sp-1.3.1.dfsg1/debian/changelog
@@ -1,3 +1,14 @@
+shibboleth-sp (1.3.1.dfsg1-3+lenny2) stable-security; urgency=high
+
+ * SECURITY: Fix improper handling of URLs that could be abused for
+ script injection and other cross-site scripting attacks.
+ (CVE-2009-3300)
+ * Fix build dependency to force libxml-security-c-dev 1.3 or later.
+ This is not strictly required for lenny since lenny shipped with 1.4,
+ but helps backports to etch.
+
+ -- Russ Allbery <rra at debian.org> Thu, 12 Nov 2009 11:17:33 -0800
+
shibboleth-sp (1.3.1.dfsg1-3+lenny1) stable-security; urgency=high
* SECURITY: Correctly handle decoding of malformed URLs, closing a
diff -u shibboleth-sp-1.3.1.dfsg1/debian/control shibboleth-sp-1.3.1.dfsg1/debian/control
--- shibboleth-sp-1.3.1.dfsg1/debian/control
+++ shibboleth-sp-1.3.1.dfsg1/debian/control
@@ -5,7 +5,7 @@
Uploaders: Russ Allbery <rra at debian.org>
Build-Depends: debhelper (>= 5), autotools-dev, autoconf, automake,
libtool, apache2-threaded-dev (>= 2.2), libsaml-dev, liblog4cpp5-dev,
- libxerces-c2-dev, libxml-security-c-dev, perl, libwrap0-dev,
+ libxerces-c2-dev, libxml-security-c-dev (>= 1.3), perl, libwrap0-dev,
libmysqlclient15-dev
Build-Conflicts: autoconf2.13, automake1.4
Standards-Version: 3.8.0
diff -u shibboleth-sp-1.3.1.dfsg1/shib-target/shib-handlers.cpp shibboleth-sp-1.3.1.dfsg1/shib-target/shib-handlers.cpp
--- shibboleth-sp-1.3.1.dfsg1/shib-target/shib-handlers.cpp
+++ shibboleth-sp-1.3.1.dfsg1/shib-target/shib-handlers.cpp
@@ -316,7 +316,7 @@
if (target=="default") {
pair<bool,const char*> homeURL=app->getString("homeURL");
- target=homeURL.first ? homeURL.second : "/";
+ target=homeURL.first ? homeURL.second : "";
}
else if (target=="cookie" || target.empty()) {
// Pull the target value from the "relay state" cookie.
@@ -325,7 +325,7 @@
if (!relay_state || !*relay_state) {
// No apparent relay state value to use, so fall back on the default.
pair<bool,const char*> homeURL=app->getString("homeURL");
- target=homeURL.first ? homeURL.second : "/";
+ target=homeURL.first ? homeURL.second : "";
}
else {
char* rscopy=strdup(relay_state);
@@ -366,6 +366,19 @@
}
}
+ if (target == "") {
+ // No homeURL, so compute a URL to the root of the site.
+ int port = st->getPort();
+ const char* scheme = st->getProtocol();
+ target = string(scheme) + "://" + st->getHostname();
+ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+ ostringstream portstr;
+ portstr << port;
+ target += ':' + portstr.str();
+ }
+ target += '/';
+ }
+
// Now redirect to the target.
return make_pair(true, st->sendRedirect(target));
}
@@ -401,8 +414,19 @@
ret=handler->getString("ResponseLocation").second;
if (!ret)
ret=st->getApplication()->getString("homeURL").second;
- if (!ret)
- ret="/";
+ if (!ret) {
+ // No homeURL, so compute a URL to the root of the site.
+ int port = st->getPort();
+ const char* scheme = st->getProtocol();
+ string dest = string(scheme) + "://" + st->getHostname();
+ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+ ostringstream portstr;
+ portstr << port;
+ dest += ':' + portstr.str();
+ }
+ dest += '/';
+ return make_pair(true, st->sendRedirect(dest));
+ }
return make_pair(true, st->sendRedirect(ret));
}
diff -u shibboleth-sp-1.3.1.dfsg1/apache/mod_apache.cpp shibboleth-sp-1.3.1.dfsg1/apache/mod_apache.cpp
--- shibboleth-sp-1.3.1.dfsg1/apache/mod_apache.cpp
+++ shibboleth-sp-1.3.1.dfsg1/apache/mod_apache.cpp
@@ -69,6 +69,7 @@
char* g_szSHIBConfig = NULL;
char* g_szSchemaDir = NULL;
ShibTargetConfig* g_Config = NULL;
+ set<string> g_allowedSchemes;
string g_unsetHeaderValue;
bool g_checkSpoofing = true;
bool g_catchAll = true;
@@ -443,9 +444,12 @@
const string& content_type="text/html",
const Iterator<header_t>& headers=EMPTY(header_t)
) {
+ checkString(content_type, "Detected control character in a response header.");
m_req->content_type = ap_psprintf(m_req->pool, content_type.c_str());
while (headers.hasNext()) {
const header_t& h=headers.next();
+ checkString(h.first, "Detected control character in a response header.");
+ checkString(h.second, "Detected control character in a response header.");
ap_table_set(m_req->headers_out, h.first.c_str(), h.second.c_str());
}
ap_send_http_header(m_req);
@@ -453,6 +457,9 @@
return (void*)((code==200) ? DONE : code);
}
virtual void* sendRedirect(const string& url) {
+ checkString(url, "Detected control character in an attempted redirect.");
+ if (g_allowedSchemes.find(url.substr(0, url.find(':'))) == g_allowedSchemes.end())
+ throw FatalProfileException("Invalid scheme in attempted redirect.");
ap_table_set(m_req->headers_out, "Location", url.c_str());
return (void*)REDIRECT;
}
@@ -465,6 +472,15 @@
shib_server_config* m_sc;
shib_request_config* m_rc;
set<string> m_allhttp;
+
+private:
+ void checkString(const string& s, const char* msg) {
+ string::const_iterator e = s.end();
+ for (string::const_iterator i=s.begin(); i!=e; ++i) {
+ if (iscntrl(*i))
+ throw FatalProfileException(msg);
+ }
+ }
};
/********************************************************************************/
@@ -1122,14 +1138,30 @@
saml::Locker locker(conf);
const IPropertySet* props=conf->getPropertySet("Local");
if (props) {
- pair<bool,const char*> unsetValue=props->getString("unsetHeaderValue");
- if (unsetValue.first)
- g_unsetHeaderValue = unsetValue.second;
+ pair<bool,const char*> str=props->getString("unsetHeaderValue");
+ if (str.first)
+ g_unsetHeaderValue = str.second;
+ str=props->getString("allowedSchemes");
+ if (str.first) {
+ string schemes=str.second;
+ unsigned int j=0;
+ for (unsigned int i=0; i < schemes.length(); i++) {
+ if (schemes.at(i)==' ') {
+ g_allowedSchemes.insert(schemes.substr(j, i-j));
+ j = i+1;
+ }
+ }
+ g_allowedSchemes.insert(schemes.substr(j, schemes.length()-j));
+ }
pair<bool,bool> flag=props->getBool("checkSpoofing");
g_checkSpoofing = !flag.first || flag.second;
flag=props->getBool("catchAll");
g_catchAll = !flag.first || flag.second;
}
+ if (g_allowedSchemes.empty()) {
+ g_allowedSchemes.insert("https");
+ g_allowedSchemes.insert("http");
+ }
}
catch (exception&) {
ap_log_error(APLOG_MARK,APLOG_CRIT|APLOG_NOERRNO,SH_AP_R(s),"shib_child_init() failed to initialize system");
only in patch2:
unchanged:
--- shibboleth-sp-1.3.1.dfsg1.orig/adfs/handlers.cpp
+++ shibboleth-sp-1.3.1.dfsg1/adfs/handlers.cpp
@@ -429,8 +429,19 @@
ret=handler->getString("ResponseLocation").second;
if (!ret)
ret=st->getApplication()->getString("homeURL").second;
- if (!ret)
- ret="/";
+ if (!ret) {
+ // No homeURL, so compute a URL to the root of the site.
+ int port = st->getPort();
+ const char* scheme = st->getProtocol();
+ string dest = string(scheme) + "://" + st->getHostname();
+ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+ ostringstream portstr;
+ portstr << port;
+ dest += ':' + portstr.str();
+ }
+ dest += '/';
+ return make_pair(true, st->sendRedirect(dest));
+ }
return make_pair(true, st->sendRedirect(ret));
}
@@ -469,7 +480,7 @@
if (target=="default") {
pair<bool,const char*> homeURL=app->getString("homeURL");
- target=homeURL.first ? homeURL.second : "/";
+ target=homeURL.first ? homeURL.second : "";
}
else if (target=="cookie" || target.empty()) {
// Pull the target value from the "relay state" cookie.
@@ -478,7 +489,7 @@
if (!relay_state || !*relay_state) {
// No apparent relay state value to use, so fall back on the default.
pair<bool,const char*> homeURL=app->getString("homeURL");
- target=homeURL.first ? homeURL.second : "/";
+ target=homeURL.first ? homeURL.second : "";
}
else {
char* rscopy=strdup(relay_state);
@@ -519,6 +530,19 @@
}
}
+ if (target == "") {
+ // No homeURL, so compute a URL to the root of the site.
+ int port = st->getPort();
+ const char* scheme = st->getProtocol();
+ target = string(scheme) + "://" + st->getHostname();
+ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+ ostringstream portstr;
+ portstr << port;
+ target += ':' + portstr.str();
+ }
+ target += '/';
+ }
+
// Now redirect to the target.
return make_pair(true, st->sendRedirect(target));
}
only in patch2:
unchanged:
--- shibboleth-sp-1.3.1.dfsg1.orig/fastcgi/shibauthorizer.cpp
+++ shibboleth-sp-1.3.1.dfsg1/fastcgi/shibauthorizer.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 <fcgio.h>
using namespace shibtarget;
+using namespace saml;
using namespace std;
typedef enum {
@@ -40,10 +41,21 @@
SHIB_RETURN_DONE
} shib_return_t;
+set<string> g_allowedSchemes;
+
class ShibTargetFCGIAuth : public ShibTarget
{
FCGX_Request* m_req;
string m_cookie;
+
+ void checkString(const string& s, const char* msg) {
+ string::const_iterator e = s.end();
+ for (string::const_iterator i=s.begin(); i!=e; ++i) {
+ if (iscntrl(*i))
+ throw runtime_error(msg);
+ }
+ }
+
public:
map<string,string> m_headers;
@@ -143,9 +155,12 @@
const string& content_type="text/html",
const saml::Iterator<header_t>& headers=EMPTY(header_t)) {
+ checkString(content_type, "Detected control character in a response header.");
string hdr = m_cookie + "Connection: close\r\nContent-type: " + content_type + "\r\n";
while (headers.hasNext()) {
const header_t& h=headers.next();
+ checkString(h.first, "Detected control character in a response header.");
+ checkString(h.second, "Detected control character in a response header.");
hdr += h.first + ": " + h.second + "\r\n";
}
@@ -162,6 +177,9 @@
}
virtual void* sendRedirect(const string& url) {
+ checkString(url, "Detected control character in an attempted redirect.");
+ if (g_allowedSchemes.find(url.substr(0, url.find(':'))) == g_allowedSchemes.end())
+ throw runtime_error("Invalid scheme in attempted redirect.");
cout << "Status: 302 Please Wait" << "\r\n"
<< "Location: " << url << "\r\n"
<< m_cookie << "\r\n"
@@ -227,12 +245,36 @@
cerr << "failed to load Shibboleth configuration" << endl;
exit(1);
}
+
+ IConfig* conf=g_Config->getINI();
+ Locker locker(conf);
+ const IPropertySet* props=conf->getPropertySet("Local");
+ if (props) {
+ pair<bool,const char*> str=props->getString("allowedSchemes");
+ if (str.first) {
+ string schemes=str.second;
+ unsigned int j=0;
+ for (unsigned int i=0; i < schemes.length(); i++) {
+ if (schemes.at(i)==' ') {
+ g_allowedSchemes.insert(schemes.substr(j, i-j));
+ j = i+1;
+ }
+ }
+ g_allowedSchemes.insert(schemes.substr(j, schemes.length()-j));
+ }
+ }
+ if (g_allowedSchemes.empty()) {
+ g_allowedSchemes.insert("https");
+ g_allowedSchemes.insert("http");
+ }
}
catch (exception& e) {
cerr << "exception while initializing Shibboleth configuration: " << e.what() << endl;
exit(1);
}
+
+
// Load "authoritative" URL fields.
char* var = getenv("SHIBSP_SERVER_NAME");
if (var)
only in patch2:
unchanged:
--- shibboleth-sp-1.3.1.dfsg1.orig/fastcgi/shibresponder.cpp
+++ shibboleth-sp-1.3.1.dfsg1/fastcgi/shibresponder.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 <fcgio.h>
using namespace shibtarget;
+using namespace saml;
using namespace std;
typedef enum {
@@ -40,6 +41,8 @@
SHIB_RETURN_DONE
} shib_return_t;
+set<string> g_allowedSchemes;
+
class ShibTargetFCGI : public ShibTarget
{
FCGX_Request* m_req;
@@ -47,6 +50,14 @@
string m_cookie;
map<string, string> m_headers;
+ void checkString(const string& s, const char* msg) {
+ string::const_iterator e = s.end();
+ for (string::const_iterator i=s.begin(); i!=e; ++i) {
+ if (iscntrl(*i))
+ throw runtime_error(msg);
+ }
+ }
+
public:
ShibTargetFCGI(FCGX_Request* req, char* post_data, const char* scheme=NULL, const char* hostname=NULL, int port=0)
: m_req(req), m_body(post_data) {
@@ -146,9 +157,12 @@
const string& content_type="text/html",
const saml::Iterator<header_t>& headers=EMPTY(header_t)) {
+ checkString(content_type, "Detected control character in a response header.");
string hdr = string ("Connection: close\r\nContent-type: ") + content_type + "\r\n" + m_cookie;
while (headers.hasNext()) {
const header_t& h=headers.next();
+ checkString(h.first, "Detected control character in a response header.");
+ checkString(h.second, "Detected control character in a response header.");
hdr += h.first + ": " + h.second + "\r\n";
}
@@ -164,6 +178,9 @@
}
virtual void* sendRedirect(const string& url) {
+ checkString(url, "Detected control character in an attempted redirect.");
+ if (g_allowedSchemes.find(url.substr(0, url.find(':'))) == g_allowedSchemes.end())
+ throw runtime_error("Invalid scheme in attempted redirect.");
cout << "Status: 302 Please Wait" << "\r\n" << "Location: " << url << "\r\n" << m_cookie << "\r\n"
<< "<HTML><BODY>Redirecting...</BODY></HTML>";
return (void*)SHIB_RETURN_DONE;
@@ -260,6 +277,29 @@
cerr << "failed to load Shibboleth configuration" << endl;
exit(1);
}
+
+ IConfig* conf=g_Config->getINI();
+ Locker locker(conf);
+ const IPropertySet* props=conf->getPropertySet("Local");
+ if (props) {
+ pair<bool,const char*> str=props->getString("allowedSchemes");
+ if (str.first) {
+ string schemes=str.second;
+ unsigned int j=0;
+ for (unsigned int i=0; i < schemes.length(); i++) {
+ if (schemes.at(i)==' ') {
+ g_allowedSchemes.insert(schemes.substr(j, i-j));
+ j = i+1;
+ }
+ }
+ g_allowedSchemes.insert(schemes.substr(j, schemes.length()-j));
+ }
+ }
+ if (g_allowedSchemes.empty()) {
+ g_allowedSchemes.insert("https");
+ g_allowedSchemes.insert("http");
+ }
+
}
catch (exception& e) {
cerr << "exception while initializing Shibboleth configuration:" << e.what() << endl;
only in patch2:
unchanged:
--- shibboleth-sp-1.3.1.dfsg1.orig/schemas/shibboleth-targetconfig-1.0.xsd
+++ shibboleth-sp-1.3.1.dfsg1/schemas/shibboleth-targetconfig-1.0.xsd
@@ -191,6 +191,7 @@
<attribute name="unsetHeaderValue" type="conf:string" use="optional"/>
<attribute name="checkSpoofing" type="boolean" use="optional"/>
<attribute name="catchAll" type="boolean" use="optional"/>
+ <attribute name="allowedSchemes" type="conf:listOfStrings"/>
<anyAttribute namespace="##other" processContents="lax"/>
</complexType>
--
Russ Allbery (rra at debian.org) <http://www.eyrie.org/~eagle/>
More information about the Pkg-shibboleth-devel
mailing list