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

diff --git a/docs-xml/smbdotconf/security/tlspriority.xml b/docs-xml/smbdotconf/security/tlspriority.xml
index d7214a4c1eafe240918b1860464b65059f206efb..6d1f0dcb91275c487f380e6ed4ebd9dde23b4b20 100644 (file)
--- 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 d00ed9dca439a1b96b8c4538060977a2fba86492..53eedeb0cb2cd4b1a2dbf24afbfeaad5aeae9615 100644 (file)
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2818,7 +2818,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 4b67bc2bd3886ac7e26665af8b6d17e166325d0f..3657d3d85cb23af0c9e28cb44aae62195df2c84c 100644 (file)
--- 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):
 
@@ -128,6 +143,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 ce2aca2c5a450a3ecc229773713d46db4c3b4e2f..0ceaa7d8edf0b9bdc866e6161d471320d596e120 100644 (file)
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -886,8 +886,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 55bca0367762e66863cec472eef97443a8adcf8b..d984addeec5f139336612225170c256cbea1417e 100644 (file)
--- 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 cd2f5596e11250608f07ca4e3cfcf605aa43ae05..9eabd0da75c445f829518121d4570230f7ddd115 100644 (file)
--- 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