From 2840bd0becee307f4ee896b26e9f29baac03c347 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 15 Jun 2020 11:50:16 +0200
Subject: [PATCH 1/2] s3:lib:tls: Use better priority lists for modern GnuTLS
We should use the default priority list. That is a good practice,
because TLS protocol hardening and phasing out of legacy algorithms,
is easier to co-ordinate when happens at a single place. See crypto
policies of Fedora.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14408
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Wed Jun 17 17:42:02 UTC 2020 on sn-devel-184
---
docs-xml/smbdotconf/security/tlspriority.xml | 10 ++---
lib/param/loadparm.c | 10 ++++-
python/samba/tests/docs.py | 20 ++++++++++
source3/param/loadparm.c | 11 +++++-
source4/lib/tls/tls_tstream.c | 40 +++++++++++++++-----
wscript_configure_system_gnutls | 3 ++
6 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/docs-xml/smbdotconf/security/tlspriority.xml b/docs-xml/smbdotconf/security/tlspriority.xml
index d7214a4c1ea..6d1f0dcb912 100644
--- a/docs-xml/smbdotconf/security/tlspriority.xml
+++ b/docs-xml/smbdotconf/security/tlspriority.xml
@@ -7,15 +7,15 @@
to be supported in the parts of Samba that use GnuTLS, specifically
the AD DC.
</para>
- <para>The default turns off SSLv3, as this protocol is no longer considered
- secure after CVE-2014-3566 (otherwise known as POODLE) impacted SSLv3 use
- in HTTPS applications.
- </para>
+ <para>The string is appended to the default priority list of GnuTLS.</para>
<para>The valid options are described in the
<ulink url="http://gnutls.org/manual/html_node/Priority-Strings.html">GNUTLS
Priority-Strings documentation at http://gnutls.org/manual/html_node/Priority-Strings.html</ulink>
</para>
+ <para>By default it will try to find a config file matching "SAMBA", but if
+ that does not exist will use the entry for "SYSTEM" and last fallback to
+ NORMAL. In all cases the SSL3.0 protocol will be disabled.</para>
</description>
- <value type="default">NORMAL:-VERS-SSL3.0</value>
+ <value type="default">@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0</value>
</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 63291283905..8fdd844fbaa 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2803,7 +2803,15 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
lpcfg_do_global_parameter(lp_ctx, "tls keyfile", "tls/key.pem");
lpcfg_do_global_parameter(lp_ctx, "tls certfile", "tls/cert.pem");
lpcfg_do_global_parameter(lp_ctx, "tls cafile", "tls/ca.pem");
- lpcfg_do_global_parameter(lp_ctx, "tls priority", "NORMAL:-VERS-SSL3.0");
+#ifdef HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND
+ lpcfg_do_global_parameter(lp_ctx,
+ "tls priority",
+ "@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0");
+#else
+ lpcfg_do_global_parameter(lp_ctx,
+ "tls priority",
+ "NORMAL:-VERS-SSL3.0");
+#endif
lpcfg_do_global_parameter(lp_ctx, "nsupdate command", "/usr/bin/nsupdate -g");
diff --git a/python/samba/tests/docs.py b/python/samba/tests/docs.py
index 32a16a98fbc..789865221cb 100644
--- a/python/samba/tests/docs.py
+++ b/python/samba/tests/docs.py
@@ -26,6 +26,21 @@ import os
import subprocess
import xml.etree.ElementTree as ET
+config_h = os.path.join("bin/default/include/config.h")
+config_hash = dict()
+
+if os.path.exists(config_h):
+ config_hash = dict()
+ f = open(config_h, 'r')
+ try:
+ lines = f.readlines()
+ config_hash = dict((x[0], ' '.join(x[1:]))
+ for x in map(lambda line: line.strip().split(' ')[1:],
+ list(filter(lambda line: (line[0:7] == '#define') and (len(line.split(' ')) > 2), lines))))
+ finally:
+ f.close()
+
+have_gnutls_system_config_support = ("HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND" in config_hash)
class TestCase(samba.tests.TestCaseInTempDir):
@@ -127,6 +142,11 @@ class SmbDotConfTests(TestCase):
'smbd max async dosmode',
])
+ # 'tls priority' has a legacy default value if we don't link against a
+ # modern GnuTLS version.
+ if not have_gnutls_system_config_support:
+ special_cases.add('tls priority')
+
def setUp(self):
super(SmbDotConfTests, self).setUp()
# create a minimal smb.conf file for testparm
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index d3d81f6ece5..2b1a63998d6 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -885,8 +885,15 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
lpcfg_string_set(Globals.ctx, &Globals._tls_keyfile, "tls/key.pem");
lpcfg_string_set(Globals.ctx, &Globals._tls_certfile, "tls/cert.pem");
lpcfg_string_set(Globals.ctx, &Globals._tls_cafile, "tls/ca.pem");
- lpcfg_string_set(Globals.ctx, &Globals.tls_priority,
- "NORMAL:-VERS-SSL3.0");
+#ifdef HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND
+ lpcfg_string_set(Globals.ctx,
+ &Globals.tls_priority,
+ "@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0");
+#else
+ lpcfg_string_set(Globals.ctx,
+ &Globals.tls_priority,
+ "NORMAL!-VERS-SSL3.0");
+#endif
lpcfg_string_set(Globals.ctx, &Globals.share_backend, "classic");
diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c
index 55bca036776..d984addeec5 100644
--- a/source4/lib/tls/tls_tstream.c
+++ b/source4/lib/tls/tls_tstream.c
@@ -1035,16 +1035,26 @@ struct tevent_req *_tstream_tls_connect_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- ret = gnutls_priority_set_direct(tlss->tls_session,
- tls_params->tls_priority,
- &error_pos);
+ ret = gnutls_set_default_priority(tlss->tls_session);
if (ret != GNUTLS_E_SUCCESS) {
- DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n",
- __location__, gnutls_strerror(ret), error_pos));
+ DBG_ERR("TLS %s - %s. Failed to set default priorities\n",
+ __location__, gnutls_strerror(ret));
tevent_req_error(req, EINVAL);
return tevent_req_post(req, ev);
}
+ if (strlen(tls_params->tls_priority) > 0) {
+ ret = gnutls_priority_set_direct(tlss->tls_session,
+ tls_params->tls_priority,
+ &error_pos);
+ if (ret != GNUTLS_E_SUCCESS) {
+ DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n",
+ __location__, gnutls_strerror(ret), error_pos));
+ tevent_req_error(req, EINVAL);
+ return tevent_req_post(req, ev);
+ }
+ }
+
ret = gnutls_credentials_set(tlss->tls_session,
GNUTLS_CRD_CERTIFICATE,
tls_params->x509_cred);
@@ -1284,16 +1294,26 @@ struct tevent_req *_tstream_tls_accept_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- ret = gnutls_priority_set_direct(tlss->tls_session,
- tlsp->tls_priority,
- &error_pos);
+ ret = gnutls_set_default_priority(tlss->tls_session);
if (ret != GNUTLS_E_SUCCESS) {
- DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n",
- __location__, gnutls_strerror(ret), error_pos));
+ DBG_ERR("TLS %s - %s. Failed to set default priorities\n",
+ __location__, gnutls_strerror(ret));
tevent_req_error(req, EINVAL);
return tevent_req_post(req, ev);
}
+ if (strlen(tlsp->tls_priority) > 0) {
+ ret = gnutls_priority_set_direct(tlss->tls_session,
+ tlsp->tls_priority,
+ &error_pos);
+ if (ret != GNUTLS_E_SUCCESS) {
+ DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n",
+ __location__, gnutls_strerror(ret), error_pos));
+ tevent_req_error(req, EINVAL);
+ return tevent_req_post(req, ev);
+ }
+ }
+
ret = gnutls_credentials_set(tlss->tls_session, GNUTLS_CRD_CERTIFICATE,
tlsp->x509_cred);
if (ret != GNUTLS_E_SUCCESS) {
diff --git a/wscript_configure_system_gnutls b/wscript_configure_system_gnutls
index b2b955f3c90..631405fa34c 100644
--- a/wscript_configure_system_gnutls
+++ b/wscript_configure_system_gnutls
@@ -20,6 +20,9 @@ conf.SET_TARGET_TYPE('gnutls', 'SYSLIB')
# Check for gnutls_pkcs7_get_embedded_data_oid (>= 3.5.5) required by libmscat
conf.CHECK_FUNCS_IN('gnutls_pkcs7_get_embedded_data_oid', 'gnutls')
+# Check for gnutls_set_default_priority_append (>= 3.6.3)
+conf.CHECK_FUNCS_IN('gnutls_set_default_priority_append', 'gnutls')
+
# Check for gnutls_aead_cipher_encryptv2
#
# This is available since version 3.6.10, but 3.6.10 has a bug which got fixed
--
2.26.2
From fdcf9f23f659025f174b32109a273e80b2ad289e Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Tue, 30 Jun 2020 17:12:17 +0200
Subject: [PATCH 2/2] tls: Use NORMAL:-VERS-SSL3.0 as the default configuration
This seems to be really broken in GnuTLS and the documentation is also
not correct.
This partially reverts 53e3a959b958a3b099df6ecc5f6e294e96bd948e
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14408
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Jul 1 14:56:33 UTC 2020 on sn-devel-184
---
docs-xml/smbdotconf/security/tlspriority.xml | 6 ++----
lib/param/loadparm.c | 6 ------
python/samba/tests/docs.py | 21 --------------------
source3/param/loadparm.c | 8 +-------
4 files changed, 3 insertions(+), 38 deletions(-)
diff --git a/docs-xml/smbdotconf/security/tlspriority.xml b/docs-xml/smbdotconf/security/tlspriority.xml
index 6d1f0dcb912..471dc25ba3b 100644
--- a/docs-xml/smbdotconf/security/tlspriority.xml
+++ b/docs-xml/smbdotconf/security/tlspriority.xml
@@ -12,10 +12,8 @@
<ulink url="http://gnutls.org/manual/html_node/Priority-Strings.html">GNUTLS
Priority-Strings documentation at http://gnutls.org/manual/html_node/Priority-Strings.html</ulink>
</para>
- <para>By default it will try to find a config file matching "SAMBA", but if
- that does not exist will use the entry for "SYSTEM" and last fallback to
- NORMAL. In all cases the SSL3.0 protocol will be disabled.</para>
+ <para>The SSL3.0 protocol will be disabled.</para>
</description>
- <value type="default">@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0</value>
+ <value type="default">NORMAL:-VERS-SSL3.0</value>
</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 8fdd844fbaa..4e7e3f599dd 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2803,15 +2803,9 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
lpcfg_do_global_parameter(lp_ctx, "tls keyfile", "tls/key.pem");
lpcfg_do_global_parameter(lp_ctx, "tls certfile", "tls/cert.pem");
lpcfg_do_global_parameter(lp_ctx, "tls cafile", "tls/ca.pem");
-#ifdef HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND
- lpcfg_do_global_parameter(lp_ctx,
- "tls priority",
- "@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0");
-#else
lpcfg_do_global_parameter(lp_ctx,
"tls priority",
"NORMAL:-VERS-SSL3.0");
-#endif
lpcfg_do_global_parameter(lp_ctx, "nsupdate command", "/usr/bin/nsupdate -g");
diff --git a/python/samba/tests/docs.py b/python/samba/tests/docs.py
index 789865221cb..654a192b510 100644
--- a/python/samba/tests/docs.py
+++ b/python/samba/tests/docs.py
@@ -26,22 +26,6 @@ import os
import subprocess
import xml.etree.ElementTree as ET
-config_h = os.path.join("bin/default/include/config.h")
-config_hash = dict()
-
-if os.path.exists(config_h):
- config_hash = dict()
- f = open(config_h, 'r')
- try:
- lines = f.readlines()
- config_hash = dict((x[0], ' '.join(x[1:]))
- for x in map(lambda line: line.strip().split(' ')[1:],
- list(filter(lambda line: (line[0:7] == '#define') and (len(line.split(' ')) > 2), lines))))
- finally:
- f.close()
-
-have_gnutls_system_config_support = ("HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND" in config_hash)
-
class TestCase(samba.tests.TestCaseInTempDir):
def _format_message(self, parameters, message):
@@ -142,11 +126,6 @@ class SmbDotConfTests(TestCase):
'smbd max async dosmode',
])
- # 'tls priority' has a legacy default value if we don't link against a
- # modern GnuTLS version.
- if not have_gnutls_system_config_support:
- special_cases.add('tls priority')
-
def setUp(self):
super(SmbDotConfTests, self).setUp()
# create a minimal smb.conf file for testparm
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 2b1a63998d6..901f01b1c6a 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -885,15 +885,9 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
lpcfg_string_set(Globals.ctx, &Globals._tls_keyfile, "tls/key.pem");
lpcfg_string_set(Globals.ctx, &Globals._tls_certfile, "tls/cert.pem");
lpcfg_string_set(Globals.ctx, &Globals._tls_cafile, "tls/ca.pem");
-#ifdef HAVE_GNUTLS_SET_DEFAULT_PRIORITY_APPEND
lpcfg_string_set(Globals.ctx,
&Globals.tls_priority,
- "@SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0");
-#else
- lpcfg_string_set(Globals.ctx,
- &Globals.tls_priority,
- "NORMAL!-VERS-SSL3.0");
-#endif
+ "NORMAL:-VERS-SSL3.0");
lpcfg_string_set(Globals.ctx, &Globals.share_backend, "classic");
--
2.26.2