Blame SOURCES/0032-Issue-50426-nsSSL3Ciphers-is-limited-to-1024-charact.patch

26521d
From c2637e437d5b9fa33410856d2059c2921a90fdac Mon Sep 17 00:00:00 2001
c04070
From: Mark Reynolds <mreynolds@redhat.com>
c04070
Date: Fri, 7 Jun 2019 09:21:31 -0400
c04070
Subject: [PATCH] Issue 50426 - nsSSL3Ciphers is limited to 1024 characters
c04070
c04070
Bug Description:  There was a hardcoded buffer for processing TLS ciphers.
c04070
                  Anything over 1024 characters was truncated and was not
c04070
                  applied.
c04070
c04070
Fix Description:  Don't use a fixed size buffer and just use the entire
c04070
                  string.  When printing errors about invalid format then
c04070
                  we must use a fixed sized buffer, but we will truncate
c04070
                  that log value as to not exceed the ssl logging function's
c04070
                  buffer, and still output a useful message.
c04070
c04070
ASAN approved
c04070
c04070
https://pagure.io/389-ds-base/issue/50426
c04070
c04070
Reviewed by: firstyear, tbordaz, and spichugi (Thanks!!!)
c04070
c04070
(cherry picked from commit 22f2f9a1502e63bb169b7d599b5a3b35ddb31b8a)
c04070
---
c04070
 dirsrvtests/tests/suites/tls/cipher_test.py | 51 +++++++++++++++++++++
c04070
 ldap/servers/slapd/ssl.c                    | 34 ++++++--------
c04070
 2 files changed, 66 insertions(+), 19 deletions(-)
c04070
 create mode 100644 dirsrvtests/tests/suites/tls/cipher_test.py
