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