From 4c5a4316b7933e9d65639430044ecdf325ef995a Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Sep 17 2019 13:21:05 +0000 Subject: Ticket 50581 - ns-slapd crashes during ldapi search Bug Description: Using ldapi, if the length of the socket file path exceeds 46 bytes it triggers a buffer overflow while reseting a connection. Reset happens at open/close/error. Fix Description: Use a buffer sized for a PRNetAddr.local.path (~100bytes) Use of MAXPATHLEN (4kb) is too much. https://pagure.io/389-ds-base/issue/50581 Reviewed by: William Brown, Alexander Bokovoy, Mark Reynolds, Simon Pichugi Platforms tested: F30 (thanks !!) Flag Day: no Doc impact: no --- diff --git a/dirsrvtests/tests/suites/basic/basic_test.py b/dirsrvtests/tests/suites/basic/basic_test.py index 8a51f9c..662357f 100644 --- a/dirsrvtests/tests/suites/basic/basic_test.py +++ b/dirsrvtests/tests/suites/basic/basic_test.py @@ -12,6 +12,7 @@ """ from subprocess import check_output, Popen +from lib389 import DirSrv from lib389.idm.user import UserAccounts import pytest from lib389.tasks import * @@ -24,6 +25,9 @@ from lib389.topologies import topology_st from lib389.paths import Paths from lib389.idm.directorymanager import DirectoryManager from lib389.config import LDBMConfig +from lib389.dseldif import DSEldif +from lib389.rootdse import RootDSE + pytestmark = pytest.mark.tier0 @@ -1270,6 +1274,109 @@ sample_entries = yes request.addfinalizer(fin) +@pytest.fixture(scope="module") +def dscreate_ldapi_instance(request): + template_file = "/tmp/dssetup.inf" + longname_serverid = "test_longname_deadbeef_deadbeef_deadbeef_deadbeef_deadbeef" + template_text = """[general] +config_version = 2 +# This invalid hostname ... +full_machine_name = localhost.localdomain +# Means we absolutely require this. +strict_host_checking = False +# In tests, we can be run in containers, NEVER trust +# that systemd is there, or functional in any capacity +systemd = False + +[slapd] +instance_name = %s +root_dn = cn=directory manager +root_password = someLongPassword_123 +# We do not have access to high ports in containers, +# so default to something higher. +port = 38999 +secure_port = 63699 + + +[backend-userroot] +suffix = dc=example,dc=com +sample_entries = yes +""" % longname_serverid + + with open(template_file, "w") as template_fd: + template_fd.write(template_text) + + # Unset PYTHONPATH to avoid mixing old CLI tools and new lib389 + tmp_env = os.environ + if "PYTHONPATH" in tmp_env: + del tmp_env["PYTHONPATH"] + try: + subprocess.check_call([ + 'dscreate', + 'from-file', + template_file + ], env=tmp_env) + except subprocess.CalledProcessError as e: + log.fatal("dscreate failed! Error ({}) {}".format(e.returncode, e.output)) + assert False + + inst = DirSrv(verbose=True, external_log=log) + dse_ldif = DSEldif(inst, + serverid=longname_serverid) + + socket_path = dse_ldif.get("cn=config", "nsslapd-ldapifilepath") + inst.local_simple_allocate( + serverid=longname_serverid, + ldapuri=f"ldapi://{socket_path[0].replace('/', '%2f')}", + password="someLongPassword_123" + ) + inst.ldapi_enabled = 'on' + inst.ldapi_socket = socket_path + inst.ldapi_autobind = 'off' + try: + inst.open() + except: + log.fatal("Failed to connect via ldapi to %s instance" % longname_serverid) + os.remove(template_file) + try: + subprocess.check_call(['dsctl', longname_serverid, 'remove', '--do-it']) + except subprocess.CalledProcessError as e: + log.fatal("Failed to remove test instance Error ({}) {}".format(e.returncode, e.output)) + + def fin(): + os.remove(template_file) + try: + subprocess.check_call(['dsctl', longname_serverid, 'remove', '--do-it']) + except subprocess.CalledProcessError as e: + log.fatal("Failed to remove test instance Error ({}) {}".format(e.returncode, e.output)) + + request.addfinalizer(fin) + + return inst + + +@pytest.mark.skipif(not get_user_is_root() or not default_paths.perl_enabled or ds_is_older('1.4.0.0'), + reason="This test is only required with new admin cli, and requires root.") +@pytest.mark.bz1748016 +@pytest.mark.ds50581 +def test_dscreate_longname(dscreate_ldapi_instance): + """Test that an instance with a long name can + handle ldapi connection using a long socket name + + :id: 5d72d955-aff8-4741-8c9a-32c1c707cf1f + :setup: None + :steps: + 1. create an instance with a long serverId name, that open a ldapi connection + 2. Connect with ldapi, that hit 50581 and crash the instance + :expectedresults: + 1. Should succeeds + 2. Should succeeds + """ + + root_dse = RootDSE(dscreate_ldapi_instance) + log.info(root_dse.get_supported_ctrls()) + + if __name__ == '__main__': # Run isolated # -s for DEBUG mode diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c index e6ce0f0..3600d3d 100644 --- a/ldap/servers/slapd/connection.c +++ b/ldap/servers/slapd/connection.c @@ -275,6 +275,8 @@ connection_reset(Connection *conn, int ns, PRNetAddr *from, int fromLen __attrib { char *pTmp = is_SSL ? "SSL " : ""; char *str_ip = NULL, *str_destip; + char buf_ldapi[sizeof(from->local.path) + 1] = {0}; + char buf_destldapi[sizeof(from->local.path) + 1] = {0}; char buf_ip[INET6_ADDRSTRLEN + 1] = {0}; char buf_destip[INET6_ADDRSTRLEN + 1] = {0}; char *str_unknown = "unknown"; @@ -296,18 +298,18 @@ connection_reset(Connection *conn, int ns, PRNetAddr *from, int fromLen __attrib slapi_ch_free((void **)&conn->cin_addr); /* just to be conservative */ if (from->raw.family == PR_AF_LOCAL) { /* ldapi */ conn->cin_addr = (PRNetAddr *)slapi_ch_malloc(sizeof(PRNetAddr)); - PL_strncpyz(buf_ip, from->local.path, sizeof(from->local.path)); + PL_strncpyz(buf_ldapi, from->local.path, sizeof(from->local.path)); memcpy(conn->cin_addr, from, sizeof(PRNetAddr)); - if (!buf_ip[0]) { + if (!buf_ldapi[0]) { PR_GetPeerName(conn->c_prfd, from); - PL_strncpyz(buf_ip, from->local.path, sizeof(from->local.path)); + PL_strncpyz(buf_ldapi, from->local.path, sizeof(from->local.path)); memcpy(conn->cin_addr, from, sizeof(PRNetAddr)); - if (!buf_ip[0]) { + if (!buf_ldapi[0]) { /* Cannot derive local address, need something for logging */ - PL_strncpyz(buf_ip, "local", sizeof(buf_ip)); + PL_strncpyz(buf_ldapi, "local", sizeof(buf_ldapi)); } } - str_ip = buf_ip; + str_ip = buf_ldapi; } else if (((from->ipv6.ip.pr_s6_addr32[0] != 0) || /* from contains non zeros */ (from->ipv6.ip.pr_s6_addr32[1] != 0) || (from->ipv6.ip.pr_s6_addr32[2] != 0) || @@ -362,21 +364,24 @@ connection_reset(Connection *conn, int ns, PRNetAddr *from, int fromLen __attrib memset(conn->cin_destaddr, 0, sizeof(PRNetAddr)); if (PR_GetSockName(conn->c_prfd, conn->cin_destaddr) == 0) { if (conn->cin_destaddr->raw.family == PR_AF_LOCAL) { /* ldapi */ - PL_strncpyz(buf_destip, conn->cin_destaddr->local.path, + PL_strncpyz(buf_destldapi, conn->cin_destaddr->local.path, sizeof(conn->cin_destaddr->local.path)); - if (!buf_destip[0]) { - PL_strncpyz(buf_destip, "unknown local file", sizeof(buf_destip)); + if (!buf_destldapi[0]) { + PL_strncpyz(buf_destldapi, "unknown local file", sizeof(buf_destldapi)); } - } else if (PR_IsNetAddrType(conn->cin_destaddr, PR_IpAddrV4Mapped)) { - PRNetAddr v4destaddr = {{0}}; - v4destaddr.inet.family = PR_AF_INET; - v4destaddr.inet.ip = conn->cin_destaddr->ipv6.ip.pr_s6_addr32[3]; - PR_NetAddrToString(&v4destaddr, buf_destip, sizeof(buf_destip)); + str_destip = buf_destldapi; } else { - PR_NetAddrToString(conn->cin_destaddr, buf_destip, sizeof(buf_destip)); + if (PR_IsNetAddrType(conn->cin_destaddr, PR_IpAddrV4Mapped)) { + PRNetAddr v4destaddr = {{0}}; + v4destaddr.inet.family = PR_AF_INET; + v4destaddr.inet.ip = conn->cin_destaddr->ipv6.ip.pr_s6_addr32[3]; + PR_NetAddrToString(&v4destaddr, buf_destip, sizeof (buf_destip)); + } else { + PR_NetAddrToString(conn->cin_destaddr, buf_destip, sizeof (buf_destip)); + } + buf_destip[sizeof (buf_destip) - 1] = '\0'; + str_destip = buf_destip; } - buf_destip[sizeof(buf_destip) - 1] = '\0'; - str_destip = buf_destip; } else { str_destip = str_unknown; }