Attachment 12880 Details for Bug 12520 – Patch for 4.4 cherry-picked from master

[patch] Patch for 4.4 cherry-picked from master

bug12520-v44.patch (text/plain), 12.45 KB, created by Ralph Böhme on 2017-01-31 06:47:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2017-01-31 06:47:55 UTC

Size: 12.45 KB

patch

obsolete

>From f877818b29440df16ffacf728b5ace8079bc92c4 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 18 Jan 2017 16:19:15 +0100
>Subject: [PATCH 1/6] s3/smbd: ensure global "smb encrypt = off" is effective
> for SMB 1 clients
>
>If encryption is disabled globally, per definition we shouldn't allow
>enabling encryption on individual shares.
>
>The behaviour of setting
>
>[Global]
>  smb encrypt = off
>
>[share_required]
>  smb encrypt = required
>
>[share_desired]
>  smb encrypt = desired
>
>must be to completely deny access to the share "share_required" and an
>unencrypted connection to "share_desired".
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 43a90cee46bb7a70f7973c4fc51eee7634e43145)
>---
> source3/smbd/service.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/source3/smbd/service.c b/source3/smbd/service.c
>index 0c70250..8c6d140 100644
>--- a/source3/smbd/service.c
>+++ b/source3/smbd/service.c
>@@ -584,6 +584,18 @@ static NTSTATUS make_connection_snum(struct smbXsrv_connection *xconn,
> 	conn->short_case_preserve = lp_short_preserve_case(snum);
> 
> 	conn->encrypt_level = lp_smb_encrypt(snum);
>+	if (conn->encrypt_level > SMB_SIGNING_OFF) {
>+		if (lp_smb_encrypt(-1) == SMB_SIGNING_OFF) {
>+			if (conn->encrypt_level == SMB_SIGNING_REQUIRED) {
>+				DBG_ERR("Service [%s] requires encryption, but "
>+					"it is disabled globally!\n",
>+					lp_servicename(talloc_tos(), snum));
>+				status = NT_STATUS_ACCESS_DENIED;
>+				goto err_root_exit;
>+			}
>+			conn->encrypt_level = SMB_SIGNING_OFF;
>+		}
>+	}
> 
> 	conn->veto_list = NULL;
> 	conn->hide_list = NULL;
>-- 
>2.9.3
>
>
>From a1ded59149ac457266ab8d1160a9b3cbbbd24baf Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 5 Jan 2017 12:14:35 +0100
>Subject: [PATCH 2/6] s3/smbd: ensure global "smb encrypt = off" is effective
> for SMB 3.1.1 clients
>
>If encryption is disabled globally, per definition we shouldn't allow
>enabling encryption on individual shares.
>
>The behaviour of setting
>
>[Global]
>  smb encrypt = off
>
>[share]
>  smb encrypt = required
>
>must be to completely deny access to the share "share".
>
>This was working correctly for clients when using SMB 3 dialects <
>3.1.1, but not for 3.1.1 with a negprot encryption context.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 6ae63d42f5aacddf5b7b6dbdfbe620344989e4e5)
>---
> source3/smbd/smb2_negprot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
>index 9c03b2c..007be6b 100644
>--- a/source3/smbd/smb2_negprot.c
>+++ b/source3/smbd/smb2_negprot.c
>@@ -429,7 +429,7 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
> 		req->preauth = &req->xconn->smb2.preauth;
> 	}
> 
>-	if (in_cipher != NULL) {
>+	if ((capabilities & SMB2_CAP_ENCRYPTION) && (in_cipher != NULL)) {
> 		size_t needed = 2;
> 		uint16_t cipher_count;
> 		const uint8_t *p;
>-- 
>2.9.3
>
>
>From 0b406a55e784562b1d6448947af3e07486743108 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 16 Jan 2017 12:56:10 +0100
>Subject: [PATCH 3/6] s3/smbd: ensure global "smb encrypt = off" is effective
> for share with "smb encrypt = desired"
>
>If encryption is disabled globally, per definition we shouldn't allow
>enabling encryption on individual shares.
>
>The behaviour of specifying
>
>[Global]
>  smb encrypt = off
>
>[share]
>  smb encrypt = desired
>
>must be an unecrypted tree connect to the share "share".
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit b0b418c22558fa1df547df9bdac2642343ac39e1)
>---
> source3/smbd/smb2_tcon.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
>index 61e2a36..5330fc3 100644
>--- a/source3/smbd/smb2_tcon.c
>+++ b/source3/smbd/smb2_tcon.c
>@@ -268,7 +268,8 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
> 	}
> 
> 	if ((lp_smb_encrypt(snum) >= SMB_SIGNING_DESIRED) &&
>-	    (conn->smb2.client.capabilities & SMB2_CAP_ENCRYPTION)) {
>+	    (conn->smb2.server.cipher != 0))
>+	{
> 		encryption_desired = true;
> 	}
> 
>-- 
>2.9.3
>
>
>From abba235c115dbfffd35c6f6ccc8ac81029ecdaa8 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 16 Jan 2017 15:45:32 +0100
>Subject: [PATCH 4/6] docs: impact of a global "smb encrypt=off" on a share
> with "smb encrypt=required"
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit f8d937b331ac985264c76d76b447683fc494d38a)
>---
> docs-xml/smbdotconf/security/smbencrypt.xml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/docs-xml/smbdotconf/security/smbencrypt.xml b/docs-xml/smbdotconf/security/smbencrypt.xml
>index 0f08966..32a22cb 100644
>--- a/docs-xml/smbdotconf/security/smbencrypt.xml
>+++ b/docs-xml/smbdotconf/security/smbencrypt.xml
>@@ -180,7 +180,11 @@
> 			<listitem>
> 			<para>
> 			Setting it to <emphasis>off</emphasis> globally will
>-			completely disable the encryption feature.
>+			completely disable the encryption feature for all
>+			connections. Setting <parameter>smb encrypt =
>+			required</parameter> for individual shares (while it's
>+			globally off) will deny access to this shares for all
>+			clients.
> 			</para>
> 			</listitem>
> 
>-- 
>2.9.3
>
>
>From 29b092ad014a75e67fa4eb139ee6a706394e1cfb Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 17 Jan 2017 17:23:51 +0100
>Subject: [PATCH 5/6] selftest: disable SMB encryption in simpleserver
> environment
>
>Encryption is currently not tested in this env so we can safely turn it
>off. The next commit will add a blackbox tests that test combinations of
>having encryption globally turned off and enabled (desired/required) on
>a share.
>
>This also adds a new share "enc_desired" with "smb encrypt = desired"
>which will be used by the test in the next commit.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 573e8e15b3ed27d6b593e635e9c24eea3fdf4fb9)
>---
> selftest/target/Samba3.pm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
>index 5e62cf1..17ad820 100755
>--- a/selftest/target/Samba3.pm
>+++ b/selftest/target/Samba3.pm
>@@ -547,6 +547,7 @@ sub setup_simpleserver($$)
> 	lanman auth = yes
> 	vfs objects = xattr_tdb streams_depot
> 	change notify = no
>+	smb encrypt = off
> 
> [vfs_aio_fork]
> 	path = $prefix_abs/share
>@@ -560,6 +561,11 @@ sub setup_simpleserver($$)
> 	store dos attributes = yes
> 	hide files = /hidefile/
> 	hide dot files = yes
>+
>+[enc_desired]
>+	path = $prefix_abs/share
>+	vfs objects =
>+	smb encrypt = desired
> ";
> 
> 	my $vars = $self->provision($path,
>-- 
>2.9.3
>
>
>From cf8a3002233c5781907b1fb57f4a07c524b44816 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 18 Jan 2017 16:23:40 +0100
>Subject: [PATCH 6/6] selftest: add test for global "smb encrypt=off"
>
>Test various combinations of having encryption globally turned off and
>enabled (desired/required) on a share, with SMB1 UNIX Extensions and SMB3.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 21d030e5bdf7dc6ef8d5f4e70bed7e70b731cd15)
>---
> .../script/tests/test_smbclient_encryption_off.sh  | 65 ++++++++++++++++++++++
> source3/selftest/tests.py                          |  5 ++
> 2 files changed, 70 insertions(+)
> create mode 100755 source3/script/tests/test_smbclient_encryption_off.sh
>
>diff --git a/source3/script/tests/test_smbclient_encryption_off.sh b/source3/script/tests/test_smbclient_encryption_off.sh
>new file mode 100755
>index 0000000..467a4ee
>--- /dev/null
>+++ b/source3/script/tests/test_smbclient_encryption_off.sh
>@@ -0,0 +1,65 @@
>+#!/bin/sh
>+
>+if [ $# -lt 4 ]; then
>+cat <<EOF
>+Usage: test_smbclient_encryption_off.sh USERNAME PASSWORD SERVER SMBCLIENT
>+EOF
>+exit 1;
>+fi
>+
>+USERNAME="$1"
>+PASSWORD="$2"
>+SERVER="$3"
>+SMBCLIENT="$VALGRIND $4"
>+
>+incdir=`dirname $0`/../../../testprogs/blackbox
>+. $incdir/subunit.sh
>+
>+failed=0
>+
>+#
>+# Let me introduce you to the shares used in this test:
>+#
>+# "tmp" has the default "smb encrypt" (which is "enabled")
>+# "tmpenc" has "smb encrypt = required"
>+# "enc_desired" has "smb encrypt = desired"
>+#
>+
>+# Unencrypted connections should work of course, let's test em to be sure...
>+
>+# SMB1
>+testit "smbclient //$SERVER/enc_desired" $SMBCLIENT -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit || failed=`expr $failed + 1`
>+testit "smbclient //$SERVER/tmp" $SMBCLIENT -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit || failed=`expr $failed + 1`
>+# SMB3_02
>+testit "smbclient -m smb3_02 //$SERVER/enc_desired" $SMBCLIENT -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit || failed=`expr $failed + 1`
>+testit "smbclient -m smb3_02 //$SERVER/tmp" $SMBCLIENT -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit || failed=`expr $failed + 1`
>+# SMB3_11
>+testit "smbclient -m smb3_11 //$SERVER/enc_desired" $SMBCLIENT -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit || failed=`expr $failed + 1`
>+testit "smbclient -m smb3_11 //$SERVER/tmp" $SMBCLIENT -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit || failed=`expr $failed + 1`
>+
>+# These tests must fail, as encryption is globally off and in combination with "smb
>+# encrypt=required" on the share "tmpenc" the server *must* reject the tcon.
>+
>+# SMB1
>+testit_expect_failure "smbclient //$SERVER/tmpenc" $SMBCLIENT -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e //$SERVER/tmpenc" $SMBCLIENT -e -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+# SMB3_02
>+testit_expect_failure "smbclient -m smb3_02 //$SERVER/tmpenc" $SMBCLIENT -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e -m smb3_02 //$SERVER/tmpenc" $SMBCLIENT -e -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+# SMB3_11
>+testit_expect_failure "smbclient -m smb3_11 //$SERVER/tmpenc" $SMBCLIENT -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e -m smb3_11 //$SERVER/tmpenc" $SMBCLIENT -e -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
>+
>+# These tests must fail, as the client requires encryption and it's off on the server
>+
>+# SMB1
>+testit_expect_failure "smbclient -e //$SERVER/enc_desired" $SMBCLIENT -e -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e //$SERVER/tmp" $SMBCLIENT -e -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit && failed=`expr $failed + 1`
>+# SMB3_02
>+testit_expect_failure "smbclient -e -m smb3_02 //$SERVER/enc_desired" $SMBCLIENT -e -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e -m smb3_02 //$SERVER/tmp" $SMBCLIENT -e -m smb3_02 -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit && failed=`expr $failed + 1`
>+# SMB3_11
>+testit_expect_failure "smbclient -e -m smb3_11 //$SERVER/enc_desired" $SMBCLIENT -e -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit && failed=`expr $failed + 1`
>+testit_expect_failure "smbclient -e -m smb3_11 //$SERVER/tmp" $SMBCLIENT -e -m smb3_11 -U $USERNAME%$PASSWORD //$SERVER/tmp -c quit && failed=`expr $failed + 1`
>+
>+testok $0 $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index e389fae..f5bddc2 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -472,6 +472,11 @@ plantestsuite("samba3.blackbox.rpcclient.pw-nt-hash", "simpleserver",
>                "$USERNAME", "$PASSWORD", "$SERVER",
>                os.path.join(bindir(), "rpcclient")])
> 
>+plantestsuite("samba3.blackbox.smbclient.encryption_off", "simpleserver",
>+              [os.path.join(samba3srcdir, "script/tests/test_smbclient_encryption_off.sh"),
>+               "$USERNAME", "$PASSWORD", "$SERVER",
>+               smbclient3])
>+
> options_list = ["", "-e"]
> for options in options_list:
>     plantestsuite("samba3.blackbox.smbclient_krb5 old ccache %s" % options, "ktest:local",
>-- 
>2.9.3
>

Flags:

jra: review+

Actions: View

Attachments on bug 12520: 12880 | 12881 | 12882