c04070
c04070
diff --git a/dirsrvtests/tests/suites/tls/cipher_test.py b/dirsrvtests/tests/suites/tls/cipher_test.py
c04070
new file mode 100644
c04070
index 000000000..058931046
c04070
--- /dev/null
c04070
+++ b/dirsrvtests/tests/suites/tls/cipher_test.py
c04070
@@ -0,0 +1,51 @@
c04070
+import pytest
c04070
+import os
c04070
+from lib389.config import Encryption
c04070
+from lib389.topologies import topology_st as topo
c04070
+
c04070
+
c04070
+def test_long_cipher_list(topo):
c04070
+    """Test a long cipher list, and makre sure it is not truncated
c04070
+
c04070
+    :id: bc400f54-3966-49c8-b640-abbf4fb2377d
c04070
+    :setup: Standalone Instance
c04070
+    :steps:
c04070
+        1. Set nsSSL3Ciphers to a very long list of ciphers
c04070
+        2. Ciphers are applied correctly
c04070
+    :expectedresults:
c04070
+        1. Success
c04070
+        2. Success
c04070
+    """
c04070
+    ENABLED_CIPHER = "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384::AES-GCM::AEAD::256"
c04070
+    DISABLED_CIPHER = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256::AES-GCM::AEAD::128"
c04070
+    CIPHER_LIST = (
c04070
+            "-all,-SSL_CK_RC4_128_WITH_MD5,-SSL_CK_RC4_128_EXPORT40_WITH_MD5,-SSL_CK_RC2_128_CBC_WITH_MD5,"
c04070
+            "-SSL_CK_RC2_128_CBC_EXPORT40_WITH_MD5,-SSL_CK_DES_64_CBC_WITH_MD5,-SSL_CK_DES_192_EDE3_CBC_WITH_MD5,"
c04070
+            "-TLS_RSA_WITH_RC4_128_MD5,-TLS_RSA_WITH_RC4_128_SHA,-TLS_RSA_WITH_3DES_EDE_CBC_SHA,"
c04070
+            "-TLS_RSA_WITH_DES_CBC_SHA,-SSL_RSA_FIPS_WITH_3DES_EDE_CBC_SHA,-SSL_RSA_FIPS_WITH_DES_CBC_SHA,"
c04070
+            "-TLS_RSA_EXPORT_WITH_RC4_40_MD5,-TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5,-TLS_RSA_WITH_NULL_MD5,"
c04070
+            "-TLS_RSA_WITH_NULL_SHA,-TLS_RSA_EXPORT1024_WITH_DES_CBC_SHA,-SSL_FORTEZZA_DMS_WITH_FORTEZZA_CBC_SHA,"
c04070
+            "-SSL_FORTEZZA_DMS_WITH_RC4_128_SHA,-SSL_FORTEZZA_DMS_WITH_NULL_SHA,-TLS_DHE_DSS_WITH_DES_CBC_SHA,"
c04070
+            "-TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA,-TLS_DHE_RSA_WITH_DES_CBC_SHA,-TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA,"
c04070
+            "+TLS_RSA_WITH_AES_128_CBC_SHA,-TLS_DHE_DSS_WITH_AES_128_CBC_SHA,-TLS_DHE_RSA_WITH_AES_128_CBC_SHA,"
c04070
+            "+TLS_RSA_WITH_AES_256_CBC_SHA,-TLS_DHE_DSS_WITH_AES_256_CBC_SHA,-TLS_DHE_RSA_WITH_AES_256_CBC_SHA,"
c04070
+            "-TLS_RSA_EXPORT1024_WITH_RC4_56_SHA,-TLS_DHE_DSS_WITH_RC4_128_SHA,-TLS_ECDHE_RSA_WITH_RC4_128_SHA,"
c04070
+            "-TLS_RSA_WITH_NULL_SHA,-TLS_RSA_EXPORT1024_WITH_DES_CBC_SHA,-SSL_CK_DES_192_EDE3_CBC_WITH_MD5,"
c04070
+            "-TLS_RSA_WITH_RC4_128_MD5,-TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,-TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,"
c04070
+            "-TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,+TLS_AES_128_GCM_SHA256,+TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"
c04070
+        )
c04070
+
c04070
+    topo.standalone.enable_tls()
c04070
+    enc = Encryption(topo.standalone)
c04070
+    enc.set('nsSSL3Ciphers', CIPHER_LIST)
c04070
+    topo.standalone.restart()
c04070
+    enabled_ciphers = enc.get_attr_vals_utf8('nssslenabledciphers')
c04070
+    assert ENABLED_CIPHER in enabled_ciphers
c04070
+    assert DISABLED_CIPHER not in enabled_ciphers
c04070
+
c04070
+
c04070
+if __name__ == '__main__':
c04070
+    # Run isolated
c04070
+    # -s for DEBUG mode
c04070
+    CURRENT_FILE = os.path.realpath(__file__)
c04070
+    pytest.main(["-s", CURRENT_FILE])
c04070
diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c
c04070
index b8eba2da4..ed054db44 100644
c04070
--- a/ldap/servers/slapd/ssl.c
c04070
+++ b/ldap/servers/slapd/ssl.c
c04070
@@ -95,7 +95,6 @@ static char *configDN = "cn=encryption,cn=config";
c04070
 #define CIPHER_SET_ALLOWWEAKDHPARAM 0x200    /* allowWeakDhParam is on */
c04070
 #define CIPHER_SET_DISALLOWWEAKDHPARAM 0x400 /* allowWeakDhParam is off */
c04070
 
c04070
-
c04070
 #define CIPHER_SET_ISDEFAULT(flag) \
c04070
     (((flag)&CIPHER_SET_DEFAULT) ? PR_TRUE : PR_FALSE)
c04070
 #define CIPHER_SET_ISALL(flag) \
c04070
@@ -689,10 +688,12 @@ _conf_setciphers(char *setciphers, int flags)
c04070
             active = 0;
c04070
             break;
c04070
         default:
c04070
-            PR_snprintf(err, sizeof(err), "invalid ciphers <%s>: format is "
c04070
-                                          "+cipher1,-cipher2...",
c04070
-                        raw);
c04070
-            return slapi_ch_strdup(err);
c04070
+            if (strlen(raw) > MAGNUS_ERROR_LEN) {
c04070
+                PR_snprintf(err, sizeof(err) - 3, "%s...", raw);
c04070
+                return slapi_ch_smprintf("invalid ciphers <%s>: format is +cipher1,-cipher2...", err);
c04070
+            } else {
c04070
+                return slapi_ch_smprintf("invalid ciphers <%s>: format is +cipher1,-cipher2...", raw);
c04070
+            }
c04070
         }
