summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>2019-06-27 23:36:19 (GMT)
committerBenjamin Dauvergne <bdauvergne@entrouvert.com>2019-07-03 21:54:57 (GMT)
commit1e85f1b2bd30c0d93b4a2ef37b35abeae3d15b56 (patch)
tree469803957925040d4dd5f6806b70325369d9d9bb
parent5853fbc15024643448b407d93baf0c5b64f259bd (diff)
downloadlasso-1e85f1b2bd30c0d93b4a2ef37b35abeae3d15b56.zip
lasso-1e85f1b2bd30c0d93b4a2ef37b35abeae3d15b56.tar.gz
lasso-1e85f1b2bd30c0d93b4a2ef37b35abeae3d15b56.tar.bz2
PAOS: Do not populate "Destination" attribute
When ECP profile (saml-ecp-v2.0-cs01) is used with PAOS binding Lasso populates an AuthnRequest with the "Destination" attribute set to AssertionConsumerURL of an SP - this leads to IdP-side errors because the destination attribute in the request does not match the IdP URL. The "Destination" attribute is mandatory only for HTTP Redirect and HTTP Post bindings when AuthRequests are signed per saml-bindings-2.0-os (sections 3.4.5.2 and 3.5.5.2). Specifically for PAOS it makes sense to avoid setting that optional attribute because an ECP decides which IdP to use, not the SP. Fixes Bug: 34409 License: MIT Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
-rw-r--r--lasso/saml-2.0/login.c18
-rw-r--r--lasso/saml-2.0/profile.c10
2 files changed, 18 insertions, 10 deletions
diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c
index 6e8f755..0d4bb1d 100644
--- a/lasso/saml-2.0/login.c
+++ b/lasso/saml-2.0/login.c
@@ -222,7 +222,7 @@ _lasso_login_must_verify_signature(LassoProfile *profile) {
gint
lasso_saml20_login_build_authn_request_msg(LassoLogin *login)
{
- char *url = NULL;
+ char *assertionConsumerServiceURL = NULL;
gboolean must_sign = TRUE;
LassoProfile *profile;
LassoSamlp2AuthnRequest *authn_request;
@@ -247,29 +247,29 @@ lasso_saml20_login_build_authn_request_msg(LassoLogin *login)
}
if (login->http_method == LASSO_HTTP_METHOD_PAOS) {
-
/*
* PAOS is special, the url passed to build_request is the
* AssertionConsumerServiceURL of this SP, not the
- * destination.
+ * destination IdP URL. This is done to fill paos:responseConsumerURL
+ * appropriately down the line in build_request_msg.
+ * See https://dev.entrouvert.org/issues/34409 for more information.
*/
if (authn_request->AssertionConsumerServiceURL) {
- url = authn_request->AssertionConsumerServiceURL;
+ assertionConsumerServiceURL = authn_request->AssertionConsumerServiceURL;
if (!lasso_saml20_provider_check_assertion_consumer_service_url(
- LASSO_PROVIDER(profile->server), url, LASSO_SAML2_METADATA_BINDING_PAOS)) {
+ LASSO_PROVIDER(profile->server), assertionConsumerServiceURL, LASSO_SAML2_METADATA_BINDING_PAOS)) {
rc = LASSO_PROFILE_ERROR_INVALID_REQUEST;
goto cleanup;
}
} else {
- url = lasso_saml20_provider_get_assertion_consumer_service_url_by_binding(
+ assertionConsumerServiceURL = lasso_saml20_provider_get_assertion_consumer_service_url_by_binding(
LASSO_PROVIDER(profile->server), LASSO_SAML2_METADATA_BINDING_PAOS);
- lasso_assign_new_string(authn_request->AssertionConsumerServiceURL, url);
+ lasso_assign_new_string(authn_request->AssertionConsumerServiceURL, assertionConsumerServiceURL);
}
}
-
lasso_check_good_rc(lasso_saml20_profile_build_request_msg(profile, "SingleSignOnService",
- login->http_method, url));
+ login->http_method, assertionConsumerServiceURL));
cleanup:
return rc;
diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c
index 22a4e08..85f535a 100644
--- a/lasso/saml-2.0/profile.c
+++ b/lasso/saml-2.0/profile.c
@@ -968,7 +968,15 @@ lasso_saml20_profile_build_request_msg(LassoProfile *profile, const char *servic
made_url = url = get_url(provider, service, http_method_to_binding(method));
}
- if (url) {
+
+ // Usage of the Destination attribute on a request is mandated only
+ // in "3.4.5.2" and "3.5.5.2" in saml-bindings-2.0-os for signed requests
+ // and is marked as optional in the XSD schema otherwise.
+ // PAOS is a special case because an SP does not select an IdP - ECP does
+ // it instead. Therefore, this attribute needs to be left unpopulated.
+ if (method == LASSO_HTTP_METHOD_PAOS) {
+ lasso_release_string(((LassoSamlp2RequestAbstract*)profile->request)->Destination);
+ } else if (url) {
lasso_assign_string(((LassoSamlp2RequestAbstract*)profile->request)->Destination,
url);
} else {