File 0006-Fix-ECP-signature-not-found-error-when-onl... of Package lasso

Overview Repositories Revisions Requests Users Attributes Meta

File 0006-Fix-ECP-signature-not-found-error-when-only-assertio.patch of Package lasso (Revision 5cd4b227a3ca5896fcc6a2f40d95ad3c)

Currently displaying revision 5cd4b227a3ca5896fcc6a2f40d95ad3c , Show latest

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

Places

File 0006-Fix-ECP-signature-not-found-error-when-only-assertio.patch of Package lasso (Revision 5cd4b227a3ca5896fcc6a2f40d95ad3c)

Places