From 8be7b0414dc19cca7b892deeccc64b5fcedaf62c Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 14 Apr 2011 16:45:43 +0200 Subject: [PATCH] [core] add flags parameter to lasso_server_load_metadata to tune signature checking on metadata files The flags parameter allows to control the checking of digital signature upon EntityDescriptor and EntitiesDescriptor nodes in SAML 2.0 metadata files. The default behaviour is to check all found signatures and to inherit signature from EntitiesDescriptor to their children. By only enabling checking of EntityDescrtiptor node signatures it's also possible to only check signature at the EntityDescriptor level and so only trust individual entities and not the aggregating provider. --- lasso/id-ff/server.c | 22 +++++----- lasso/id-ff/server.h | 22 +++++++++- lasso/saml-2.0/server.c | 77 ++++++++++++++++++++++++++------- lasso/saml-2.0/serverprivate.h | 4 +- tests/basic_tests.c | 11 ++++- tests/data/renater-metadata.xml | 4 +- 6 files changed, 109 insertions(+), 31 deletions(-) diff --git a/lasso/id-ff/server.c b/lasso/id-ff/server.c index e209017e..e959376f 100644 --- a/lasso/id-ff/server.c +++ b/lasso/id-ff/server.c @@ -760,6 +760,8 @@ lasso_server_get_encryption_private_key(LassoServer *server) * loaded, can be NULL. * @loaded_entity_ids:(transfer full)(element-type string)(allow-none): an output parameter for the * list of the loaded EntityID, can be NULL. + * @flags: flags modifying the behaviour for checking signatures on EntityDescriptor and + * EntitiesDescriptors nodes. * * Load all the SAML 2.0 entities from @federation_file which contains a declaration for @role. If * @trusted_roots is non-NULL, use it to check a signature on the metadata file, otherwise ignore @@ -778,18 +780,24 @@ lasso_server_get_encryption_private_key(LassoServer *server) */ lasso_error_t lasso_server_load_metadata(LassoServer *server, LassoProviderRole role, const gchar *federation_file, - const gchar *trusted_roots, GList *blacklisted_entity_ids, GList **loaded_entity_ids) + const gchar *trusted_roots, GList *blacklisted_entity_ids, + GList **loaded_entity_ids, enum LassoServerLoadMetadataFlag flags) { xmlDoc *doc = NULL; xmlNode *root = NULL; xmlSecKeysMngr *keys_mngr = NULL; lasso_error_t rc = 0; - GList *uri_references = NULL; lasso_bad_param(SERVER, server); g_return_val_if_fail(role == LASSO_PROVIDER_ROLE_SP || role == LASSO_PROVIDER_ROLE_IDP, LASSO_PARAM_ERROR_BAD_TYPE_OR_NULL_OBJ); + if (flags == LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT) { + flags = LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITIES_DESCRIPTOR_SIGNATURE + | LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITY_DESCRIPTOR_SIGNATURE + | LASSO_SERVER_LOAD_METADATA_FLAG_INHERIT_SIGNATURE; + } + if (trusted_roots) { keys_mngr = lasso_load_certs_from_pem_certs_chain_file(trusted_roots); lasso_return_val_if_fail(keys_mngr != NULL, @@ -798,20 +806,14 @@ lasso_server_load_metadata(LassoServer *server, LassoProviderRole role, const gc doc = lasso_xml_parse_file(federation_file); goto_cleanup_if_fail_with_rc(doc, LASSO_SERVER_ERROR_INVALID_XML); root = xmlDocGetRootElement(doc); - if (trusted_roots) { - /* check metadata file signature */ - lasso_check_good_rc(lasso_verify_signature(root, doc, "ID", keys_mngr, NULL, - EMPTY_URI, &uri_references)); - } if (lasso_strisequal((char*)root->ns->href, LASSO_SAML2_METADATA_HREF)) { - lasso_check_good_rc(lasso_saml20_server_load_metadata(server, role, root, - blacklisted_entity_ids, loaded_entity_ids)); + lasso_check_good_rc(lasso_saml20_server_load_metadata(server, role, doc, root, + blacklisted_entity_ids, loaded_entity_ids, keys_mngr, flags)); } else { goto_cleanup_with_rc(LASSO_ERROR_UNIMPLEMENTED); } cleanup: - lasso_release_list_of_strings(uri_references); lasso_release_key_manager(keys_mngr); lasso_release_doc(doc); return rc; diff --git a/lasso/id-ff/server.h b/lasso/id-ff/server.h index 43033858..110f5b5e 100644 --- a/lasso/id-ff/server.h +++ b/lasso/id-ff/server.h @@ -67,6 +67,24 @@ struct _LassoServerClass { LassoProviderClass parent; }; +/** + * LassoServerLoadMetadataFlag: + * @LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT: the default policy is to check signature on entity and + * entities descriptor, and to let signature be inherited by child nodes. + * @LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITIES_DESCRIPTOR_SIGNATURE: check signature on + * EntitiesDesctiptor nodes, + * @LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITY_DESCRIPTOR_SIGNATURE: check signature on + * EntityDescriptor nodes, + * @LASSO_SERVER_LOAD_METADATA_FLAG_INHERIT_SIGNATURE: when an EntitiesDescriptor is signed, all its + * children inherit the trust from this signature and their signature is not checked. + */ +enum LassoServerLoadMetadataFlag { + LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT = 0, + LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITIES_DESCRIPTOR_SIGNATURE = 1, + LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITY_DESCRIPTOR_SIGNATURE = 2, + LASSO_SERVER_LOAD_METADATA_FLAG_INHERIT_SIGNATURE = 4 +}; + LASSO_EXPORT GType lasso_server_get_type(void); LASSO_EXPORT LassoServer* lasso_server_new(const gchar *metadata, @@ -104,7 +122,9 @@ LASSO_EXPORT lasso_error_t lasso_server_set_encryption_private_key_with_password const gchar *filename_or_buffer, const gchar *password); LASSO_EXPORT lasso_error_t lasso_server_load_metadata(LassoServer *server, LassoProviderRole role, - const gchar *federation_file, const gchar *trusted_roots, GList *blacklisted_entity_ids, GList **loaded_entity_ids); + const gchar *federation_file, const gchar *trusted_roots, GList + *blacklisted_entity_ids, GList **loaded_entity_ids, + enum LassoServerLoadMetadataFlag flags); #ifdef __cplusplus } diff --git a/lasso/saml-2.0/server.c b/lasso/saml-2.0/server.c index ad54e6d6..75cebecb 100644 --- a/lasso/saml-2.0/server.c +++ b/lasso/saml-2.0/server.c @@ -88,6 +88,15 @@ lasso_saml20_server_load_affiliation(LassoServer *server, xmlNode *node) return 0; } +static void +debug_report_signature_error(xmlNode *node, lasso_error_t result) { + xmlChar *path; + + path = xmlGetNodePath(node); + debug("Could not check signature whose xpath is '%s': %s", path, lasso_strerror(result)); + lasso_release_xml_string(path); +} + static gboolean _lasso_test_sp_descriptor(xmlNode *node) { return xmlSecFindChild(node, @@ -104,9 +113,11 @@ _lasso_test_idp_descriptor(xmlNode *node) { static lasso_error_t lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole role, - xmlNode *entity, GList *blacklisted_entity_ids, GList **loaded_end) + xmlDoc *doc, xmlNode *entity, GList *blacklisted_entity_ids, GList **loaded_end, + xmlSecKeysMngr *keys_mngr, enum LassoServerLoadMetadataFlag flags) { LassoProvider *provider = NULL; + gboolean check_signature = flags & LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITY_DESCRIPTOR_SIGNATURE; if (role == LASSO_PROVIDER_ROLE_IDP && ! _lasso_test_idp_descriptor(entity)) { return 0; @@ -115,6 +126,17 @@ lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole return 0; } + if (keys_mngr && check_signature) { + lasso_error_t result; + + result = lasso_verify_signature(entity, doc, "ID", keys_mngr, NULL, EMPTY_URI, + NULL); + if (result != 0) { + debug_report_signature_error(entity, result); + return result; + } + } + provider = lasso_provider_new_from_xmlnode(role, entity); if (provider) { char *name = g_strdup(provider->ProviderID); @@ -127,6 +149,7 @@ lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole if (*loaded_end) { GList *l = *loaded_end; l->next = g_new0(GList, 1); + l->next->prev = l; l->next->data = g_strdup(name); *loaded_end = l->next; } @@ -138,22 +161,41 @@ lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole } static lasso_error_t lasso_saml20_server_load_metadata_child(LassoServer *server, - LassoProviderRole role, xmlNode *child, GList *blacklisted_entity_ids, - GList **loaded_end); + LassoProviderRole role, xmlDoc *doc, xmlNode *child, GList *blacklisted_entity_ids, + GList **loaded_end, xmlSecKeysMngr *keys_mngr, enum LassoServerLoadMetadataFlag flags); static lasso_error_t -lasso_saml20_server_load_metadata_entities(LassoServer *server, LassoProviderRole role, xmlNode *entities, - GList *blacklisted_entity_ids, GList **loaded_end) +lasso_saml20_server_load_metadata_entities(LassoServer *server, LassoProviderRole role, xmlDoc *doc, xmlNode *entities, + GList *blacklisted_entity_ids, GList **loaded_end, + xmlSecKeysMngr *keys_mngr, enum LassoServerLoadMetadataFlag flags) { xmlNode *child; gboolean at_least_one = FALSE; + gboolean check_signature = flags & LASSO_SERVER_LOAD_METADATA_FLAG_CHECK_ENTITIES_DESCRIPTOR_SIGNATURE; + gboolean inherit_signature = flags & LASSO_SERVER_LOAD_METADATA_FLAG_INHERIT_SIGNATURE; + + /* if a key store is passed, check signature */ + if (keys_mngr && check_signature) { + lasso_error_t result; + + result = lasso_verify_signature(entities, doc, "ID", keys_mngr, NULL, EMPTY_URI, + NULL); + if (result == 0) { + if (inherit_signature) { + keys_mngr = NULL; + } + } else { + debug_report_signature_error(entities, result); + return result; + } + } child = xmlSecGetNextElementNode(entities->children); while (child) { lasso_error_t rc = 0; - rc = lasso_saml20_server_load_metadata_child(server, role, child, - blacklisted_entity_ids, loaded_end); + rc = lasso_saml20_server_load_metadata_child(server, role, doc, child, + blacklisted_entity_ids, loaded_end, keys_mngr, flags); if (rc == 0) { at_least_one = TRUE; } @@ -163,19 +205,20 @@ lasso_saml20_server_load_metadata_entities(LassoServer *server, LassoProviderRol } static lasso_error_t -lasso_saml20_server_load_metadata_child(LassoServer *server, LassoProviderRole role, xmlNode *child, - GList *blacklisted_entity_ids, GList **loaded_end) +lasso_saml20_server_load_metadata_child(LassoServer *server, LassoProviderRole role, xmlDoc *doc, + xmlNode *child, GList *blacklisted_entity_ids, GList **loaded_end, + xmlSecKeysMngr *keys_mngr, enum LassoServerLoadMetadataFlag flags) { if (xmlSecCheckNodeName(child, BAD_CAST LASSO_SAML2_METADATA_ELEMENT_ENTITY_DESCRIPTOR, BAD_CAST LASSO_SAML2_METADATA_HREF)) { - return lasso_saml20_server_load_metadata_entity(server, role, child, - blacklisted_entity_ids, loaded_end); + return lasso_saml20_server_load_metadata_entity(server, role, doc, child, + blacklisted_entity_ids, loaded_end, keys_mngr, flags); } else if (xmlSecCheckNodeName(child, BAD_CAST LASSO_SAML2_METADATA_ELEMENT_ENTITIES_DESCRIPTOR, BAD_CAST LASSO_SAML2_METADATA_HREF)) { - return lasso_saml20_server_load_metadata_entities(server, role, child, - blacklisted_entity_ids, loaded_end); + return lasso_saml20_server_load_metadata_entities(server, role, doc, child, + blacklisted_entity_ids, loaded_end, keys_mngr, flags); } return LASSO_SERVER_ERROR_INVALID_XML; } @@ -195,8 +238,10 @@ lasso_saml20_server_load_metadata_child(LassoServer *server, LassoProviderRole r * otherwise. */ lasso_error_t -lasso_saml20_server_load_metadata(LassoServer *server, LassoProviderRole role, xmlNode *root_node, - GList *blacklisted_entity_ids, GList **loaded_entity_ids) +lasso_saml20_server_load_metadata(LassoServer *server, LassoProviderRole role, + xmlDoc *doc, xmlNode *root_node, + GList *blacklisted_entity_ids, GList **loaded_entity_ids, + xmlSecKeysMngr *keys_mngr, enum LassoServerLoadMetadataFlag flags) { lasso_error_t rc = 0; GList loaded = { .data = NULL, .next = NULL }; @@ -206,7 +251,7 @@ lasso_saml20_server_load_metadata(LassoServer *server, LassoProviderRole role, x loaded_end = &loaded; } rc = lasso_saml20_server_load_metadata_child(server, role, - root_node, blacklisted_entity_ids, &loaded_end); + doc, root_node, blacklisted_entity_ids, &loaded_end, keys_mngr, flags); if (loaded_entity_ids) { lasso_release_list_of_strings(*loaded_entity_ids); *loaded_entity_ids = loaded.next; diff --git a/lasso/saml-2.0/serverprivate.h b/lasso/saml-2.0/serverprivate.h index 08fea735..30529d2c 100644 --- a/lasso/saml-2.0/serverprivate.h +++ b/lasso/saml-2.0/serverprivate.h @@ -34,7 +34,9 @@ extern "C" { int lasso_saml20_server_load_affiliation(LassoServer *server, xmlNode *node); lasso_error_t lasso_saml20_server_load_metadata(LassoServer *server, LassoProviderRole role, - xmlNode *root_node, GList *blacklisted_entity_ids, GList **loaded_entity_ids); + xmlDoc *doc, xmlNode *root_node, GList *blacklisted_entity_ids, + GList **loaded_entity_ids, xmlSecKeysMngr *keys_mngr, + enum LassoServerLoadMetadataFlag flags); #ifdef __cplusplus } diff --git a/tests/basic_tests.c b/tests/basic_tests.c index 2d5af9db..d469c474 100644 --- a/tests/basic_tests.c +++ b/tests/basic_tests.c @@ -1956,9 +1956,17 @@ START_TEST(test13_test_lasso_server_load_metadata) check_good_rc(lasso_server_load_metadata(server, LASSO_PROVIDER_ROLE_IDP, TESTSDATADIR "/renater-metadata.xml", TESTSDATADIR "/metadata-federation-renater.crt", - &blacklisted_1, &loaded_entity_ids)); + &blacklisted_1, &loaded_entity_ids, + LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT)); check_equals(g_hash_table_size(server->providers), 101); check_equals(g_list_length(loaded_entity_ids), 101); + check_good_rc(lasso_server_load_metadata(server, LASSO_PROVIDER_ROLE_IDP, + TESTSDATADIR "/ukfederation-metadata.xml", + TESTSDATADIR "/ukfederation.pem", + &blacklisted_1, &loaded_entity_ids, + LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT)); + check_equals(g_list_length(loaded_entity_ids), 283); + check_equals(g_hash_table_size(server->providers), 384); lasso_release_gobject(server); } @@ -2005,6 +2013,7 @@ basic_suite() tcase_add_test(tc_response_new_from_xmlNode, test11_get_default_name_id_format); tcase_add_test(tc_custom_namespace, test12_custom_namespace); tcase_add_test(tc_load_metadata, test13_test_lasso_server_load_metadata); + tcase_set_timeout(tc_load_metadata, 10); return s; } diff --git a/tests/data/renater-metadata.xml b/tests/data/renater-metadata.xml index 7a9ff7da..1f8bce66 100644 --- a/tests/data/renater-metadata.xml +++ b/tests/data/renater-metadata.xml @@ -12,7 +12,7 @@ -X1YhQCh7ZYc4dN36bWBYviDKxxfishiqMdF3E7PNLYLxCK8wZL58dOvNVnYTJ1CkKa2iFp9Tejyc +1YhQCh7ZYc4dN36bWBYviDKxxfishiqMdF3E7PNLYLxCK8wZL58dOvNVnYTJ1CkKa2iFp9Tejyc 4DRkvzU6vGSGsX2M6k92ON16zanpekHgjFMv4DvtPevRyYHJaeoOzE/6k0Es1kIvbsYMWrVWZdsO XNRvItqOZdBBWNyxXsQ= @@ -27411,4 +27411,4 @@ cHF5 - \ No newline at end of file +