Bug#1100464: Ready to upload the security fix
Ferenc Wágner
wferi at debian.org
Fri Mar 14 21:12:36 GMT 2025
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 <wferi at 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 <cantor.2 at 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.
More information about the Pkg-shibboleth-devel
mailing list