From 2840bd0becee307f4ee896b26e9f29baac03c347 Mon Sep 17 00:00:00 2001 From: Andreas Schneider 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 Reviewed-by: Alexander Bokovoy Autobuild-User(master): Andreas Schneider 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. - 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. - + The string is appended to the default priority list of GnuTLS. The valid options are described in the GNUTLS Priority-Strings documentation at http://gnutls.org/manual/html_node/Priority-Strings.html + 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. - NORMAL:-VERS-SSL3.0 + @SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0 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 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 Reviewed-by: Alexander Bokovoy Autobuild-User(master): Andrew Bartlett 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 @@ GNUTLS Priority-Strings documentation at http://gnutls.org/manual/html_node/Priority-Strings.html - 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. + The SSL3.0 protocol will be disabled. - @SAMBA,SYSTEM,NORMAL:!-VERS-SSL3.0 + NORMAL:-VERS-SSL3.0 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