From bc9ae5a810b8024e7ab1179f492c425793e0ddcf 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.21.0