From 725b1db5fa700eebc2cc946bbce456063da0a408 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 24d1e37e9cb..fce35318b88 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1756,6 +1756,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 e4a6830eecb..0bec7c5daaa 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -273,6 +273,7 @@ NTSTATUS make_session_info_from_username(TALLOC_CTX *mem_ctx, struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx, const struct auth_session_info *src); 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 39930ebf05404ea0a8ffecb5109e32549adb8c19 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 b73ac2bb731..9ba4be87319 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 88afa6d8c7a8e73b8380ca097e755870b162505f 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 (backported 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 75e4585ce67..14252344175 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -597,6 +597,7 @@ sub setup_ad_member_idmap_rid idmap config * : range = 1000000-1999999 idmap config $dcvars->{DOMAIN} : backend = rid idmap config $dcvars->{DOMAIN} : range = 2000000-2999999 + map to guest = bad user "; my $ret = $self->provision($prefix, $dcvars->{DOMAIN}, -- 2.21.0 From c96ba6e982a60898ef7f0b2e26e28a8e9ea05168 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 e390ca390a8..64546900d83 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -737,3 +737,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 21f06cff69bd22a493ba89b6a4dcd2690aa0becf 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 fce35318b88..a08df0aabe4 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1383,6 +1383,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