c04070
         if ((t = strchr(setciphers, ',')))
c04070
             *t++ = '\0';
c04070
@@ -1689,7 +1690,6 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)
c04070
     PRUint16 NSSVersionMax = enabledNSSVersions.max;
c04070
     char mymin[VERSION_STR_LENGTH], mymax[VERSION_STR_LENGTH];
c04070
     char newmax[VERSION_STR_LENGTH];
c04070
-    char cipher_string[1024];
c04070
     int allowweakcipher = CIPHER_SET_DEFAULTWEAKCIPHER;
c04070
     int_fast16_t renegotiation = (int_fast16_t)SSL_RENEGOTIATE_REQUIRES_XTN;
c04070
 
c04070
@@ -1730,21 +1730,17 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)
c04070
                            "Ignoring it and set it to default.", val, configDN);
c04070
         }
c04070
     }
c04070
-    slapi_ch_free((void **)&val);
c04070
+    slapi_ch_free_string(&val);
c04070
 
c04070
     /* Set SSL cipher preferences */
c04070
-    *cipher_string = 0;
c04070
-    if (ciphers && (*ciphers) && PL_strcmp(ciphers, "blank"))
c04070
-        PL_strncpyz(cipher_string, ciphers, sizeof(cipher_string));
c04070
-    slapi_ch_free((void **)&ciphers);
c04070
-
c04070
-    if (NULL != (val = _conf_setciphers(cipher_string, allowweakcipher))) {
c04070
+    if (NULL != (val = _conf_setciphers(ciphers, allowweakcipher))) {
c04070
         errorCode = PR_GetError();
c04070
         slapd_SSL_warn("Failed to set SSL cipher "
c04070
                        "preference information: %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",
c04070
                        val, errorCode, slapd_pr_strerror(errorCode));
c04070
-        slapi_ch_free((void **)&val);
c04070
+        slapi_ch_free_string(&val);
c04070
     }
c04070
+    slapi_ch_free_string(&ciphers);
c04070
     freeConfigEntry(&e);
c04070
 
c04070
     /* Import pr fd into SSL */
c04070
@@ -1815,12 +1811,12 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)
c04070
             activation = slapi_entry_attr_get_charptr(e, "nssslactivation");
c04070
             if ((!activation) || (!PL_strcasecmp(activation, "off"))) {
c04070
                 /* this family was turned off, goto next */
c04070
-                slapi_ch_free((void **)&activation);
c04070
+                slapi_ch_free_string(&activation);
c04070
                 freeConfigEntry(&e);
c04070
                 continue;
c04070
             }
c04070
 
c04070
-            slapi_ch_free((void **)&activation);
c04070
+            slapi_ch_free_string(&activation);
c04070
 
c04070
             token = slapi_entry_attr_get_charptr(e, "nsssltoken");
c04070
             personality = slapi_entry_attr_get_charptr(e, "nssslpersonalityssl");
c04070
@@ -1837,8 +1833,8 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)
c04070
                                "family information. Missing nsssltoken or"
c04070
                                "nssslpersonalityssl in %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",
c04070
                                *family, errorCode, slapd_pr_strerror(errorCode));
c04070
-                slapi_ch_free((void **)&token);
c04070
-                slapi_ch_free((void **)&personality);
c04070
+                slapi_ch_free_string(&token);
c04070
+                slapi_ch_free_string(&personality);
c04070
                 freeConfigEntry(&e);
c04070
                 continue;
c04070
             }
c04070
@@ -1865,7 +1861,7 @@ slapd_ssl_init2(PRFileDesc **fd, int startTLS)
c04070
                                "private key for cert %s of family %s (" SLAPI_COMPONENT_NAME_NSPR " error %d - %s)",
c04070
                                cert_name, *family,
c04070
                                errorCode, slapd_pr_strerror(errorCode));
c04070
-                slapi_ch_free((void **)&personality);
c04070
+                slapi_ch_free_string(&personality);
c04070
                 CERT_DestroyCertificate(cert);
c04070
                 cert = NULL;
c04070
                 freeConfigEntry(&e);
c04070
-- 
26521d
2.17.2
c04070