From cba77de52e85b67ba112d4a7df92acca1372b72f Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: May 20 2020 12:04:02 +0000 Subject: Issue 51086 - Improve dscreate instance name validation Bug Description: When creating an instance using dscreate, it doesn't enforce max name length. The ldapi socket name contains name of the instance. If it's too long, we can hit limits, and the file name will be truncated. Also, it doesn't sanitize the instance name, it's possible to create an instance with non-ascii symbols in its name. Fix Description: Add more checks to 'dscreate from-file' installation. Add a limitation for nsslapd-ldapifilepath string lenght because it is limited by sizeof((*ports_info.i_listenaddr)->local.path)) it is copied to. https://pagure.io/389-ds-base/issue/51086 Reviewed by: firstyear, mreynolds (Thanks!) --- diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c index 7810389..96d9106 100644 --- a/ldap/servers/slapd/libglobs.c +++ b/ldap/servers/slapd/libglobs.c @@ -2330,11 +2330,23 @@ config_set_ldapi_filename(const char *attrname, char *value, char *errorbuf, int { int retVal = LDAP_SUCCESS; slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); + /* + * LDAPI file path length is limited by sizeof((*ports_info.i_listenaddr)->local.path)) + * which is set in main.c inside of "#if defined(ENABLE_LDAPI)" block + * ports_info.i_listenaddr is sizeof(PRNetAddr) and our required sizes is 8 bytes less + */ + size_t result_size = sizeof(PRNetAddr) - 8; if (config_value_is_null(attrname, value, errorbuf, 0)) { return LDAP_OPERATIONS_ERROR; } + if (strlen(value) >= result_size) { + slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, "%s: \"%s\" is invalid, its length must be less than %d", + attrname, value, result_size); + return LDAP_OPERATIONS_ERROR; + } + if (apply) { CFG_LOCK_WRITE(slapdFrontendConfig); diff --git a/src/cockpit/389-console/src/ds.jsx b/src/cockpit/389-console/src/ds.jsx index 90d9e5a..53aa5cb 100644 --- a/src/cockpit/389-console/src/ds.jsx +++ b/src/cockpit/389-console/src/ds.jsx @@ -793,10 +793,14 @@ class CreateInstanceModal extends React.Component { return; } newServerId = newServerId.replace(/^slapd-/i, ""); // strip "slapd-" - if (newServerId.length > 128) { + if (newServerId === "admin") { + addNotification("warning", "Instance Name 'admin' is reserved, please choose a different name"); + return; + } + if (newServerId.length > 80) { addNotification( "warning", - "Instance name is too long, it must not exceed 128 characters" + "Instance name is too long, it must not exceed 80 characters" ); return; } diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py index 8217557..2aca136 100644 --- a/src/lib389/lib389/instance/setup.py +++ b/src/lib389/lib389/instance/setup.py @@ -566,6 +566,15 @@ class SetupDs(object): # We need to know the prefix before we can do the instance checks assert_c(slapd['instance_name'] is not None, "Configuration instance_name in section [slapd] not found") + assert_c(len(slapd['instance_name']) <= 80, "Server identifier should not be longer than 80 symbols") + assert_c(all(ord(c) < 128 for c in slapd['instance_name']), "Server identifier can not contain non ascii characters") + assert_c(' ' not in slapd['instance_name'], "Server identifier can not contain a space") + assert_c(slapd['instance_name'] != 'admin', "Server identifier \"admin\" is reserved, please choose a different identifier") + + # Check that valid characters are used + safe = re.compile(r'^[#%:\w@_-]+$').search + assert_c(bool(safe(slapd['instance_name'])), "Server identifier has invalid characters, please choose a different value") + # Check if the instance exists or not. # Should I move this import? I think this prevents some recursion from lib389 import DirSrv