|
 |
f2be37 |
From 1e85f1b2bd30c0d93b4a2ef37b35abeae3d15b56 Mon Sep 17 00:00:00 2001
|
|
|
f2be37 |
From: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
|
|
|
f2be37 |
Date: Fri, 28 Jun 2019 02:36:19 +0300
|
|
|
f2be37 |
Subject: [PATCH] PAOS: Do not populate "Destination" attribute
|
|
|
f2be37 |
|
|
|
f2be37 |
When ECP profile (saml-ecp-v2.0-cs01) is used with PAOS binding Lasso
|
|
|
f2be37 |
populates an AuthnRequest with the "Destination" attribute set to
|
|
|
f2be37 |
AssertionConsumerURL of an SP - this leads to IdP-side errors because
|
|
|
f2be37 |
the destination attribute in the request does not match the IdP URL.
|
|
|
f2be37 |
|
|
|
f2be37 |
The "Destination" attribute is mandatory only for HTTP Redirect and HTTP
|
|
|
f2be37 |
Post bindings when AuthRequests are signed per saml-bindings-2.0-os
|
|
|
f2be37 |
(sections 3.4.5.2 and 3.5.5.2). Specifically for PAOS it makes sense to
|
|
|
f2be37 |
avoid setting that optional attribute because an ECP decides which IdP
|
|
|
f2be37 |
to use, not the SP.
|
|
|
f2be37 |
|
|
|
f2be37 |
Fixes Bug: 34409
|
|
|
f2be37 |
License: MIT
|
|
|
f2be37 |
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
|
|
|
f2be37 |
---
|
|
|
f2be37 |
lasso/saml-2.0/login.c | 18 +++++++++---------
|
|
|
f2be37 |
lasso/saml-2.0/profile.c | 10 +++++++++-
|
|
|
f2be37 |
2 files changed, 18 insertions(+), 10 deletions(-)
|
|
|
f2be37 |
|
|
|
f2be37 |
diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c
|
|
|
f2be37 |
index 6e8f7553..0d4bb1da 100644
|
|
|
f2be37 |
--- a/lasso/saml-2.0/login.c
|
|
|
f2be37 |
+++ b/lasso/saml-2.0/login.c
|
|
|
f2be37 |
@@ -222,7 +222,7 @@ _lasso_login_must_verify_signature(LassoProfile *profile) {
|
|
|
f2be37 |
gint
|
|
|
f2be37 |
lasso_saml20_login_build_authn_request_msg(LassoLogin *login)
|
|
|
f2be37 |
{
|
|
|
f2be37 |
- char *url = NULL;
|
|
|
f2be37 |
+ char *assertionConsumerServiceURL = NULL;
|
|
|
f2be37 |
gboolean must_sign = TRUE;
|
|
|
f2be37 |
LassoProfile *profile;
|
|
|
f2be37 |
LassoSamlp2AuthnRequest *authn_request;
|
|
|
f2be37 |
@@ -247,29 +247,29 @@ lasso_saml20_login_build_authn_request_msg(LassoLogin *login)
|
|
|
f2be37 |
}
|
|
|
f2be37 |
|
|
|
f2be37 |
if (login->http_method == LASSO_HTTP_METHOD_PAOS) {
|
|
|
f2be37 |
-
|
|
|
f2be37 |
/*
|
|
|
f2be37 |
* PAOS is special, the url passed to build_request is the
|
|
|
f2be37 |
* AssertionConsumerServiceURL of this SP, not the
|
|
|
f2be37 |
- * destination.
|
|
|
f2be37 |
+ * destination IdP URL. This is done to fill paos:responseConsumerURL
|
|
|
f2be37 |
+ * appropriately down the line in build_request_msg.
|
|
|
f2be37 |
+ * See https://dev.entrouvert.org/issues/34409 for more information.
|
|
|
f2be37 |
*/
|
|
|
f2be37 |
if (authn_request->AssertionConsumerServiceURL) {
|
|
|
f2be37 |
- url = authn_request->AssertionConsumerServiceURL;
|
|
|
f2be37 |
+ assertionConsumerServiceURL = authn_request->AssertionConsumerServiceURL;
|
|
|
f2be37 |
if (!lasso_saml20_provider_check_assertion_consumer_service_url(
|
|
|
f2be37 |
- LASSO_PROVIDER(profile->server), url, LASSO_SAML2_METADATA_BINDING_PAOS)) {
|
|
|
f2be37 |
+ LASSO_PROVIDER(profile->server), assertionConsumerServiceURL, LASSO_SAML2_METADATA_BINDING_PAOS)) {
|
|
|
f2be37 |
rc = LASSO_PROFILE_ERROR_INVALID_REQUEST;
|
|
|
f2be37 |
goto cleanup;
|
|
|
f2be37 |
}
|
|
|
f2be37 |
} else {
|
|
|
f2be37 |
- url = lasso_saml20_provider_get_assertion_consumer_service_url_by_binding(
|
|
|
f2be37 |
+ assertionConsumerServiceURL = lasso_saml20_provider_get_assertion_consumer_service_url_by_binding(
|
|
|
f2be37 |
LASSO_PROVIDER(profile->server), LASSO_SAML2_METADATA_BINDING_PAOS);
|
|
|
f2be37 |
- lasso_assign_new_string(authn_request->AssertionConsumerServiceURL, url);
|
|
|
f2be37 |
+ lasso_assign_new_string(authn_request->AssertionConsumerServiceURL, assertionConsumerServiceURL);
|
|
|
f2be37 |
}
|
|
|
f2be37 |
}
|
|
|
f2be37 |
|
|
|
f2be37 |
-
|
|
|
f2be37 |
lasso_check_good_rc(lasso_saml20_profile_build_request_msg(profile, "SingleSignOnService",
|
|
|
f2be37 |
- login->http_method, url));
|
|
|
f2be37 |
+ login->http_method, assertionConsumerServiceURL));
|
|
|
f2be37 |
|
|
|
f2be37 |
cleanup:
|
|
|
f2be37 |
return rc;
|
|
|
f2be37 |
diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c
|
|
|
f2be37 |
index 22a4e08c..85f535ae 100644
|
|
|
f2be37 |
--- a/lasso/saml-2.0/profile.c
|
|
|
f2be37 |
+++ b/lasso/saml-2.0/profile.c
|
|
|
f2be37 |
@@ -968,7 +968,15 @@ lasso_saml20_profile_build_request_msg(LassoProfile *profile, const char *servic
|
|
|
f2be37 |
made_url = url = get_url(provider, service, http_method_to_binding(method));
|
|
|
f2be37 |
}
|
|
|
f2be37 |
|
|
|
f2be37 |
- if (url) {
|
|
|
f2be37 |
+
|
|
|
f2be37 |
+ // Usage of the Destination attribute on a request is mandated only
|
|
|
f2be37 |
+ // in "3.4.5.2" and "3.5.5.2" in saml-bindings-2.0-os for signed requests
|
|
|
f2be37 |
+ // and is marked as optional in the XSD schema otherwise.
|
|
|
f2be37 |
+ // PAOS is a special case because an SP does not select an IdP - ECP does
|
|
|
f2be37 |
+ // it instead. Therefore, this attribute needs to be left unpopulated.
|
|
|
f2be37 |
+ if (method == LASSO_HTTP_METHOD_PAOS) {
|
|
|
f2be37 |
+ lasso_release_string(((LassoSamlp2RequestAbstract*)profile->request)->Destination);
|
|
|
f2be37 |
+ } else if (url) {
|
|
|
f2be37 |
lasso_assign_string(((LassoSamlp2RequestAbstract*)profile->request)->Destination,
|
|
|
f2be37 |
url);
|
|
|
f2be37 |
} else {
|
|
|
f2be37 |
--
|
|
|
f2be37 |
2.20.1
|
|
|
f2be37 |
|