From 642182bdf49c9c93a86b093ad7335c8a7a5ae8cc Mon Sep 17 00:00:00 2001
From: John Dennis <jdennis@redhat.com>
Date: Wed, 9 Jan 2019 17:23:09 -0500
Subject: [PATCH] Fix ECP signature not found error when only assertion is
signed (#26828)
With a SAML Authn Response either the message or the assertion
contained in the response message or both can be signed. Most IdP's
sign the message. This fixes a bug when processing an ECP authn
response when only the assertion is signed.
lasso_saml20_profile_process_soap_response_with_headers() performs a
signature check on the SAML message. A signature can also appear on
the assertion which is checked by
lasso_saml20_login_process_response_status_and_assertion() The problem
occurred when the message was not signed and
lasso_saml20_profile_process_soap_response_with_headers() returned
LASSO_DS_ERROR_SIGNATURE_NOT_FOUND as an error code which is not
actually an error because we haven't checked the signature on the
assertion yet. We were returning the first
LASSO_DS_ERROR_SIGNATURE_NOT_FOUND error when in fact the subsequent
signature check in
lasso_saml20_login_process_response_status_and_assertion() succeeded.
The ECP unit tests were enhanced to cover these cases.
The enhanced unit test revealed a problem in two switch statements
operating on the return value of
lasso_profile_get_signature_verify_hint() which were missing a case
statement for LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE which caused
an abort due to an unknown enumeration value.
Fixes Bug: 26828
License: MIT
Signed-off-by: John Dennis <jdennis@redhat.com>
---
lasso/saml-2.0/login.c | 29 ++++++++----
lasso/saml-2.0/profile.c | 2 +
tests/login_tests_saml2.c | 97 +++++++++++++++++++++++++++++----------
3 files changed, 95 insertions(+), 33 deletions(-)
diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c
index 028ffb31..91ff302d 100644
--- a/lasso/saml-2.0/login.c
+++ b/lasso/saml-2.0/login.c
@@ -1107,18 +1107,31 @@ lasso_saml20_login_process_paos_response_msg(LassoLogin *login, gchar *msg)
{
LassoSoapHeader *header = NULL;
LassoProfile *profile;
- int rc1, rc2;
+ int rc;
lasso_null_param(msg);
profile = LASSO_PROFILE(login);
- rc1 = lasso_saml20_profile_process_soap_response_with_headers(profile, msg, &header);
+ /*
+ * lasso_saml20_profile_process_soap_response_with_headers()
+ * performs a signature check on the SAML message. A signature
+ * can also appear on the assertion which is checked by
+ * lasso_saml20_login_process_response_status_and_assertion()
+ * (below). Therefore if the error is SIGNATURE_NOT_FOUND we
+ * proceed because
+ * lasso_saml20_login_process_response_status_and_assertion()
+ * will test the signature on the assertion.
+ */
+ rc = lasso_saml20_profile_process_soap_response_with_headers(profile, msg, &header);
+ if (rc != 0 && rc != LASSO_DS_ERROR_SIGNATURE_NOT_FOUND) {
+ return rc;
+ }
/*
* If the SOAP message contained a header check for the optional
- * paos:Response and ecp:RelayState elements, if they exist extract their
- * values into the profile.
+ * paos:Response and ecp:RelayState elements, if they exist extract their
+ * values into the profile.
*/
if (header) {
GList *i = NULL;
@@ -1142,12 +1155,8 @@ lasso_saml20_login_process_paos_response_msg(LassoLogin *login, gchar *msg)
lasso_release_gobject(header);
}
- rc2 = lasso_saml20_login_process_response_status_and_assertion(login);
- if (rc1) {
- return rc1;
- }
- return rc2;
-
+ rc = lasso_saml20_login_process_response_status_and_assertion(login);
+ return rc;
}
/**
diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c
index 8171e79e..22a4e08c 100644
--- a/lasso/saml-2.0/profile.c
+++ b/lasso/saml-2.0/profile.c
@@ -398,6 +398,7 @@ lasso_saml20_profile_process_artifact_resolve(LassoProfile *profile, const char
switch (lasso_profile_get_signature_verify_hint(profile)) {
case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE:
+ case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE:
rc = profile->signature_status;
break;
case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE:
@@ -1559,6 +1560,7 @@ lasso_saml20_profile_process_soap_response_with_headers(LassoProfile *profile,
remote_provider, response_msg, "ID", LASSO_MESSAGE_FORMAT_SOAP);
switch (lasso_profile_get_signature_verify_hint(profile)) {
case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE:
+ case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE:
rc = profile->signature_status;
break;
case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE:
diff --git a/tests/login_tests_saml2.c b/tests/login_tests_saml2.c
index 54c7fb63..e331c07a 100644
--- a/tests/login_tests_saml2.c
+++ b/tests/login_tests_saml2.c
@@ -1090,42 +1090,42 @@ START_TEST(test08_test_authnrequest_flags)
make_context(sp_context, "sp5-saml2", "", LASSO_PROVIDER_ROLE_IDP, "idp5-saml2", "")
block_lasso_logs;
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.use_assertion_consumer_service_idx = 1,
.assertion_consumer_service_idx = 0,
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.assertion_consumer_service_url = "http://sp5/singleSignOnPost",
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.protocol_binding = LASSO_SAML2_METADATA_BINDING_ARTIFACT,
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.assertion_consumer_service_url = "http://sp5/singleSignOnPost",
.protocol_binding = LASSO_SAML2_METADATA_BINDING_POST,
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.assertion_consumer_service_url = "http://sp5/singleSignOnArtifact",
.protocol_binding = LASSO_SAML2_METADATA_BINDING_ARTIFACT,
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.assertion_consumer_service_url = "http://sp5/singleSignOnPostAndArtifact",
.protocol_binding = LASSO_SAML2_METADATA_BINDING_ARTIFACT,
.stop_after_build_assertion = 1,
});
- sso_initiated_by_sp2(idp_context, sp_context,
- (SsoSettings) {
+ sso_initiated_by_sp2(idp_context, sp_context,
+ (SsoSettings) {
.assertion_consumer_service_url = "http://sp5/singleSignOnPostAndArtifact",
.protocol_binding = LASSO_SAML2_METADATA_BINDING_POST,
.stop_after_build_assertion = 1,
@@ -1278,7 +1278,9 @@ static void validate_idp_list(LassoEcp *ecp, EcpIdpListVariant ecpIDPListVariant
check_str_equals((char*)g_list_nth(ecp->known_idp_entity_ids_supporting_ecp, 0)->data, "http://idp5/metadata");
}
-void test_ecp(EcpIdpListVariant ecpIDPListVariant)
+void test_ecp(EcpIdpListVariant ecpIDPListVariant,
+ LassoProfileSignatureHint signature_hint,
+ LassoProfileSignatureVerifyHint signature_verify_hint)
{
char *serviceProviderContextDump = NULL, *identityProviderContextDump = NULL;
LassoServer *spContext = NULL, *ecpContext=NULL, *idpContext = NULL;
@@ -1286,7 +1288,7 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
LassoEcp *ecp = NULL;
LassoSamlp2AuthnRequest *request = NULL;
gboolean is_passive = FALSE;
- char *provider_name = NULL;
+ char *provider_name = NULL;
char *relayState = NULL;
char *messageID = NULL;
char *extracted_messageID = NULL;
@@ -1296,7 +1298,7 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
char *ecpPaosResponseMsg = NULL;
char *spLoginDump = NULL;
LassoSaml2Assertion *assertion;
- LassoSamlp2IDPList *idp_list = NULL;
+ LassoSamlp2IDPList *idp_list = NULL;
/*
* SAML2 Profile for ECP (Section 4.2) defines these steps for an ECP
@@ -1322,6 +1324,8 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
spContext = lasso_server_new_from_dump(serviceProviderContextDump);
spLoginContext = lasso_login_new(spContext);
check_not_null(spLoginContext);
+ lasso_profile_set_signature_hint(LASSO_PROFILE(spLoginContext), signature_hint);
+ lasso_profile_set_signature_verify_hint(LASSO_PROFILE(spLoginContext), signature_verify_hint);
check_good_rc(lasso_login_init_authn_request(spLoginContext, "http://idp5/metadata",
LASSO_HTTP_METHOD_PAOS));
@@ -1419,6 +1423,8 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
idpContext = lasso_server_new_from_dump(identityProviderContextDump);
idpLoginContext = lasso_login_new(idpContext);
check_not_null(idpLoginContext);
+ lasso_profile_set_signature_hint(LASSO_PROFILE(idpLoginContext), signature_hint);
+ lasso_profile_set_signature_verify_hint(LASSO_PROFILE(idpLoginContext), signature_verify_hint);
/* Parse the ecpSoapRequestMsg */
check_good_rc(lasso_login_process_authn_request_msg(idpLoginContext, ecpSoapRequestMsg));
@@ -1465,7 +1471,7 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
check_str_equals(ecp->relaystate, relayState);
check_str_equals(ecp->issuer->content, "http://sp5/metadata");
check_str_equals(ecp->provider_name, provider_name);
- check_equals(ecp->is_passive, is_passive);
+ check_equals(ecp->is_passive, is_passive);
/* Validate ECP IdP list info */
validate_idp_list(ecp, ecpIDPListVariant, idp_list);
@@ -1480,6 +1486,8 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
spContext = lasso_server_new_from_dump(serviceProviderContextDump);
spLoginContext = lasso_login_new(spContext);
check_not_null(spLoginContext);
+ lasso_profile_set_signature_hint(LASSO_PROFILE(spLoginContext), signature_hint);
+ lasso_profile_set_signature_verify_hint(LASSO_PROFILE(spLoginContext), signature_verify_hint);
/* Parse the ecpPaosResponseMsg */
check_good_rc(lasso_login_process_paos_response_msg(spLoginContext, ecpPaosResponseMsg));
@@ -1515,19 +1523,61 @@ void test_ecp(EcpIdpListVariant ecpIDPListVariant)
START_TEST(test09_ecp)
{
- test_ecp(ECP_IDP_LIST_NONE);
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE);
}
END_TEST
START_TEST(test10_ecp)
{
- test_ecp(ECP_IDP_LIST_ECP);
+ test_ecp(ECP_IDP_LIST_ECP,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE);
}
END_TEST
START_TEST(test11_ecp)
{
- test_ecp(ECP_IDP_LIST_BOGUS);
+ test_ecp(ECP_IDP_LIST_BOGUS,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE);
+}
+END_TEST
+
+START_TEST(test12_ecp)
+{
+ /* Maybe Sign */
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE);
+
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE);
+
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_MAYBE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE);
+
+ /* Force Sign */
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_FORCE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE);
+
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_FORCE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE);
+
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_FORCE,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE);
+
+ /* Forbid Sign */
+ test_ecp(ECP_IDP_LIST_NONE,
+ LASSO_PROFILE_SIGNATURE_HINT_FORBID,
+ LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE);
+
}
END_TEST
@@ -1538,7 +1588,7 @@ void check_digest_method(G_GNUC_UNUSED LassoLogin *idp_login_context, LassoLogin
lasso_release_string(dump)
}
-START_TEST(test12_sso_sp_with_rsa_sha256_signatures)
+START_TEST(test13_sso_sp_with_rsa_sha256_signatures)
{
LassoServer *idp_context = NULL;
LassoServer *sp_context = NULL;
@@ -1595,7 +1645,8 @@ login_saml2_suite()
tcase_add_test(tc_ecp, test09_ecp);
tcase_add_test(tc_ecp, test10_ecp);
tcase_add_test(tc_ecp, test11_ecp);
- tcase_add_test(tc_spLogin, test12_sso_sp_with_rsa_sha256_signatures);
+ tcase_add_test(tc_ecp, test12_ecp);
+ tcase_add_test(tc_spLogin, test13_sso_sp_with_rsa_sha256_signatures);
return s;
}
--
2.20.1