From 81c35bbe2effac358126f5aea78e4f5b9e115f6e Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 16 Apr 2010 15:37:17 +0000 Subject: [PATCH] Ameliorate support for lasso_profile_set_signature_verify_hint * lasso/id-ff/profile.h: - add end symbol for enum LassoProfileSignatureVerifyHint * lasso/id-ff/profile.c: - fix documentation of lasso_profile_set_signature_verify_hint - do not allow to set or return invalid value for the signature_verify_hint attribute. * lasso/saml-2.0/login.c: - handle new enum value * lasso/saml-2.0/profile.c: - handle new enum value - fix missing catch of signature error reporting when signature_verify_hint is IGNORE. * docs/reference/lasso/lasso-sections.txt: - export enums LassoProfileSignatureHint and LassoProfileSignatureVerifyHint * tests/metadata_tests.c: - fix test of all Role enumerations --- docs/reference/lasso/lasso-sections.txt | 2 + lasso/id-ff/profile.c | 27 +++++++++--- lasso/id-ff/profile.h | 4 +- lasso/saml-2.0/login.c | 4 ++ lasso/saml-2.0/profile.c | 58 ++++++++++++++----------- tests/metadata_tests.c | 2 +- 6 files changed, 64 insertions(+), 33 deletions(-) diff --git a/docs/reference/lasso/lasso-sections.txt b/docs/reference/lasso/lasso-sections.txt index bceab812..a05e2d35 100644 --- a/docs/reference/lasso/lasso-sections.txt +++ b/docs/reference/lasso/lasso-sections.txt @@ -72,6 +72,8 @@ LASSO_NAME_REGISTRATION_GET_CLASS LassoProfile LassoProfile LassoRequestType +LassoProfileSignatureHint +LassoProfileSignatureVerifyHint lasso_profile_get_request_type_from_soap_msg lasso_profile_set_soap_fault_response lasso_profile_is_liberty_query diff --git a/lasso/id-ff/profile.c b/lasso/id-ff/profile.c index f3b3307a..7b590d6b 100644 --- a/lasso/id-ff/profile.c +++ b/lasso/id-ff/profile.c @@ -495,8 +495,8 @@ lasso_profile_set_artifact_message(LassoProfile *profile, const char *message) * Return the #LassoServer linked to this profile object. A profile object should always contains * one. It allows to find metadatas of other providers and to know our own metadatas. * - * Return value: (transfer none): a #LassoServer or NULL if profile is not a #LassoProfile or no #LassoServer object - * was setup at the creation of this profile. + * Return value: (transfer none): a #LassoServer or NULL if profile is not a #LassoProfile or no + * #LassoServer object was setup at the creation of this profile. */ LassoServer* lasso_profile_get_server(LassoProfile *profile) @@ -524,10 +524,12 @@ static struct XmlSnippet schema_snippets[] = { { "Response", SNIPPET_NODE_IN_CHILD, G_STRUCT_OFFSET(LassoProfile, response), NULL, NULL, NULL}, { "NameIdentifier", SNIPPET_NODE_IN_CHILD, G_STRUCT_OFFSET(LassoProfile, nameIdentifier), NULL, NULL, NULL}, - { "RemoteProviderID", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, remote_providerID), NULL, NULL, NULL}, + { "RemoteProviderID", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, remote_providerID), + NULL, NULL, NULL}, { "MsgUrl", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, msg_url), NULL, NULL, NULL}, { "MsgBody", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, msg_body), NULL, NULL, NULL}, - { "MsgRelayState", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, msg_relayState), NULL, NULL, NULL}, + { "MsgRelayState", SNIPPET_CONTENT, G_STRUCT_OFFSET(LassoProfile, msg_relayState), NULL, + NULL, NULL}, { "HttpRequestMethod", SNIPPET_CONTENT | SNIPPET_INTEGER, G_STRUCT_OFFSET(LassoProfile, http_request_method), NULL, NULL, NULL}, {NULL, 0, 0, NULL, NULL, NULL} @@ -621,8 +623,15 @@ lasso_profile_set_signature_hint(LassoProfile *profile, LassoProfileSignatureHin LassoProfileSignatureHint lasso_profile_get_signature_hint(LassoProfile *profile) { + LassoProfileSignatureVerifyHint signature_verify_hint; if (! LASSO_IS_PROFILE(profile) && ! profile->private_data) return LASSO_PROFILE_SIGNATURE_HINT_MAYBE; + signature_verify_hint = profile->private_data->signature_verify_hint; + if (signature_verify_hint >= LASSO_PROFILE_SIGNATURE_VERIFY_HINT_LAST) { + message(G_LOG_LEVEL_WARNING, "%u is an invalid signature verify hint", + signature_verify_hint); + return LASSO_PROFILE_SIGNATURE_HINT_MAYBE; + } return profile->private_data->signature_hint; } @@ -632,14 +641,20 @@ lasso_profile_get_signature_hint(LassoProfile *profile) * @signature_verify_hint: whether next received message signatures should be checked or not (or let * Lasso choose from implicit information). * - * By default each profile will choose to sign or not its messages, this method allow to force or + * By default each profile will choose to verify or not its messages, this method allow to force or * forbid the signature of messages, on a per transaction basis. */ void -lasso_profile_set_signature_verify_hint(LassoProfile *profile, LassoProfileSignatureVerifyHint signature_verify_hint) +lasso_profile_set_signature_verify_hint(LassoProfile *profile, + LassoProfileSignatureVerifyHint signature_verify_hint) { if (! LASSO_IS_PROFILE(profile) && ! profile->private_data) return; + if (signature_verify_hint >= LASSO_PROFILE_SIGNATURE_VERIFY_HINT_LAST) { + message(G_LOG_LEVEL_WARNING, "%i is an invalid argument for " __FUNCTION__, + signature_verify_hint); + return; + } profile->private_data->signature_verify_hint = signature_verify_hint; } diff --git a/lasso/id-ff/profile.h b/lasso/id-ff/profile.h index 270a3034..90224ffa 100644 --- a/lasso/id-ff/profile.h +++ b/lasso/id-ff/profile.h @@ -110,6 +110,7 @@ typedef enum { /** * LassoProfileSignatureVerifyHint: * @LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE: let Lasso decide what to do. + * @LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE: always check signatures. * @LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: check signatures but do not stop protocol handling * on failures. The result of signature checking is still available in * #LassoProfile.signature_status @@ -119,7 +120,8 @@ typedef enum { */ typedef enum { LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE = 0, - LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE = 1 + LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE = 1, + LASSO_PROFILE_SIGNATURE_VERIFY_HINT_LAST } LassoProfileSignatureVerifyHint; /** diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c index 7e6e3a5f..c6262944 100644 --- a/lasso/saml-2.0/login.c +++ b/lasso/saml-2.0/login.c @@ -1004,6 +1004,8 @@ lasso_saml20_login_process_authn_response_msg(LassoLogin *login, gchar *authn_re break; case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: break; + default: + g_assert(0); } return 0; } @@ -1225,6 +1227,8 @@ lasso_saml20_login_process_response_status_and_assertion(LassoLogin *login) break; case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: break; + default: + g_assert(0); } lasso_extract_node_or_fail(subject, assertion->Subject, SAML2_SUBJECT, LASSO_PROFILE_ERROR_MISSING_SUBJECT); diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c index 7b282726..829929ad 100644 --- a/lasso/saml-2.0/profile.c +++ b/lasso/saml-2.0/profile.c @@ -363,6 +363,9 @@ lasso_saml20_profile_process_artifact_resolve(LassoProfile *profile, const char break; case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: break; + default: + g_assert(0); + break; } lasso_assign_string(profile->private_data->artifact, @@ -472,11 +475,14 @@ lasso_saml20_profile_set_session_from_dump_decrypt( lasso_release_gobject(assertion->Subject->EncryptedID); } else { /* decrypt */ int rc; - rc = lasso_saml2_encrypted_element_decrypt(assertion->Subject->EncryptedID, lasso_server_get_encryption_private_key(profile->server), (LassoNode**) &assertion->Subject->NameID); + rc = lasso_saml2_encrypted_element_decrypt(assertion->Subject->EncryptedID, + lasso_server_get_encryption_private_key(profile->server), + (LassoNode**) &assertion->Subject->NameID); if (rc == 0) { lasso_release_gobject(assertion->Subject->EncryptedID); } else { - message(G_LOG_LEVEL_WARNING, "Could not decrypt EncrypteID from assertion in session dump: %s", lasso_strerror(rc)); + message(G_LOG_LEVEL_WARNING, "Could not decrypt EncrypteID from" + " assertion in session dump: %s", lasso_strerror(rc)); } } } @@ -678,6 +684,8 @@ lasso_saml20_profile_process_soap_request(LassoProfile *profile, break; case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: break; + default: + g_assert(0); } cleanup: @@ -997,8 +1005,8 @@ lasso_saml20_profile_validate_request(LassoProfile *profile, gboolean needs_iden lasso_saml20_profile_init_response(profile, status_response, LASSO_SAML2_STATUS_CODE_SUCCESS, NULL); - if (profile->signature_status) { - message(G_LOG_LEVEL_WARNING, "Request signature is invalid"); + if (lasso_profile_get_signature_verify_hint(profile) == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE && + profile->signature_status) { lasso_saml20_profile_set_response_status(profile, LASSO_SAML2_STATUS_CODE_REQUESTER, LASSO_LIB_STATUS_CODE_INVALID_SIGNATURE); @@ -1256,6 +1264,7 @@ lasso_saml20_profile_process_any_response(LassoProfile *profile, LassoSamlp2StatusCode *status_code1 = NULL; LassoMessageFormat format; gboolean missing_issuer = FALSE; + LassoProfileSignatureVerifyHint signature_verify_hint; xmlDoc *doc = NULL; xmlNode *content = NULL; @@ -1263,6 +1272,7 @@ lasso_saml20_profile_process_any_response(LassoProfile *profile, lasso_bad_param(PROFILE, profile); lasso_bad_param(SAMLP2_STATUS_RESPONSE, status_response); + signature_verify_hint = lasso_profile_get_signature_verify_hint(profile); /* reset signature_status */ profile->signature_status = 0; format = lasso_node_init_from_message_with_format((LassoNode*)status_response, @@ -1293,31 +1303,25 @@ lasso_saml20_profile_process_any_response(LassoProfile *profile, LASSO_PROFILE_ERROR_MISSING_SERVER); if (_lasso_saml20_is_valid_issuer(response_abstract->Issuer)) { lasso_assign_string(profile->remote_providerID, response_abstract->Issuer->content); + remote_provider = lasso_server_get_provider(server, profile->remote_providerID); } else { - /* no issuer ? use implicit provider id */ missing_issuer = TRUE; } - remote_provider = lasso_server_get_provider(server, profile->remote_providerID); - goto_cleanup_if_fail_with_rc(remote_provider != NULL, LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND); - - /* verify the signature at the message level */ - if (content && doc && format != LASSO_MESSAGE_FORMAT_QUERY) { - profile->signature_status = - lasso_provider_verify_saml_signature(remote_provider, content, doc); - } else if (format == LASSO_MESSAGE_FORMAT_QUERY) { - profile->signature_status = - lasso_provider_verify_query_signature(remote_provider, response_msg); + if (remote_provider) { + /* verify the signature at the message level */ + if (content && doc && format != LASSO_MESSAGE_FORMAT_QUERY) { + profile->signature_status = + lasso_provider_verify_saml_signature(remote_provider, content, doc); + } else if (format == LASSO_MESSAGE_FORMAT_QUERY) { + profile->signature_status = + lasso_provider_verify_query_signature(remote_provider, response_msg); + } else { + profile->signature_status = LASSO_DS_ERROR_SIGNATURE_VERIFICATION_FAILED; + } } else { - profile->signature_status = LASSO_DS_ERROR_SIGNATURE_VERIFICATION_FAILED; - } - /* propagate signature status ? */ - switch (lasso_profile_get_signature_verify_hint(profile)) { - case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE: - rc = profile->signature_status; - break; - case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: - break; + rc = LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND; + profile->signature_status = LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND; } /* verify status code */ @@ -1337,7 +1341,8 @@ cleanup: if (rc) { return rc; } - if (profile->signature_status) { + if ((signature_verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE) && + profile->signature_status) { return LASSO_PROFILE_ERROR_CANNOT_VERIFY_SIGNATURE; } if (missing_issuer) { @@ -1390,6 +1395,9 @@ lasso_saml20_profile_process_soap_response(LassoProfile *profile, break; case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE: break; + default: + g_assert(0); + break; } cleanup: diff --git a/tests/metadata_tests.c b/tests/metadata_tests.c index 975e4d10..cb4c5154 100644 --- a/tests/metadata_tests.c +++ b/tests/metadata_tests.c @@ -94,7 +94,7 @@ START_TEST(test07_metadata_role_descriptors) int i = 0; check_not_null(provider); - for (i = LASSO_PROVIDER_ROLE_ANY+1; i < LASSO_PROVIDER_ROLE_LAST; i++) { + for (i = 1; i < LASSO_PROVIDER_ROLE_LAST; i *= 2) { l = lasso_provider_get_metadata_keys_for_role(provider, i); if (i == LASSO_PROVIDER_ROLE_IDP) { check_equals(g_list_length(l), 10);