Dear Security Team, Please review the following source debdiff:
diff -Nru opensaml-3.2.1/debian/changelog opensaml-3.2.1/debian/changelog --- opensaml-3.2.1/debian/changelog 2023-01-18 19:21:05.000000000 +0100 +++ opensaml-3.2.1/debian/changelog 2025-03-14 21:47:50.000000000 +0100 @@ -1,3 +1,45 @@ +opensaml (3.2.1-3+deb12u1) bookworm-security; urgency=high + + * [b3e86fd] New patch: CPPOST-126 - Simple signature verification fails to + detect parameter smuggling. + Security fix cherry-picked from v3.3.1 (upstream commit + 22a610b322e2178abd03e97cdbc8fb50b45efaee). + Parameter manipulation allows the forging of signed SAML messages + ================================================================= + A number of vulnerabilities in the OpenSAML library used by the + Shibboleth Service Provider allowed for creative manipulation of + parameters combined with reuse of the contents of older requests + to fool the library's signature verification of non-XML based + signed messages. + Most uses of that feature involve very low or low impact use cases + without critical security implications; however, there are two + scenarios that are much more critical, one affecting the SP and + one affecting some implementers who have implemented their own + code on top of our OpenSAML library and done so improperly. + The SP's support for the HTTP-POST-SimpleSign SAML binding for + Single Sign-On responses is its critical vulnerability, and + it is enabled by default (regardless of what one's published + SAML metadata may advertise). + The other critical case involves a mistake that does *not* + impact the Shibboleth SP, allowing SSO to occur over the + HTTP-Redirect binding contrary to the plain language of the + SAML Browser SSO profile. The SP does not support this, but + other implementers may have done so. + Contrary to the initial publication of this advisory, there is no + workaround within the SP configuration other than to remove the + "SimpleSigning" security policy rule from the security-policy.xml + file entirely. + That will also prevent support of legitimate signed requests or + responses via the HTTP-Redirect binding, which is generally used + only for logout messages within the SP itself. Removing support + for that binding in favor of HTTP-POST in any published metadata + is an option of course. + Full advisory: + https://shibboleth.net/community/advisories/secadv_20250313.txt + Thanks to Scott Cantor (Closes: #1100464) + + -- Ferenc Wágner <wf...@debian.org> Fri, 14 Mar 2025 21:47:50 +0100 + opensaml (3.2.1-3) unstable; urgency=medium * [02e846c] Update Standards-Version to 4.6.2 (no changes required) diff -Nru opensaml-3.2.1/debian/gbp.conf opensaml-3.2.1/debian/gbp.conf --- opensaml-3.2.1/debian/gbp.conf 2023-01-18 19:04:34.000000000 +0100 +++ opensaml-3.2.1/debian/gbp.conf 2025-03-14 21:47:14.000000000 +0100 @@ -1,5 +1,5 @@ [DEFAULT] -debian-branch = debian/master +debian-branch = debian/bookworm upstream-branch = upstream/latest pristine-tar = True diff -Nru opensaml-3.2.1/debian/patches/CPPOST-126-Simple-signature-verification-fails-to-detect-.patch opensaml-3.2.1/debian/patches/CPPOST-126-Simple-signature-verification-fails-to-detect-.patch --- opensaml-3.2.1/debian/patches/CPPOST-126-Simple-signature-verification-fails-to-detect-.patch 1970-01-01 01:00:00.000000000 +0100 +++ opensaml-3.2.1/debian/patches/CPPOST-126-Simple-signature-verification-fails-to-detect-.patch 2025-03-14 21:47:33.000000000 +0100 @@ -0,0 +1,339 @@ +From: Scott Cantor <canto...@osu.edu> +Date: Thu, 13 Mar 2025 14:14:12 -0400 +Subject: CPPOST-126 - Simple signature verification fails to detect parameter + smuggling + +https://shibboleth.atlassian.net/browse/CPPOST-126 +--- + saml/binding/impl/SimpleSigningRule.cpp | 88 +++++++++++++++++------- + saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp | 2 + + saml/saml2/binding/impl/SAML2ECPDecoder.cpp | 3 +- + saml/saml2/binding/impl/SAML2POSTDecoder.cpp | 31 ++++++--- + saml/saml2/binding/impl/SAML2RedirectDecoder.cpp | 32 ++++++--- + saml/saml2/binding/impl/SAML2SOAPDecoder.cpp | 2 +- + 6 files changed, 110 insertions(+), 48 deletions(-) + +diff --git a/saml/binding/impl/SimpleSigningRule.cpp b/saml/binding/impl/SimpleSigningRule.cpp +index 24e95a6..9c2122e 100644 +--- a/saml/binding/impl/SimpleSigningRule.cpp ++++ b/saml/binding/impl/SimpleSigningRule.cpp +@@ -29,6 +29,7 @@ + #include "binding/SecurityPolicy.h" + #include "binding/SecurityPolicyRule.h" + #include "saml2/core/Assertions.h" ++#include "saml2/core/Protocols.h" + #include "saml2/metadata/Metadata.h" + #include "saml2/metadata/MetadataCredentialCriteria.h" + #include "saml2/metadata/MetadataProvider.h" +@@ -41,6 +42,7 @@ + #include <xmltooling/signature/KeyInfo.h> + #include <xmltooling/signature/Signature.h> + #include <xmltooling/util/ParserPool.h> ++#include <xmltooling/util/URLEncoder.h> + + using namespace opensaml::saml2md; + using namespace opensaml; +@@ -66,7 +68,8 @@ namespace opensaml { + + private: + // Appends a raw parameter=value pair to the string. +- static bool appendParameter(string& s, const char* data, const char* name); ++ static bool appendParameter(const GenericRequest& request, string& s, const char* data, const char* name); ++ static const char* getMessageParameterName(const XMLObject* message); + + bool m_errorFatal; + }; +@@ -79,21 +82,48 @@ namespace opensaml { + static const XMLCh errorFatal[] = UNICODE_LITERAL_10(e,r,r,o,r,F,a,t,a,l); + }; + +-bool SimpleSigningRule::appendParameter(string& s, const char* data, const char* name) ++bool SimpleSigningRule::appendParameter(const GenericRequest& request, string& s, const char* data, const char* name) + { +- const char* start = strstr(data,name); ++ // Make sure only a single instance of the parameter specified is found in the decoded query. ++ vector<const char*> valueHolder; ++ if (request.getParameters(name, valueHolder) > 1) { ++ throw SecurityPolicyException("Multiple $1 parameters present.", params(1, name)); ++ } ++ ++ string param_name(name); ++ param_name += '='; ++ ++ const char* start = strstr(data, param_name.c_str()); + if (!start) + return false; ++ if (start > data && *(start - 1) != '&') ++ throw SecurityPolicyException("Detected attempt to smuggle a prefixed $1 parameter.", params(1, name)); ++ + if (!s.empty()) + s += '&'; +- const char* end = strchr(start,'&'); ++ ++ const char* end = strchr(start, '&'); + if (end) +- s.append(start, end-start); ++ s.append(start, end - start); + else + s.append(start); ++ + return true; + } + ++const char* SimpleSigningRule::getMessageParameterName(const XMLObject* message) ++{ ++ if (dynamic_cast<const saml2p::StatusResponseType*>(message)) { ++ return "SAMLResponse"; ++ } ++ else if (dynamic_cast<const saml2p::RequestAbstractType*>(message)) { ++ return "SAMLRequest"; ++ } ++ else { ++ return nullptr; ++ } ++} ++ + SimpleSigningRule::SimpleSigningRule(const DOMElement* e) + : SecurityPolicyRule(e), m_errorFatal(XMLHelper::getAttrBool(e, false, errorFatal)) + { +@@ -119,34 +149,50 @@ bool SimpleSigningRule::evaluate(const XMLObject& message, const GenericRequest* + } + + const HTTPRequest* httpRequest = dynamic_cast<const HTTPRequest*>(request); +- if (!request || !httpRequest) ++ if (!request || !httpRequest) { + return false; ++ } + +- const char* signature = request->getParameter("Signature"); +- if (!signature) ++ // Make sure Signature only shows up once. ++ vector<const char*> valueHolder; ++ request->getParameters("Signature", valueHolder); ++ if (valueHolder.empty()) { + return false; +- ++ } ++ else if (valueHolder.size() > 1) { ++ throw SecurityPolicyException("Multiple Signature parameters present."); ++ } ++ const char* signature = valueHolder[0]; ++ ++ // The multiple parameter copy check for the GET case is applied down below in appendParameter. + const char* sigAlgorithm = request->getParameter("SigAlg"); + if (!sigAlgorithm) { + log.warn("SigAlg parameter not found, no way to verify the signature"); + return false; + } + ++ const char* messageParameterName = getMessageParameterName(&message); ++ if (!messageParameterName) { ++ log.debug("ignoring unrecognized message type"); ++ return false; ++ } ++ + string input; + const char* pch; + if (!strcmp(httpRequest->getMethod(), "GET")) { + // We have to construct a string containing the signature input by accessing the + // request directly. We can't use the decoded parameters because we need the raw +- // data and URL-encoding isn't canonical. ++ // data and URL-encoding isn't canonical. We have to ensure only one copy a given ++ // parameter appears in the string in its decoded form, to ensure that other layers ++ // of the code only saw/see the same value we see here. + + // NOTE: SimpleSign for GET means Redirect binding, which means we verify over the + // base64-encoded message directly. + + pch = httpRequest->getQueryString(); +- if (!appendParameter(input, pch, "SAMLRequest=")) +- appendParameter(input, pch, "SAMLResponse="); +- appendParameter(input, pch, "RelayState="); +- appendParameter(input, pch, "SigAlg="); ++ appendParameter(*request, input, pch, messageParameterName); ++ appendParameter(*request, input, pch, "RelayState"); ++ appendParameter(*request, input, pch, "SigAlg"); + } + else { + // With POST, the input string is concatenated from the decoded form controls. +@@ -158,24 +204,14 @@ bool SimpleSigningRule::evaluate(const XMLObject& message, const GenericRequest* + // why XMLSignature exists, and why this isn't really "simpler"). + + XMLSize_t x; +- pch = httpRequest->getParameter("SAMLRequest"); ++ pch = httpRequest->getParameter(messageParameterName); + if (pch) { + XMLByte* decoded=Base64::decode(reinterpret_cast<const XMLByte*>(pch),&x); + if (!decoded) { + log.warn("unable to decode base64 in POST binding message"); + return false; + } +- input = string("SAMLRequest=") + reinterpret_cast<const char*>(decoded); +- XMLString::release((char**)&decoded); +- } +- else { +- pch = httpRequest->getParameter("SAMLResponse"); +- XMLByte* decoded=Base64::decode(reinterpret_cast<const XMLByte*>(pch),&x); +- if (!decoded) { +- log.warn("unable to decode base64 in POST binding message"); +- return false; +- } +- input = string("SAMLResponse=") + reinterpret_cast<const char*>(decoded); ++ input = string(messageParameterName) + "=" + reinterpret_cast<const char*>(decoded); + XMLString::release((char**)&decoded); + } + +diff --git a/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp b/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp +index 724c01e..9378942 100644 +--- a/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp ++++ b/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp +@@ -95,6 +95,8 @@ XMLObject* SAML2ArtifactDecoder::decode( + const char* state = httpRequest->getParameter("RelayState"); + if (state) + relayState = state; ++ if (httpRequest->getParameter("Signature")) ++ throw BindingException("Request contained unexpected Signature parameter."); + + if (!m_artifactResolver || !policy.getMetadataProvider() || !policy.getRole()) + throw BindingException("Artifact binding requires ArtifactResolver and MetadataProvider implementations be supplied."); +diff --git a/saml/saml2/binding/impl/SAML2ECPDecoder.cpp b/saml/saml2/binding/impl/SAML2ECPDecoder.cpp +index 610ac2f..3d1c180 100644 +--- a/saml/saml2/binding/impl/SAML2ECPDecoder.cpp ++++ b/saml/saml2/binding/impl/SAML2ECPDecoder.cpp +@@ -86,7 +86,8 @@ XMLObject* SAML2ECPDecoder::decode( + const HTTPRequest* httpRequest = dynamic_cast<const HTTPRequest*>(&genericRequest); + if (httpRequest) { + string s = httpRequest->getContentType(); +- if (s.find("application/vnd.paos+xml") == string::npos) { ++ if (s.find("application/vnd.paos+xml") == string::npos || ++ s.find("application/x-www-form-urlencoded") != string::npos) { + log.warn("ignoring incorrect content type (%s)", s.c_str() ? s.c_str() : "none"); + throw BindingException("Invalid content type for PAOS message."); + } +diff --git a/saml/saml2/binding/impl/SAML2POSTDecoder.cpp b/saml/saml2/binding/impl/SAML2POSTDecoder.cpp +index 534b8ff..e534944 100644 +--- a/saml/saml2/binding/impl/SAML2POSTDecoder.cpp ++++ b/saml/saml2/binding/impl/SAML2POSTDecoder.cpp +@@ -92,11 +92,18 @@ XMLObject* SAML2POSTDecoder::decode( + throw BindingException("Unable to cast request object to HTTPRequest type."); + if (strcmp(httpRequest->getMethod(),"POST")) + throw BindingException("Invalid HTTP method ($1).", params(1, httpRequest->getMethod())); +- const char* msg = httpRequest->getParameter("SAMLResponse"); +- if (!msg) +- msg = httpRequest->getParameter("SAMLRequest"); ++ ++ bool isRequest = false; ++ const char* msg = httpRequest->getParameter("SAMLRequest"); ++ if (msg) { ++ isRequest = true; ++ } else { ++ msg = httpRequest->getParameter("SAMLResponse"); ++ } ++ + if (!msg) + throw BindingException("Request missing SAMLRequest or SAMLResponse form parameter."); ++ + const char* state = httpRequest->getParameter("RelayState"); + if (state) + relayState = state; +@@ -121,16 +128,20 @@ XMLObject* SAML2POSTDecoder::decode( + + saml2::RootObject* root = nullptr; + StatusResponseType* response = nullptr; +- RequestAbstractType* request = dynamic_cast<RequestAbstractType*>(xmlObject.get()); +- if (!request) { ++ RequestAbstractType* request = nullptr; ++ if (isRequest) { ++ request = dynamic_cast<RequestAbstractType*>(xmlObject.get()); ++ if (!request) { ++ throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder was not a SAML 2.0 request message."); ++ } ++ root = static_cast<saml2::RootObject*>(request); ++ } else { + response = dynamic_cast<StatusResponseType*>(xmlObject.get()); +- if (!response) +- throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); ++ if (!response) { ++ throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder was not a SAML 2.0 response message."); ++ } + root = static_cast<saml2::RootObject*>(response); + } +- else { +- root = static_cast<saml2::RootObject*>(request); +- } + + SchemaValidators.validate(root); + +diff --git a/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp b/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp +index ca46e57..a5956d0 100644 +--- a/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp ++++ b/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp +@@ -90,16 +90,24 @@ XMLObject* SAML2RedirectDecoder::decode( + const HTTPRequest* httpRequest=dynamic_cast<const HTTPRequest*>(&genericRequest); + if (!httpRequest) + throw BindingException("Unable to cast request object to HTTPRequest type."); +- const char* msg = httpRequest->getParameter("SAMLResponse"); +- if (!msg) +- msg = httpRequest->getParameter("SAMLRequest"); ++ ++ bool isRequest = false; ++ const char* msg = httpRequest->getParameter("SAMLRequest"); ++ if (msg) { ++ isRequest = true; ++ } else { ++ msg = httpRequest->getParameter("SAMLResponse"); ++ } ++ + if (!msg) + throw BindingException("Request missing SAMLRequest or SAMLResponse query string parameter."); ++ + const char* state = httpRequest->getParameter("RelayState"); + if (state) + relayState = state; + else + relayState.erase(); ++ + state = httpRequest->getParameter("SAMLEncoding"); + if (state && strcmp(state,samlconstants::SAML20_BINDING_URL_ENCODING_DEFLATE)) { + log.warn("SAMLEncoding (%s) was not recognized", state); +@@ -132,16 +140,20 @@ XMLObject* SAML2RedirectDecoder::decode( + + saml2::RootObject* root = nullptr; + StatusResponseType* response = nullptr; +- RequestAbstractType* request = dynamic_cast<RequestAbstractType*>(xmlObject.get()); +- if (!request) { ++ RequestAbstractType* request = nullptr; ++ if (isRequest) { ++ request = dynamic_cast<RequestAbstractType*>(xmlObject.get()); ++ if (!request) { ++ throw BindingException("XML content for SAML 2.0 HTTP-Redirect Decoder was not a SAML 2.0 request message."); ++ } ++ root = static_cast<saml2::RootObject*>(request); ++ } else { + response = dynamic_cast<StatusResponseType*>(xmlObject.get()); +- if (!response) +- throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); ++ if (!response) { ++ throw BindingException("XML content for SAML 2.0 HTTP-Redirect Decoder was not a SAML 2.0 response message."); ++ } + root = static_cast<saml2::RootObject*>(response); + } +- else { +- root = static_cast<saml2::RootObject*>(request); +- } + + SchemaValidators.validate(root); + +diff --git a/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp b/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp +index 812342b..bbde3e5 100644 +--- a/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp ++++ b/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp +@@ -86,7 +86,7 @@ XMLObject* SAML2SOAPDecoder::decode( + + log.debug("validating input"); + string s = genericRequest.getContentType(); +- if (s.find("text/xml") == string::npos) { ++ if (s.find("text/xml") == string::npos || s.find("application/x-www-form-urlencoded") != string::npos) { + log.warn("ignoring incorrect content type (%s)", s.c_str() ? s.c_str() : "none"); + throw BindingException("Invalid content type for SOAP message."); + } diff -Nru opensaml-3.2.1/debian/patches/series opensaml-3.2.1/debian/patches/series --- opensaml-3.2.1/debian/patches/series 1970-01-01 01:00:00.000000000 +0100 +++ opensaml-3.2.1/debian/patches/series 2025-03-14 21:47:33.000000000 +0100 @@ -0,0 +1 @@ +CPPOST-126-Simple-signature-verification-fails-to-detect-.patch I'm ready to upload on your word. -- Feri.
