Attachment 16059 Details for Bug 14408 – patch for 4.12

[patch] patch for 4.12

0001-s3-lib-tls-Use-better-priority-lists-for-modern-GnuT.patch (text/plain), 8.37 KB, created by Andreas Schneider on 2020-06-18 11:30:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Schneider

Created: 2020-06-18 11:30:24 UTC

Size: 8.37 KB

patch

obsolete

>From 6981475bbe11029d88de8294734d7cb29f1d0799 Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Mon, 15 Jun 2020 11:50:16 +0200
>Subject: [PATCH] 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>
>(cherry picked from commit 53e3a959b958a3b099df6ecc5f6e294e96bd948e)
>---
> 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.27.0
>

Flags:

asn: review? (ab)

Actions: View

Attachments on bug 14408: 16059