[PATCH] Patch for bug 12520
Ralph Böhme
slow at samba.org
Thu Jan 19 07:28:46 UTC 2017
On Wed, Jan 18, 2017 at 06:41:45AM +1100, Andrew Bartlett wrote:
> On Mon, 2017-01-16 at 15:54 +0100, Ralph Böhme wrote:
> > Hi!
> >
> > Attached is a fix for:
> > <https://bugzilla.samba.org/show_bug.cgi?id=12520>
> >
> > I didn't add a test because that would require an additional server
> > in selftest
> > just for that. Shall I?
>
> Given the issue we had around smb signing not being enabled for years
> in the AD DC because we didn't actually test it correctly in the
> default, let along all the reasonable configurations, I would really
> prefer the costs of the extra environments. Particularly for the
> standalone fileserver, they are not expensive.
instead of adding a new env I got away with disabling it in the simpleserver
env. :)
The test then pointed me at our SMB1 UNIX extensions encrpytion code that got
this wrog as well.
Updated patchset attached. Please review & push if ok. Thanks!
Cheerio!
-slow
-------------- next part --------------
From 91d803050bcf1765a496f3e2210dfb796680ebf7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at 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 at samba.org>
---
source3/smbd/process.c | 4 ++++
source3/smbd/service.c | 12 ++++++++++++
2 files changed, 16 insertions(+)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 8f097ec..3b6ff4c 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1636,6 +1636,10 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
/* load service specific parameters */
if (conn) {
if (req->encrypted) {
+ if (lp_smb_encrypt(-1) == SMB_SIGNING_OFF) {
+ reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return conn;
+ }
conn->encrypted_tid = true;
/* encrypted required from now on. */
conn->encrypt_level = SMB_SIGNING_REQUIRED;
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 3308e9d..22cc0af 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -623,6 +623,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 encrpytion, 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.7.4
From da95c05fab6ad8ebbb67a006d5ebc3d4b316b9c4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at 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 at samba.org>
---
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 6cfa64f..d9ccdbe 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -441,7 +441,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.7.4
From b4f8c1a63baae46578499a13724e3949714ce2fa Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at 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 at samba.org>
---
source3/smbd/smb2_tcon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
index 61e2a36..cc4b6b6 100644
--- a/source3/smbd/smb2_tcon.c
+++ b/source3/smbd/smb2_tcon.c
@@ -268,6 +268,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
}
if ((lp_smb_encrypt(snum) >= SMB_SIGNING_DESIRED) &&
+ (conn->smb2.server.cipher != 0) &&
(conn->smb2.client.capabilities & SMB2_CAP_ENCRYPTION)) {
encryption_desired = true;
}
--
2.7.4
From 961c12111665529b2ad86c2b804efe6abb92e02a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 16 Jan 2017 15:45:32 +0100
Subject: [PATCH 4/6] docs: impact of a global "smb encrpyt=off" on a share
with "smb encrpyt=required"
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12520
Signed-off-by: Ralph Boehme <slow at samba.org>
---
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.7.4
From d21f1803c5027403b9c93407bb25522a739d3fae Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at 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 at samba.org>
---
selftest/target/Samba3.pm | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 1d77c97..6f76896 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -579,6 +579,7 @@ sub setup_simpleserver($$)
ntlm auth = yes
vfs objects = xattr_tdb streams_depot time_audit full_audit
change notify = no
+ smb encrypt = off
full_audit:syslog = no
full_audit:success = none
@@ -596,6 +597,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.7.4
From 9760a3778374159a26d4b1926ad76c003d5a7d7f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at 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 at samba.org>
---
.../script/tests/test_smbclient_encryption_off.sh | 47 ++++++++++++++++++++++
source3/selftest/tests.py | 5 +++
2 files changed, 52 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..6fdfac5
--- /dev/null
+++ b/source3/script/tests/test_smbclient_encryption_off.sh
@@ -0,0 +1,47 @@
+#!/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 encrpyt" (which is "enabled")
+# "tmpenc" has "smb encrpyt = required"
+# "enc_desired" has "smb encrpyt = desired"
+#
+
+# These tests must fail, as encrpytion 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
+testit_expect_failure "smbclient -m smb3 //$SERVER/tmpenc" $SMBCLIENT -m smb3 -U $USERNAME%$PASSWORD //$SERVER/tmpenc -c quit && failed=`expr $failed + 1`
+testit_expect_failure "smbclient -e -m smb3 //$SERVER/tmpenc" $SMBCLIENT -e -m smb3 -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
+testit_expect_failure "smbclient -e -m smb3 //$SERVER/enc_desired" $SMBCLIENT -e -m smb3 -U $USERNAME%$PASSWORD //$SERVER/enc_desired -c quit && failed=`expr $failed + 1`
+testit_expect_failure "smbclient -e -m smb3 //$SERVER/tmp" $SMBCLIENT -e -m smb3 -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 37cf1a4..e6ed2ec 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -493,6 +493,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.7.4
More information about the samba-technical
mailing list