From c2637e437d5b9fa33410856d2059c2921a90fdac Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Fri, 7 Jun 2019 09:21:31 -0400

Subject: [PATCH] Issue 50426 - nsSSL3Ciphers is limited to 1024 characters

Bug Description:  There was a hardcoded buffer for processing TLS ciphers.

                  Anything over 1024 characters was truncated and was not

                  applied.

Fix Description:  Don't use a fixed size buffer and just use the entire

                  string.  When printing errors about invalid format then

                  we must use a fixed sized buffer, but we will truncate

                  that log value as to not exceed the ssl logging function's

                  buffer, and still output a useful message.

ASAN approved

https://pagure.io/389-ds-base/issue/50426

Reviewed by: firstyear, tbordaz, and spichugi (Thanks!!!)

(cherry picked from commit 22f2f9a1502e63bb169b7d599b5a3b35ddb31b8a)

---

 dirsrvtests/tests/suites/tls/cipher_test.py | 51 +++++++++++++++++++++

 ldap/servers/slapd/ssl.c                    | 34 ++++++--------

 2 files changed, 66 insertions(+), 19 deletions(-)

 create mode 100644 dirsrvtests/tests/suites/tls/cipher_test.py

diff --git a/dirsrvtests/tests/suites/tls/cipher_test.py b/dirsrvtests/tests/suites/tls/cipher_test.py

new file mode 100644

index 000000000..058931046

--- /dev/null

+++ b/dirsrvtests/tests/suites/tls/cipher_test.py

@@ -0,0 +1,51 @@

+import pytest

+import os

+from lib389.config import Encryption

+from lib389.topologies import topology_st as topo

+

+

+def test_long_cipher_list(topo):

+    """Test a long cipher list, and makre sure it is not truncated

+

+    :id: bc400f54-3966-49c8-b640-abbf4fb2377d

+    :setup: Standalone Instance

+    :steps:

+        1. Set nsSSL3Ciphers to a very long list of ciphers

+        2. Ciphers are applied correctly

+    :expectedresults:

+        1. Success

+        2. Success

+    """

+    ENABLED_CIPHER = "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384::AES-GCM::AEAD::256"

+    DISABLED_CIPHER = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256::AES-GCM::AEAD::128"

+    CIPHER_LIST = (

+            "-all,-SSL_CK_RC4_128_WITH_MD5,-SSL_CK_RC4_128_EXPORT40_WITH_MD5,-SSL_CK_RC2_128_CBC_WITH_MD5,"

+            "-SSL_CK_RC2_128_CBC_EXPORT40_WITH_MD5,-SSL_CK_DES_64_CBC_WITH_MD5,-SSL_CK_DES_192_EDE3_CBC_WITH_MD5,"

+            "-TLS_RSA_WITH_RC4_128_MD5,-TLS_RSA_WITH_RC4_128_SHA,-TLS_RSA_WITH_3DES_EDE_CBC_SHA,"

+            "-TLS_RSA_WITH_DES_CBC_SHA,-SSL_RSA_FIPS_WITH_3DES_EDE_CBC_SHA,-SSL_RSA_FIPS_WITH_DES_CBC_SHA,"

+            "-TLS_RSA_EXPORT_WITH_RC4_40_MD5,-TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5,-TLS_RSA_WITH_NULL_MD5,"

+            "-TLS_RSA_WITH_NULL_SHA,-TLS_RSA_EXPORT1024_WITH_DES_CBC_SHA,-SSL_FORTEZZA_DMS_WITH_FORTEZZA_CBC_SHA,"

+            "-SSL_FORTEZZA_DMS_WITH_RC4_128_SHA,-SSL_FORTEZZA_DMS_WITH_NULL_SHA,-TLS_DHE_DSS_WITH_DES_CBC_SHA,"

+            "-TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA,-TLS_DHE_RSA_WITH_DES_CBC_SHA,-TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA,"

+            "+TLS_RSA_WITH_AES_128_CBC_SHA,-TLS_DHE_DSS_WITH_AES_128_CBC_SHA,-TLS_DHE_RSA_WITH_AES_128_CBC_SHA,"

+            "+TLS_RSA_WITH_AES_256_CBC_SHA,-TLS_DHE_DSS_WITH_AES_256_CBC_SHA,-TLS_DHE_RSA_WITH_AES_256_CBC_SHA,"

+            "-TLS_RSA_EXPORT1024_WITH_RC4_56_SHA,-TLS_DHE_DSS_WITH_RC4_128_SHA,-TLS_ECDHE_RSA_WITH_RC4_128_SHA,"

+            "-TLS_RSA_WITH_NULL_SHA,-TLS_RSA_EXPORT1024_WITH_DES_CBC_SHA,-SSL_CK_DES_192_EDE3_CBC_WITH_MD5,"

+            "-TLS_RSA_WITH_RC4_128_MD5,-TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,-TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,"

+            "-TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,+TLS_AES_128_GCM_SHA256,+TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"

+        )

+

+    topo.standalone.enable_tls()

+    enc = Encryption(topo.standalone)

