From 642182bdf49c9c93a86b093ad7335c8a7a5ae8cc Mon Sep 17 00:00:00 2001 From: John Dennis 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 --- 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 028ffb3..91ff302 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 8171e79..22a4e08 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 54c7fb6..e331c07 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; } -- 1.8.3.1