From 03695c416f7f8311afbded390f3c0ff3637a10d4 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 20 May 2019 11:38:05 -0400
Subject: [PATCH] Ticket 50393 - maxlogsperdir accepting negative values
Description: Improve the log "digit" config setting validation
for all settings.
https://pagure.io/389-ds-base/issue/50393
Reviewed by: tbordaz, firstyear, mhonek, and spichugi (Thanks!!!!)
(cherry picked from commit ca70d06fbb7a2c06c62f0ba5b192dba36f24b8e3)
---
dirsrvtests/tests/suites/logging/__init__.py | 3 +
.../suites/logging/logging_config_test.py | 86 +++++++++++
ldap/servers/slapd/log.c | 143 +++++++++++++-----
3 files changed, 192 insertions(+), 40 deletions(-)
create mode 100644 dirsrvtests/tests/suites/logging/__init__.py
create mode 100644 dirsrvtests/tests/suites/logging/logging_config_test.py
diff --git a/dirsrvtests/tests/suites/logging/__init__.py b/dirsrvtests/tests/suites/logging/__init__.py
new file mode 100644
index 000000000..7f812e357
--- /dev/null
+++ b/dirsrvtests/tests/suites/logging/__init__.py
@@ -0,0 +1,3 @@
+"""
+ :Requirement: 389-ds-base: Directory Server Logging Configurations
+"""
diff --git a/dirsrvtests/tests/suites/logging/logging_config_test.py b/dirsrvtests/tests/suites/logging/logging_config_test.py
new file mode 100644
index 000000000..4d8d68ab5
--- /dev/null
+++ b/dirsrvtests/tests/suites/logging/logging_config_test.py
@@ -0,0 +1,86 @@
+import logging
+import pytest
+import os
+import ldap
+from lib389._constants import *
+from lib389.topologies import topology_st as topo
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+ logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+ logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+big_value = "1111111111111111111111111111111111111111111"
+
+
+@pytest.mark.parametrize("attr, invalid_vals, valid_vals",
+ [
+ ("logexpirationtime", ["-2", "0"], ["1", "-1"]),
+ ("maxlogsize", ["-2", "0"], ["100", "-1"]),
+ ("logmaxdiskspace", ["-2", "0"], ["100", "-1"]),
+ ("logminfreediskspace", ["-2", "0"], ["100", "-1"]),
+ ("mode", ["888", "778", "77", "7777"], ["777", "000", "600"]),
+ ("maxlogsperdir", ["-1", "0"], ["1", "20"]),
+ ("logrotationsynchour", ["-1", "24"], ["0", "23"]),
+ ("logrotationsyncmin", ["-1", "60"], ["0", "59"]),
+ ("logrotationtime", ["-2", "0"], ["100", "-1"])
+ ])
+def test_logging_digit_config(topo, attr, invalid_vals, valid_vals):
+ """Validate logging config settings
+
+ :id: a0ef30e5-538b-46fa-9762-01a4435a15e9
+ :setup: Standalone Instance
+ :steps:
+ 1. Test log expiration time
+ 2. Test log max size
+ 3. Test log max disk space
+ 4. Test log min disk space
+ 5. Test log mode
+ 6. Test log max number of logs
+ 7. Test log rotation hour
+ 8. Test log rotation minute
+ 9. Test log rotation time
+ :expectedresults:
+ 1. Success
+ 2. Success
+ 3. Success
+ 4. Success
+ 5. Success
+ 6. Success
+ 7. Success
+ 8. Success
+ 9. Success
+ """
+
+ accesslog_attr = "nsslapd-accesslog-{}".format(attr)
+ auditlog_attr = "nsslapd-auditlog-{}".format(attr)
+ auditfaillog_attr = "nsslapd-auditfaillog-{}".format(attr)
+ errorlog_attr = "nsslapd-errorlog-{}".format(attr)
+
+ # Test each log
+ for attr in [accesslog_attr, auditlog_attr, auditfaillog_attr, errorlog_attr]:
+ # Invalid values
+ for invalid_val in invalid_vals:
+ with pytest.raises(ldap.LDAPError):
+ topo.standalone.config.set(attr, invalid_val)
+
+ # Invalid high value
+ with pytest.raises(ldap.LDAPError):
+ topo.standalone.config.set(attr, big_value)
+
+ # Non digits
+ with pytest.raises(ldap.LDAPError):
+ topo.standalone.config.set(attr, "abc")
+
+ # Valid values
+ for valid_val in valid_vals:
+ topo.standalone.config.set(attr, valid_val)
+
+
+if __name__ == '__main__':
+ # Run isolated
+ # -s for DEBUG mode
+ CURRENT_FILE = os.path.realpath(__file__)
+ pytest.main(["-s", CURRENT_FILE])
diff --git a/ldap/servers/slapd/log.c b/ldap/servers/slapd/log.c
index 7dd71541b..2456abf1e 100644
--- a/ldap/servers/slapd/log.c
+++ b/ldap/servers/slapd/log.c
@@ -817,8 +817,9 @@ log_update_auditfaillogdir(char *pathname, int apply)
int
log_set_mode(const char *attrname, char *value, int logtype, char *errorbuf, int apply)
{
- int v = 0;
+ int64_t v = 0;
int retval = LDAP_SUCCESS;
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (NULL == value) {
@@ -833,7 +834,18 @@ log_set_mode(const char *attrname, char *value, int logtype, char *errorbuf, int
return LDAP_SUCCESS;
}
- v = strtol(value, NULL, 8);
+ errno = 0;
+ v = strtol(value, &endp, 8);
+ if (*endp != '\0' || errno == ERANGE ||
+ strlen(value) != 3 ||
+ v > 0777 /* octet of 777 511 */ ||
+ v < 0)
+ {
+ slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s) (%ld), value must be three digits between 000 and 777",
+ value, attrname, v);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
switch (logtype) {
case SLAPD_ACCESS_LOG:
@@ -895,9 +907,9 @@ int
log_set_numlogsperdir(const char *attrname, char *numlogs_str, int logtype, char *returntext, int apply)
{
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
-
+ char *endp = NULL;
int rv = LDAP_SUCCESS;
- int numlogs;
+ int64_t numlogs;
if (logtype != SLAPD_ACCESS_LOG &&
logtype != SLAPD_ERROR_LOG &&
@@ -911,7 +923,14 @@ log_set_numlogsperdir(const char *attrname, char *numlogs_str, int logtype, char
return rv;
}
- numlogs = atoi(numlogs_str);
+ errno = 0;
+ numlogs = strtol(numlogs_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || numlogs < 1) {
+ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be between 1 and 2147483647",
+ numlogs_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
if (numlogs >= 1) {
switch (logtype) {
@@ -960,21 +979,25 @@ int
log_set_logsize(const char *attrname, char *logsize_str, int logtype, char *returntext, int apply)
{
int rv = LDAP_SUCCESS;
- PRInt64 max_logsize; /* in bytes */
- int logsize; /* in megabytes */
+ int64_t max_logsize; /* in bytes */
+ int64_t logsize; /* in megabytes */
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (!apply || !logsize_str || !*logsize_str)
return rv;
- logsize = atoi(logsize_str);
+ errno = 0;
+ logsize = strtol(logsize_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || logsize < -1 || logsize == 0) {
+ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"-1\" or greater than 0",
+ logsize_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
/* convert it to bytes */
- max_logsize = (PRInt64)logsize * LOG_MB_IN_BYTES;
-
- if (max_logsize <= 0) {
- max_logsize = -1;
- }
+ max_logsize = logsize * LOG_MB_IN_BYTES;
switch (logtype) {
case SLAPD_ACCESS_LOG:
@@ -1101,8 +1124,9 @@ log_set_rotationsync_enabled(const char *attrname, char *value, int logtype, cha
int
log_set_rotationsynchour(const char *attrname, char *rhour_str, int logtype, char *returntext, int apply)
{
- int rhour = -1;
+ int64_t rhour = -1;
int rv = LDAP_SUCCESS;
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1115,12 +1139,19 @@ log_set_rotationsynchour(const char *attrname, char *rhour_str, int logtype, cha
}
/* return if we aren't doing this for real */
- if (!apply) {
+ if (!apply || !rhour_str || !*rhour_str) {
return rv;
}
- if (rhour_str && *rhour_str != '\0')
- rhour = atol(rhour_str);
+ errno = 0;
+ rhour = strtol(rhour_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || rhour < 0 || rhour > 23) {
+ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"0\" thru \"23\"",
+ rhour_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
+
if (rhour > 23)
rhour = rhour % 24;
@@ -1161,8 +1192,9 @@ log_set_rotationsynchour(const char *attrname, char *rhour_str, int logtype, cha
int
log_set_rotationsyncmin(const char *attrname, char *rmin_str, int logtype, char *returntext, int apply)
{
- int rmin = -1;
+ int64_t rmin = -1;
int rv = LDAP_SUCCESS;
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1175,14 +1207,18 @@ log_set_rotationsyncmin(const char *attrname, char *rmin_str, int logtype, char
}
/* return if we aren't doing this for real */
- if (!apply) {
+ if (!apply || !rmin_str || !*rmin_str) {
return rv;
}
- if (rmin_str && *rmin_str != '\0')
- rmin = atol(rmin_str);
- if (rmin > 59)
- rmin = rmin % 60;
+ errno = 0;
+ rmin = strtol(rmin_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || rmin < 0 || rmin > 59) {
+ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be between \"0\" and \"59\"",
+ rmin_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
switch (logtype) {
case SLAPD_ACCESS_LOG:
@@ -1229,8 +1265,9 @@ log_set_rotationtime(const char *attrname, char *rtime_str, int logtype, char *r
{
int runit = 0;
- int value, rtime;
+ int64_t value, rtime;
int rv = LDAP_SUCCESS;
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1247,7 +1284,14 @@ log_set_rotationtime(const char *attrname, char *rtime_str, int logtype, char *r
return rv;
}
- rtime = atoi(rtime_str);
+ errno = 0;
+ rtime = strtol(rtime_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || rtime < -1 || rtime == 0) {
+ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"-1\" or greater than \"0\"",
+ rtime_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
if (0 == rtime) {
rtime = -1; /* Value Range: -1 | 1 to PR_INT32_MAX */
@@ -1332,7 +1376,6 @@ log_set_rotationtimeunit(const char *attrname, char *runit, int logtype, char *e
int origvalue = 0, value = 0;
int runitType;
int rv = 0;
-
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1448,10 +1491,10 @@ int
log_set_maxdiskspace(const char *attrname, char *maxdiskspace_str, int logtype, char *errorbuf, int apply)
{
int rv = 0;
- PRInt64 mlogsize = 0; /* in bytes */
- PRInt64 maxdiskspace; /* in bytes */
- int s_maxdiskspace; /* in megabytes */
-
+ int64_t mlogsize = 0; /* in bytes */
+ int64_t maxdiskspace; /* in bytes */
+ int64_t s_maxdiskspace; /* in megabytes */
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1465,7 +1508,14 @@ log_set_maxdiskspace(const char *attrname, char *maxdiskspace_str, int logtype,
if (!apply || !maxdiskspace_str || !*maxdiskspace_str)
return rv;
- s_maxdiskspace = atoi(maxdiskspace_str);
+ errno = 0;
+ s_maxdiskspace = strtol(maxdiskspace_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || s_maxdiskspace < -1 || s_maxdiskspace == 0) {
+ slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"-1\" or greater than 0",
+ maxdiskspace_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
/* Disk space are in MB but store in bytes */
switch (logtype) {
@@ -1538,9 +1588,9 @@ int
log_set_mindiskspace(const char *attrname, char *minfreespace_str, int logtype, char *errorbuf, int apply)
{
int rv = LDAP_SUCCESS;
- int minfreespace; /* in megabytes */
- PRInt64 minfreespaceB; /* in bytes */
-
+ int64_t minfreespace; /* in megabytes */
+ int64_t minfreespaceB; /* in bytes */
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1556,11 +1606,18 @@ log_set_mindiskspace(const char *attrname, char *minfreespace_str, int logtype,
return rv;
}
- minfreespace = atoi(minfreespace_str);
+ errno = 0;
+ minfreespace = strtol(minfreespace_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || minfreespace < -1 || minfreespace == 0) {
+ slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"-1\" or greater than 0",
+ minfreespace_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
/* Disk space are in MB but store in bytes */
if (minfreespace >= 1) {
- minfreespaceB = (PRInt64)minfreespace * LOG_MB_IN_BYTES;
+ minfreespaceB = minfreespace * LOG_MB_IN_BYTES;
switch (logtype) {
case SLAPD_ACCESS_LOG:
LOG_ACCESS_LOCK_WRITE();
@@ -1602,10 +1659,10 @@ log_set_mindiskspace(const char *attrname, char *minfreespace_str, int logtype,
int
log_set_expirationtime(const char *attrname, char *exptime_str, int logtype, char *errorbuf, int apply)
{
-
- int eunit, value, exptime;
+ int64_t eunit, value, exptime;
int rsec = 0;
int rv = 0;
+ char *endp = NULL;
slapdFrontendConfig_t *fe_cfg = getFrontendConfig();
if (logtype != SLAPD_ACCESS_LOG &&
@@ -1621,7 +1678,14 @@ log_set_expirationtime(const char *attrname, char *exptime_str, int logtype, cha
return rv;
}
- exptime = atoi(exptime_str); /* <= 0: no exptime */
+ errno = 0;
+ exptime = strtol(exptime_str, &endp, 10);
+ if (*endp != '\0' || errno == ERANGE || exptime < -1 || exptime == 0) {
+ slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
+ "Invalid value \"%s\" for attribute (%s), value must be \"-1\" or greater than 0",
+ exptime_str, attrname);
+ return LDAP_UNWILLING_TO_PERFORM;
+ }
switch (logtype) {
case SLAPD_ACCESS_LOG:
@@ -1734,7 +1798,6 @@ log_set_expirationtimeunit(const char *attrname, char *expunit, int logtype, cha
} else {
slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, "%s: invalid time unit \"%s\"", attrname, expunit);
rv = LDAP_OPERATIONS_ERROR;
- ;
}
/* return if we aren't doing this for real */
--
2.17.2