Skip to content

Commit

Permalink
SYSDB: Allow multiple services associated to the same port
Browse files Browse the repository at this point in the history 
RFC6335 section 5 allow multiple services associated to the same
transport and port. This commit allows storing multiple service entries
in the cache for the same port and the lookup functions will return all
matching entries. The cache_req plugin will pick the first result.

Signed-off-by: Samuel Cabrero <[email protected]>
  • Loading branch information
@scabrero
scabrero committed Jan 15, 2025
1 parent 33d3a72 commit 3f32b62
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 61 deletions.
59 changes: 14 additions & 45 deletions src/db/sysdb_services.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand Down
54 changes: 38 additions & 16 deletions src/tests/sysdb-tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 */
Expand All @@ -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));

Expand Down

0 comments on commit 3f32b62

Please sign in to comment.