diff --git a/src/db/sysdb_services.c b/src/db/sysdb_services.c index 9c608415f1..40766bf0f0 100644 --- a/src/db/sysdb_services.c +++ b/src/db/sysdb_services.c @@ -193,51 +193,21 @@ sysdb_store_service(struct sss_domain_info *domain, in_transaction = true; - /* Check that the port is unique - * If the port appears for any service other than - * the one matching the primary_name, we need to - * remove them so that getservbyport() can work - * properly. Last entry saved to the cache should - * always "win". - */ + /* RFC6335 Section 5 allows more than one service associated with a + * particular transport protocol and port. In such case the names + * must be different so check that all entries have a name. */ ret = sysdb_getservbyport(tmp_ctx, domain, port, NULL, &res); if (ret != EOK && ret != ENOENT) { goto done; } else if (ret != ENOENT) { - if (res->count != 1) { - /* Somehow the cache has multiple entries with - * the same port. This is corrupted. We'll delete - * them all to sort it out. - */ - for (i = 0; i < res->count; i++) { - DEBUG(SSSDBG_TRACE_FUNC, - "Corrupt cache entry [%s] detected. Deleting\n", - ldb_dn_canonical_string(tmp_ctx, - res->msgs[i]->dn)); - - ret = sysdb_delete_entry(sysdb, res->msgs[i]->dn, true); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Could not delete corrupt cache entry [%s]\n", - ldb_dn_canonical_string(tmp_ctx, - res->msgs[i]->dn)); - goto done; - } - } - } else { - /* Check whether this is the same name as we're currently - * saving to the cache. - */ - name = ldb_msg_find_attr_as_string(res->msgs[0], + /* Check whether this is the same name as we're currently + * saving to the cache. */ + for (i = 0; i < res->count; i++) { + name = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_NAME, NULL); - if (!name || strcmp(name, primary_name) != 0) { - - if (!name) { - DEBUG(SSSDBG_CRIT_FAILURE, - "A service with no name?\n"); - /* Corrupted */ - } + if (!name) { + DEBUG(SSSDBG_CRIT_FAILURE, "A service with no name?\n"); /* Either this is a corrupt entry or it's another service * claiming ownership of this port. In order to account @@ -247,22 +217,21 @@ sysdb_store_service(struct sss_domain_info *domain, "Corrupt or replaced cache entry [%s] detected. " "Deleting\n", ldb_dn_canonical_string(tmp_ctx, - res->msgs[0]->dn)); + res->msgs[i]->dn)); - ret = sysdb_delete_entry(sysdb, res->msgs[0]->dn, true); + ret = sysdb_delete_entry(sysdb, res->msgs[i]->dn, true); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Could not delete cache entry [%s]\n", ldb_dn_canonical_string(tmp_ctx, - res->msgs[0]->dn)); + res->msgs[i]->dn)); } } } } talloc_zfree(res); - /* Ok, ports should now be unique. Now look - * the service up by name to determine if we + /* Now look the service up by name to determine if we * need to update existing entries or modify * aliases. */ @@ -683,7 +652,7 @@ sysdb_svc_delete(struct sss_domain_info *domain, /* There should only be one matching entry, * but if there are multiple, we should delete - * them all to de-corrupt the DB. + * them all. */ for (i = 0; i < res->count; i++) { ret = sysdb_delete_entry(sysdb, res->msgs[i]->dn, false); diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 8d05520398..45fb9e3a81 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -4880,7 +4880,7 @@ void services_check_match(struct sysdb_test_ctx *test_ctx, const char *ret_name; int ret_port; struct ldb_result *res; - struct ldb_message *msg; + struct ldb_message *msg = NULL; struct ldb_message_element *el; if (by_name) { @@ -4889,18 +4889,29 @@ void services_check_match(struct sysdb_test_ctx *test_ctx, NULL, &res); sss_ck_fail_if_msg(ret != EOK, "sysdb_getservbyname error [%s]\n", strerror(ret)); + sss_ck_fail_if_msg(res == NULL, "ENOMEM"); + ck_assert_int_eq(res->count, 1); + msg = res->msgs[0]; } else { /* Look up the newly-added service by port */ ret = sysdb_getservbyport(test_ctx, test_ctx->domain, port, NULL, &res); sss_ck_fail_if_msg(ret != EOK, "sysdb_getservbyport error [%s]\n", strerror(ret)); + sss_ck_fail_if_msg(res == NULL, "ENOMEM"); + /* Port lookups can return multiple results. Select the correct one */ + for (i = 0; i < res->count; i++) { + ret_name = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_NAME, NULL); + sss_ck_fail_if_msg(ret_name == NULL, "ENOENT"); + if (strcmp(ret_name, primary_name) == 0) { + msg = res->msgs[i]; + break; + } + } } - sss_ck_fail_if_msg(res == NULL, "ENOMEM"); - ck_assert_int_eq(res->count, 1); + sss_ck_fail_if_msg(msg == NULL, "ENOENT"); /* Make sure the returned entry matches */ - msg = res->msgs[0]; ret_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL); sss_ck_fail_if_msg(ret_name == NULL, "Cannot find attribute: " SYSDB_NAME); ck_assert_msg(strcmp(ret_name, primary_name) == 0, @@ -5075,7 +5086,7 @@ START_TEST(test_sysdb_store_services) primary_name, port, aliases, protocols); - /* Change the service name */ + /* Add a second service using the same port */ ret = sysdb_store_service(test_ctx->domain, alt_primary_name, port, aliases, protocols, @@ -5086,34 +5097,42 @@ START_TEST(test_sysdb_store_services) alt_primary_name, port, aliases, protocols); + services_check_match_name(test_ctx, + primary_name, port, + aliases, protocols); + /* Search by port and make sure the results match */ services_check_match_port(test_ctx, - alt_primary_name, port, + primary_name, port, aliases, protocols); - - /* Change it back */ - ret = sysdb_store_service(test_ctx->domain, - primary_name, port, - aliases, protocols, - NULL, NULL, 1, 1); - sss_ck_fail_if_msg(ret != EOK, "[%s]", strerror(ret)); + services_check_match_port(test_ctx, + alt_primary_name, port, + aliases, protocols); /* Change the port number */ ret = sysdb_store_service(test_ctx->domain, - primary_name, altport, + alt_primary_name, altport, aliases, protocols, NULL, NULL, 1, 1); sss_ck_fail_if_msg(ret != EOK, "[%s]", strerror(ret)); /* Search by name and make sure the results match */ services_check_match_name(test_ctx, - primary_name, altport, + alt_primary_name, altport, + aliases, protocols); + + services_check_match_name(test_ctx, + primary_name, port, aliases, protocols); /* Search by port and make sure the results match */ services_check_match_port(test_ctx, - primary_name, altport, + alt_primary_name, altport, + aliases, protocols); + + services_check_match_port(test_ctx, + primary_name, port, aliases, protocols); /* TODO: Test changing aliases and protocols */ @@ -5127,6 +5146,9 @@ START_TEST(test_sysdb_store_services) * doesn't like adding and deleting the same entry in a * single transaction. */ + ret = sysdb_svc_delete(test_ctx->domain, NULL, port, NULL); + sss_ck_fail_if_msg(ret != EOK, "[%s]", strerror(ret)); + ret = sysdb_svc_delete(test_ctx->domain, NULL, altport, NULL); sss_ck_fail_if_msg(ret != EOK, "[%s]", strerror(ret));