From f36a7f5641ede3f8c66d08278d9039cf0d17df2c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 16 May 2019 12:42:29 +0200 Subject: [PATCH 1/5] s3:auth: add reinit_guest_session_info() BUG: https://bugzilla.samba.org/show_bug.cgi?id=13944 Signed-off-by: Ralph Boehme Reviewed-by: Andrew Bartlett (cherry picked from commit 8096cc7eb2b36b074ff17a52dc3540be4ecff6bb) --- source3/auth/auth_util.c | 11 +++++++++++ source3/auth/proto.h | 1 + 2 files changed, 12 insertions(+) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index d78dbed14b2..8363d386c12 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1722,6 +1722,17 @@ bool init_guest_session_info(TALLOC_CTX *mem_ctx) return true; } +bool reinit_guest_session_info(TALLOC_CTX *mem_ctx) +{ + TALLOC_FREE(guest_info); + TALLOC_FREE(guest_server_info); + TALLOC_FREE(anonymous_info); + + DBG_DEBUG("Reinitialing guest info\n"); + + return init_guest_session_info(mem_ctx); +} + NTSTATUS make_server_info_guest(TALLOC_CTX *mem_ctx, struct auth_serversupplied_info **server_info) { diff --git a/source3/auth/proto.h b/source3/auth/proto.h index 75cf1e6724f..fcfd1f36ca2 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -271,6 +271,7 @@ NTSTATUS make_session_info_from_username(TALLOC_CTX *mem_ctx, bool is_guest, struct auth_session_info **session_info); bool init_guest_session_info(TALLOC_CTX *mem_ctx); +bool reinit_guest_session_info(TALLOC_CTX *mem_ctx); NTSTATUS init_system_session_info(TALLOC_CTX *mem_ctx); bool session_info_set_session_key(struct auth_session_info *info, DATA_BLOB session_key); -- 2.21.0 From a07d010cb7260bbdd473bfe56ebea684bd2f5f81 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 16 May 2019 12:42:54 +0200 Subject: [PATCH 2/5] s3:smbd: call reinit_guest_session_info() in the conf updated handler BUG: https://bugzilla.samba.org/show_bug.cgi?id=13944 Signed-off-by: Ralph Boehme Reviewed-by: Andrew Bartlett (cherry picked from commit f4e340a48b6f059a1daa66deb9c26da9e8fcd5e7) --- source3/smbd/server.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/server.c b/source3/smbd/server.c index c59759fed98..7d96a5762ec 100644 --- a/source3/smbd/server.c +++ b/source3/smbd/server.c @@ -117,12 +117,18 @@ static void smbd_parent_conf_updated(struct messaging_context *msg, { struct tevent_context *ev_ctx = talloc_get_type_abort(private_data, struct tevent_context); + bool ok; DEBUG(10,("smbd_parent_conf_updated: Got message saying smb.conf was " "updated. Reloading.\n")); change_to_root_user(); reload_services(NULL, NULL, false); printing_subsystem_update(ev_ctx, msg, false); + + ok = reinit_guest_session_info(NULL); + if (!ok) { + DBG_ERR("Failed to reinit guest info\n"); + } } /******************************************************************* -- 2.21.0 From 24d7cd00ffac2313757f27129293f0b1b7891b60 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 16 May 2019 12:43:40 +0200 Subject: [PATCH 3/5] selftest: allow guest login in the ad_member_idmap_rid env BUG: https://bugzilla.samba.org/show_bug.cgi?id=13944 Signed-off-by: Ralph Boehme Reviewed-by: Andrew Bartlett (cherry picked from commit ac2167eb2349dc1c453e14a65692f16c8ba6532e) --- selftest/target/Samba3.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 892a6a15e2d..9d88253c9fe 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -609,6 +609,7 @@ sub setup_ad_member_idmap_rid # Prevent overridding the provisioned lib/krb5.conf which sets certain # values required for tests to succeed create krb5 conf = no + map to guest = bad user "; my $ret = $self->provision($prefix, $dcvars->{DOMAIN}, -- 2.21.0 From 794cd6dbcecc0cf0f02e85baeff46d6d1651dbf5 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 16 May 2019 12:47:34 +0200 Subject: [PATCH 4/5] tests: add a test for guest authentication This verifies that smbd always adds BUILTIN\Guests to the guest token which is required for guest authentication. Currently the guest token depends on the on-disk configured group mappings. If there's an existing group mapping for BUILTIN\Guests, but LOCALSAM\Guest is not a member, the final guest token won't contain BUILTIN\Guests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13944 Signed-off-by: Ralph Boehme Reviewed-by: Andrew Bartlett (cherry picked from commit 0e88f98855e24cfddb55bef65c5910b8e662c630) --- selftest/knownfail.d/samba3.blackbox.guest | 1 + source3/script/tests/test_guest_auth.sh | 103 +++++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 109 insertions(+) create mode 100644 selftest/knownfail.d/samba3.blackbox.guest create mode 100755 source3/script/tests/test_guest_auth.sh diff --git a/selftest/knownfail.d/samba3.blackbox.guest b/selftest/knownfail.d/samba3.blackbox.guest new file mode 100644 index 00000000000..cbb62d71c87 --- /dev/null +++ b/selftest/knownfail.d/samba3.blackbox.guest @@ -0,0 +1 @@ +^samba3.blackbox.guest.*smbclient_guest_auth_without_members diff --git a/source3/script/tests/test_guest_auth.sh b/source3/script/tests/test_guest_auth.sh new file mode 100755 index 00000000000..4ad4a5cbd63 --- /dev/null +++ b/source3/script/tests/test_guest_auth.sh @@ -0,0 +1,103 @@ +#!/bin/sh +# +# Test guest authentication +# +# Copyright (C) 2019 Ralph Boehme +# + +if [ $# -lt 5 ]; then +cat <&1) + bg_exists=$? + if [ $bg_exists != 0 ] ; then + printf "Group map for BUILTIN\\Guests must exist for test\n" + return 1 + fi + + SIDS=$($NET $CONFIGURATION groupmap listmem S-1-5-32-546) + if [ $? != 0 ] ; then + printf "$NET $CONFIGURATION groupmap listmem S-1-5-32-546 failed. Returned:\n" + printf "$SIDS\n" + return 1 + fi + printf "Got S-1-5-32-546 members:\n$SIDS\n" + + if [ "$SIDS" != "" ] ; then + for SID in $SIDS ; do + printf "Deleting member $SID from S-1-5-32-546\n" + $NET $CONFIGURATION groupmap delmem S-1-5-32-546 $SID || return 1 + done + fi + + return 0 +} + +add_local_guest_to_builtin_guests() { + if [ "$SIDS" != "" ] ; then + for SID in $SIDS ; do + printf "Adding $SID as member to S-1-5-32-546\n" + $NET $CONFIGURATION groupmap addmem S-1-5-32-546 $SID || return 1 + done + fi +} + +test_smbclient() { + $SMBCLIENT -U foo%bar //$SERVER/tmpguest -c exit + if [ $? != 0 ] ; then + printf "smbclient failed\n" + return 1 + fi + return 0 +} + +testit "smbclient_guest_at_startup" \ + test_smbclient || + failed=$(expr $failed + 1) + +printf "Prepare BUILTIN\\Guests group mapping without members\n" + +prepare_empty_builtin_guests || { + printf "Setting up BUILTIN\\Guests without members failed\n" + exit 1 +} + +$SMBCONTROL $CONFIGURATION smbd reload-config || { + printf "Reloading parent smbd guest info failed\n" + exit 1 +} + +testit "smbclient_guest_auth_without_members" \ + test_smbclient && + failed=$(expr $failed + 1) + +# restore config +add_local_guest_to_builtin_guests + +$SMBCONTROL $CONFIGURATION smbd reload-config || { + printf "Reloading parent smbd guest info failed\n" + exit 1 +} + +testit "smbclient_works_after_restored_setup" \ + test_smbclient || + failed=$(expr $failed + 1) + +testok $0 $failed diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 1db806ef887..91b870430b9 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -807,3 +807,8 @@ options_list = ["-mNT1", "-mNT1 -e", "-mSMB3", "-mSMB3 -e"] plansmbtorture4testsuite('rpc.epmapper', 'nt4_dc:local', 'ncalrpc: -U$USERNAME%$PASSWORD', 'over ncalrpc') plansmbtorture4testsuite('rpc.fsrvp', 'nt4_dc:local', 'ncacn_np:$SERVER_IP[/pipe/FssagentRpc] -U$USERNAME%$PASSWORD', 'over ncacn_np') + +for env in ["ad_member_idmap_rid:local", "maptoguest:local"]: + plantestsuite("samba3.blackbox.guest (%s)" % env , env, + [os.path.join(samba3srcdir, "script/tests/test_guest_auth.sh"), + '$SERVER', smbclient3, smbcontrol, net, configuration]) -- 2.21.0 From 919a34f0508c9abb3442c3367b27cc1d99be7732 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 13 May 2019 20:16:47 +0200 Subject: [PATCH 5/5] s3:auth: explicitly add BUILTIN\Guests to the guest token This changes ensures that smbd always adds BUILTIN\Guests to the guest token which is required for guest authentication. Currently the guest token depends on the on-disk configured group mappings. If there's an existing group mapping for BUILTIN\Guests, but LOCALSAM\Guest is not a member, the final guest token won't contain BUILTIN\Guests. For SMB2 the flag SMB2_SESSION_FLAG_IS_GUEST will not be set in the final SMB2 SESSION_SETUP response, because smbd sets it based on the token containing the BUILTIN\Guests SID S-1-5-32-546. At the same time, the packet is not signed which causes Windows clients and smbclient to reject the unsigned SMB2 SESSION_SETUP response. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13944 Pair-programmed-with: Stefan Metzmacher Signed-off-by: Ralph Boehme Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Wed Jun 5 16:55:26 UTC 2019 on sn-devel-184 (cherry picked from commit a66af4c96accba4ee64eeb1958458b69f3ccec1d) --- selftest/knownfail.d/samba3.blackbox.guest | 1 - source3/auth/auth_util.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/samba3.blackbox.guest diff --git a/selftest/knownfail.d/samba3.blackbox.guest b/selftest/knownfail.d/samba3.blackbox.guest deleted file mode 100644 index cbb62d71c87..00000000000 --- a/selftest/knownfail.d/samba3.blackbox.guest +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.guest.*smbclient_guest_auth_without_members diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 8363d386c12..8ff20c33759 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1387,6 +1387,21 @@ static NTSTATUS make_new_session_info_guest(TALLOC_CTX *mem_ctx, goto done; } + /* + * It's ugly, but for now it's + * needed to force Builtin_Guests + * here, because memberships of + * Builtin_Guests might be incomplete. + */ + status = add_sid_to_array_unique(session_info->security_token, + &global_sid_Builtin_Guests, + &session_info->security_token->sids, + &session_info->security_token->num_sids); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("Failed to force Builtin_Guests to nt token\n"); + goto done; + } + /* annoying, but the Guest really does have a session key, and it is all zeros! */ session_info->session_key = data_blob_talloc_zero(session_info, 16); -- 2.21.0