+    enc.set('nsSSL3Ciphers', CIPHER_LIST)

+    topo.standalone.restart()

+    enabled_ciphers = enc.get_attr_vals_utf8('nssslenabledciphers')

+    assert ENABLED_CIPHER in enabled_ciphers

+    assert DISABLED_CIPHER not in enabled_ciphers

+

+

+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/ssl.c b/ldap/servers/slapd/ssl.c

index b8eba2da4..ed054db44 100644

--- a/ldap/servers/slapd/ssl.c

+++ b/ldap/servers/slapd/ssl.c

@@ -95,7 +95,6 @@ static char *configDN = "cn=encryption,cn=config";

 #define CIPHER_SET_ALLOWWEAKDHPARAM 0x200    /* allowWeakDhParam is on */

 #define CIPHER_SET_DISALLOWWEAKDHPARAM 0x400 /* allowWeakDhParam is off */

-

 #define CIPHER_SET_ISDEFAULT(flag) \

     (((flag)&CIPHER_SET_DEFAULT) ? PR_TRUE : PR_FALSE)

 #define CIPHER_SET_ISALL(flag) \

@@ -689,10 +688,12 @@ _conf_setciphers(char *setciphers, int flags)

             active = 0;

             break;

         default:

-            PR_snprintf(err, sizeof(err), "invalid ciphers <%s>: format is "

-                                          "+cipher1,-cipher2...",

-                        raw);

-            return slapi_ch_strdup(err);

+            if (strlen(raw) > MAGNUS_ERROR_LEN) {

+                PR_snprintf(err, sizeof(err) - 3, "%s...", raw);

+                return slapi_ch_smprintf("invalid ciphers <%s>: format is +cipher1,-cipher2...", err);

+            } else {

+                return slapi_ch_smprintf("invalid ciphers <%s>: format is +cipher1,-cipher2...", raw);

+            }

         }

         if ((t = strchr(setciphers, ',')))

             *t++ = '\0';

@@ -1689,7 +1690,6 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)

     PRUint16 NSSVersionMax = enabledNSSVersions.max;

     char mymin[VERSION_STR_LENGTH], mymax[VERSION_STR_LENGTH];

     char newmax[VERSION_STR_LENGTH];

-    char cipher_string[1024];

     int allowweakcipher = CIPHER_SET_DEFAULTWEAKCIPHER;

     int_fast16_t renegotiation = (int_fast16_t)SSL_RENEGOTIATE_REQUIRES_XTN;

@@ -1730,21 +1730,17 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)

                            "Ignoring it and set it to default.", val, configDN);

         }

     }

-    slapi_ch_free((void **)&val;;

+    slapi_ch_free_string(&val;;

     /* Set SSL cipher preferences */

-    *cipher_string = 0;

-    if (ciphers && (*ciphers) && PL_strcmp(ciphers, "blank"))

-        PL_strncpyz(cipher_string, ciphers, sizeof(cipher_string));

-    slapi_ch_free((void **)&ciphers);

-

-    if (NULL != (val = _conf_setciphers(cipher_string, allowweakcipher))) {

+    if (NULL != (val = _conf_setciphers(ciphers, allowweakcipher))) {

         errorCode = PR_GetError();

         slapd_SSL_warn("Failed to set SSL cipher "

                        "preference information: %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",

                        val, errorCode, slapd_pr_strerror(errorCode));

-        slapi_ch_free((void **)&val;;

+        slapi_ch_free_string(&val;;

     }

+    slapi_ch_free_string(&ciphers);

     freeConfigEntry(&e);

     /* Import pr fd into SSL */

@@ -1815,12 +1811,12 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)

             activation = slapi_entry_attr_get_charptr(e, "nssslactivation");

             if ((!activation) || (!PL_strcasecmp(activation, "off"))) {

                 /* this family was turned off, goto next */

-                slapi_ch_free((void **)&activation);

+                slapi_ch_free_string(&activation);

                 freeConfigEntry(&e);

                 continue;

             }

-            slapi_ch_free((void **)&activation);

+            slapi_ch_free_string(&activation);

             token = slapi_entry_attr_get_charptr(e, "nsssltoken");

             personality = slapi_entry_attr_get_charptr(e, "nssslpersonalityssl");

@@ -1837,8 +1833,8 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)

                                "family information. Missing nsssltoken or"

                                "nssslpersonalityssl in %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",

                                *family, errorCode, slapd_pr_strerror(errorCode));

-                slapi_ch_free((void **)&token);

-                slapi_ch_free((void **)&personality);

+                slapi_ch_free_string(&token);

+                slapi_ch_free_string(&personality);

                 freeConfigEntry(&e);

                 continue;

             }

@@ -1865,7 +1861,7 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)

                                "private key for cert %s of family %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",

                                cert_name, *family,

                                errorCode, slapd_pr_strerror(errorCode));

-                slapi_ch_free((void **)&personality);

+                slapi_ch_free_string(&personality);

                 CERT_DestroyCertificate(cert);

                 cert = NULL;

                 freeConfigEntry(&e);

-- 

2.17.2