diff --git a/src/db/sysdb_services.c b/src/db/sysdb_services.c index 81293699bb3..40766bf0f0e 100644 --- a/src/db/sysdb_services.c +++ b/src/db/sysdb_services.c @@ -139,10 +139,10 @@ sysdb_getservbyport(TALLOC_CTX *mem_ctx, goto done; } - ret = sysdb_search_services(mem_ctx, domain, subfilter, + ret = sysdb_search_services(tmp_ctx, domain, subfilter, attrs, &msgs_count, &msgs); if (ret == EOK) { - res = talloc_zero(mem_ctx, struct ldb_result); + res = talloc_zero(tmp_ctx, struct ldb_result); if (!res) { ret = ENOMEM; goto done; @@ -151,8 +151,7 @@ sysdb_getservbyport(TALLOC_CTX *mem_ctx, res->msgs = talloc_steal(res, msgs); } - *_res = res; - + *_res = talloc_move(mem_ctx, &res); done: talloc_free(tmp_ctx); @@ -194,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 @@ -248,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. */ @@ -684,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/responder/common/cache_req/plugins/cache_req_svc_by_port.c b/src/responder/common/cache_req/plugins/cache_req_svc_by_port.c index 4c005df3972..108b64a18b2 100644 --- a/src/responder/common/cache_req/plugins/cache_req_svc_by_port.c +++ b/src/responder/common/cache_req/plugins/cache_req_svc_by_port.c @@ -89,8 +89,22 @@ cache_req_svc_by_port_lookup(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, struct ldb_result **_result) { - return sysdb_getservbyport(mem_ctx, domain, data->svc.port, - data->svc.protocol.lookup, _result); + struct ldb_result *res = NULL; + errno_t ret; + + ret = sysdb_getservbyport(mem_ctx, domain, data->svc.port, + data->svc.protocol.lookup, &res); + + /* RFC6335 Section 5 allows more than one service associated with a + * particular transport protocol and port. In such case return only + * the first result. */ + if (ret == EOK && res->count > 1) { + res->count = 1; + } + + *_result = talloc_move(mem_ctx, &res); + + return ret; } static struct tevent_req * @@ -111,7 +125,7 @@ const struct cache_req_plugin cache_req_svc_by_port = { .parse_name = false, .ignore_default_domain = false, .bypass_cache = false, - .only_one_result = false, + .only_one_result = true, .search_all_domains = false, .require_enumeration = false, .allow_missing_fqn = false, diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index 2a56f3d9cb5..e5d1af43ac8 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -171,6 +171,24 @@ def ip_net(base_dn, name, address, aliases=()): return ("cn=" + name + ",ou=Networks," + base_dn, attr_list) +def ip_service(base_dn, name, proto, port, aliases=()): + """ + Generate an RFC2307 ipService add-modlist for passing to ldap.add*. + """ + attr_list = [ + ('objectClass', [b'top', b'ipService']), + ('ipServicePort', [str(port).encode('utf-8')]), + ('ipServiceProtocol', [proto.encode('utf-8')]), + ] + if (len(aliases)) > 0: + alias_list = [alias.encode('utf-8') for alias in aliases] + alias_list.insert(0, name.encode('utf-8')) + attr_list.append(('cn', alias_list)) + else: + attr_list.append(('cn', [name.encode('utf-8')])) + return ("cn=" + name + ",ou=Services," + base_dn, attr_list) + + class List(list): """LDAP add-modlist list""" @@ -233,3 +251,8 @@ def add_ipnet(self, name, address, aliases=[], base_dn=None): """Add an RFC2307 ipNetwork add-modlist.""" self.append(ip_net(base_dn or self.base_dn, name, address, aliases)) + + def add_service(self, name, proto, port, aliases=[], base_dn=None): + """Add an RFC2307 ipService add-modlist.""" + self.append(ip_service(base_dn or self.base_dn, + name, proto, port, aliases)) diff --git a/src/tests/intg/sssd_services.py b/src/tests/intg/sssd_services.py new file mode 100644 index 00000000000..e114cc3766e --- /dev/null +++ b/src/tests/intg/sssd_services.py @@ -0,0 +1,163 @@ +# +# Module for simulation of utility "getent services -s sss" from coreutils +# +# Authors: +# Samuel Cabrero +# +# Copyright (C) 2025 SUSE LINUX GmbH, Nuernberg, Germany. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from ctypes import ( + c_int, + c_char_p, + c_ulong, + POINTER, + Structure, + create_string_buffer, +) +from sssd_nss import NssReturnCode, SssdNssError, nss_sss_ctypes_loader +import socket + +SERVICE_BUFLEN = 1024 + + +# struct servent from netdb.h +class Servent(Structure): + _fields_ = [ + ("s_name", c_char_p), + ("s_aliases", POINTER(c_char_p)), + ("s_port", c_int), + ("s_proto", c_char_p), + ] + + +def getservbyname_r(name, proto, result_p, buffer_p, buflen): + """ + ctypes wrapper for: + enum nss_status _nss_sss_getservbyname_r(const char *name, + const char *protocol, + struct servent *result, + char *buffer, size_t buflen, + int *errnop) + """ + func = nss_sss_ctypes_loader("_nss_sss_getservbyname_r") + func.restype = c_int + func.argtypes = [ + c_char_p, + c_char_p, + POINTER(Servent), + c_char_p, + c_ulong, + POINTER(c_int), + ] + + errno = POINTER(c_int)(c_int(0)) + + name = name.encode("utf-8") + proto = proto.encode("utf-8") + res = func(c_char_p(name), c_char_p(proto), result_p, buffer_p, buflen, errno) + + return (int(res), int(errno[0]), result_p) + + +def getservbyport_r(port, proto, result_p, buffer_p, buflen): + """ + ctypes wrapper for: + enum nss_status _nss_sss_getservbyport_r(int port, const char *protocol, + struct servent *result, + char *buffer, size_t buflen, + int *errnop) + """ + func = nss_sss_ctypes_loader("_nss_sss_getservbyport_r") + func.restype = c_int + func.argtypes = [ + c_int, + c_char_p, + POINTER(Servent), + c_char_p, + c_ulong, + POINTER(c_int), + ] + + errno = POINTER(c_int)(c_int(0)) + + port = socket.htons(port) + proto = proto.encode("utf-8") + res = func(port, c_char_p(proto), result_p, buffer_p, buflen, errno) + + return (int(res), int(errno[0]), result_p) + + +def set_servent_dict(res, result_p): + if res != NssReturnCode.SUCCESS: + return dict() + + servent_dict = dict() + servent_dict["name"] = result_p[0].s_name.decode("utf-8") + servent_dict["aliases"] = list() + servent_dict["port"] = result_p[0].s_port + servent_dict["proto"] = result_p[0].s_proto + + i = 0 + while result_p[0].s_aliases[i] is not None: + alias = result_p[0].s_aliases[i].decode("utf-8") + servent_dict["aliases"].append(alias) + i = i + 1 + + return servent_dict + + +def call_sssd_getservbyname(name, proto): + """ + A Python wrapper to retrieve a service by name and protocol. Returns: + (res, servent_dict) + if res is NssReturnCode.SUCCESS, then servent_dict contains the keys + corresponding to the C servent structure fields. Otherwise, the dictionary + is empty and errno indicates the error code + """ + result = Servent() + result_p = POINTER(Servent)(result) + buff = create_string_buffer(SERVICE_BUFLEN) + + (res, errno, result_p) = getservbyname_r( + name, proto, result_p, buff, SERVICE_BUFLEN + ) + if errno != 0: + raise SssdNssError(errno, "getservbyname_r") + + servent_dict = set_servent_dict(res, result_p) + return (res, servent_dict) + + +def call_sssd_getservbyport(port, proto): + """ + A Python wrapper to retrieve a service by port and protocol. Returns: + (res, servent_dict) + if res is NssReturnCode.SUCCESS, then servent_dict contains the keys + corresponding to the C servent structure fields. Otherwise, the dictionary + is empty and errno indicates the error code + """ + result = Servent() + result_p = POINTER(Servent)(result) + buff = create_string_buffer(SERVICE_BUFLEN) + + (res, errno, result_p) = getservbyport_r( + port, proto, result_p, buff, SERVICE_BUFLEN + ) + if errno != 0: + raise SssdNssError(errno, "getservbyport_r") + + servent_dict = set_servent_dict(res, result_p) + return (res, servent_dict) diff --git a/src/tests/intg/test_resolver.py b/src/tests/intg/test_resolver.py index 072d41e7bfd..e6e565b2feb 100644 --- a/src/tests/intg/test_resolver.py +++ b/src/tests/intg/test_resolver.py @@ -34,6 +34,7 @@ from sssd_nss import NssReturnCode, HostError from sssd_hosts import call_sssd_gethostbyname from sssd_nets import call_sssd_getnetbyname, call_sssd_getnetbyaddr +from sssd_services import call_sssd_getservbyname, call_sssd_getservbyport LDAP_BASE_DN = "dc=example,dc=com" @@ -240,6 +241,25 @@ def add_nets(request, ldap_conn): return None +@pytest.fixture +def add_services(request, ldap_conn): + ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) + + ent_list.add_service("svc1", "tcp", 11111, aliases=["svc1_alias1", "svc1_alias2"]) + ent_list.add_service( + "svc2_1", "tcp", 22222, aliases=["svc2_1_alias1", "svc2_1_alias2"] + ) + ent_list.add_service( + "svc2_2", "tcp", 22222, aliases=["svc2_2_alias1", "svc2_2_alias2"] + ) + + create_ldap_fixture(request, ldap_conn, ent_list) + conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + return None + + def test_hostbyname(add_hosts): (res, hres, _) = call_sssd_gethostbyname("invalid") @@ -325,3 +345,46 @@ def test_netbyaddr(add_nets): (res, hres, _) = call_sssd_getnetbyaddr("10.2.2.2", socket.AF_UNSPEC) assert res == NssReturnCode.SUCCESS assert hres == 0 + + +def test_servbyname(add_services): + (res, _) = call_sssd_getservbyname("svcX", "tcp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyname("svcX", "udp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyname("svc1", "tcp") + assert res == NssReturnCode.SUCCESS + + (res, _) = call_sssd_getservbyname("svc1", "udp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyname("svc1_alias2", "tcp") + assert res == NssReturnCode.SUCCESS + + (res, _) = call_sssd_getservbyname("svc2_1", "tcp") + assert res == NssReturnCode.SUCCESS + + (res, _) = call_sssd_getservbyname("svc2_2", "tcp") + assert res == NssReturnCode.SUCCESS + + +def test_servbyport(add_services): + (res, _) = call_sssd_getservbyport(1234, "tcp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyport(1234, "udp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyport(11111, "tcp") + assert res == NssReturnCode.SUCCESS + + (res, _) = call_sssd_getservbyport(11111, "udp") + assert res == NssReturnCode.NOTFOUND + + (res, _) = call_sssd_getservbyport(22222, "tcp") + assert res == NssReturnCode.SUCCESS + + (res, _) = call_sssd_getservbyport(22222, "udp") + assert res == NssReturnCode.NOTFOUND diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 8d055203985..45fb9e3a813 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));