From 1fe1ad3b1faf9baa398ab5027ca2590e79182144 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:16:41 +0200 Subject: [PATCH 1/9] srvsvc: Introduce ctx3 helper var in enum_file_fn() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit a9397f87881b9a67407b557e09478cdd40f75b75) --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index 79744344537..c98323bd729 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -87,9 +87,9 @@ static int enum_file_fn(struct file_id id, { struct file_enum_count *fenum = (struct file_enum_count *)private_data; - + struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; - int i = fenum->ctr3->count; + int i = ctr3->count; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -110,13 +110,16 @@ static int enum_file_fn(struct file_id id, return 0; } - f = talloc_realloc(fenum->ctx, fenum->ctr3->array, - struct srvsvc_NetFileInfo3, i+1); + f = talloc_realloc( + fenum->ctx, + ctr3->array, + struct srvsvc_NetFileInfo3, + i+1); if ( !f ) { DEBUG(0,("conn_enum_fn: realloc failed for %d items\n", i+1)); return 0; } - fenum->ctr3->array = f; + ctr3->array = f; /* need to count the number of locks on a file */ @@ -151,14 +154,14 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - fenum->ctr3->array[i].fid = + ctr3->array[i].fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id); - fenum->ctr3->array[i].permissions = permissions; - fenum->ctr3->array[i].num_locks = num_locks; - fenum->ctr3->array[i].path = fullpath; - fenum->ctr3->array[i].user = username; + ctr3->array[i].permissions = permissions; + ctr3->array[i].num_locks = num_locks; + ctr3->array[i].path = fullpath; + ctr3->array[i].user = username; - fenum->ctr3->count++; + ctr3->count++; return 0; } -- 2.20.1 From 1dfe1e57dc383a99d9b9affd229b12a8dfacb457 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:21:49 +0200 Subject: [PATCH 2/9] srvsvc: Use a struct assignment in enum_file_fn() Looks nicer than 5 complex array references... Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit ff80f68c3020be0a92eb41115a64518ece097ee7) --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index c98323bd729..b81de3d2b9e 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -154,12 +154,14 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - ctr3->array[i].fid = - (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id); - ctr3->array[i].permissions = permissions; - ctr3->array[i].num_locks = num_locks; - ctr3->array[i].path = fullpath; - ctr3->array[i].user = username; + ctr3->array[i] = (struct srvsvc_NetFileInfo3) { + .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | + e->share_file_id), + .permissions = permissions, + .num_locks = num_locks, + .path = fullpath, + .user = username, + }; ctr3->count++; -- 2.20.1 From 53b509fead657bf9171e78a3beb54b9c0a463108 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:24:48 +0200 Subject: [PATCH 3/9] srvsvc: Directly use "ctr3->count" instead of "i" To me this was not very transparent, and now that we have "ctr3" a single indirect looks okay Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 96d68bb9f26a0c99d00e92130a2f2c91c7b985e2) --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index b81de3d2b9e..a24ea638624 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -89,7 +89,6 @@ static int enum_file_fn(struct file_id id, (struct file_enum_count *)private_data; struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; - int i = ctr3->count; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -114,9 +113,9 @@ static int enum_file_fn(struct file_id id, fenum->ctx, ctr3->array, struct srvsvc_NetFileInfo3, - i+1); + ctr3->count+1); if ( !f ) { - DEBUG(0,("conn_enum_fn: realloc failed for %d items\n", i+1)); + DBG_ERR("realloc failed for %"PRIu32" items\n", ctr3->count+1); return 0; } ctr3->array = f; @@ -154,7 +153,7 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - ctr3->array[i] = (struct srvsvc_NetFileInfo3) { + ctr3->array[ctr3->count] = (struct srvsvc_NetFileInfo3) { .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id), .permissions = permissions, -- 2.20.1 From e0093fcb7ecb3a9b18c0834a9ca9a19fb23f148f Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:42:50 +0200 Subject: [PATCH 4/9] srvsvc: Use a struct initializer in net_enum_files() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit bda0b3875d965c5cccd09dc09f593229e268ee9b) --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index a24ea638624..d4358d2d49f 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -175,11 +175,9 @@ static WERROR net_enum_files(TALLOC_CTX *ctx, struct srvsvc_NetFileCtr3 **ctr3, uint32_t resume) { - struct file_enum_count f_enum_cnt; - - f_enum_cnt.ctx = ctx; - f_enum_cnt.username = username; - f_enum_cnt.ctr3 = *ctr3; + struct file_enum_count f_enum_cnt = { + .ctx = ctx, .username = username, .ctr3 = *ctr3, + }; share_entry_forall(enum_file_fn, (void *)&f_enum_cnt ); -- 2.20.1 From fd831fc57482eb347474cbb10fb737ffc6ee2cd6 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:32:16 +0200 Subject: [PATCH 5/9] srvsvc: Collect file ids in enum_file_fn() Will be used a few patches down Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 46ab1d478d8c27bb4837bf277f8eae5d59613dd2) --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index d4358d2d49f..9bdd494c2f9 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -53,6 +53,7 @@ struct file_enum_count { TALLOC_CTX *ctx; const char *username; struct srvsvc_NetFileCtr3 *ctr3; + struct file_id *fids; }; struct sess_file_info { @@ -89,6 +90,7 @@ static int enum_file_fn(struct file_id id, (struct file_enum_count *)private_data; struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; + struct file_id *fids = NULL; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -120,10 +122,18 @@ static int enum_file_fn(struct file_id id, } ctr3->array = f; + fids = talloc_realloc( + fenum->ctx, fenum->fids, struct file_id, ctr3->count+1); + if (fids == NULL) { + DBG_ERR("realloc failed for %"PRIu32" items\n", ctr3->count+1); + return 0; + } + fids[ctr3->count] = id; + fenum->fids = fids; + /* need to count the number of locks on a file */ - ZERO_STRUCT( fsp ); - fsp.file_id = id; + fsp = (struct files_struct) { .file_id = id, }; if ( (brl = brl_get_locks(talloc_tos(), &fsp)) != NULL ) { num_locks = brl_num_locks(brl); -- 2.20.1 From 00aa0f9db6413bb9e8dd6938244036b77a3e86d2 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 22 Apr 2020 13:21:40 +0200 Subject: [PATCH 6/9] rpcclient: Use struct initializers in cmd_srvsvc_net_file_enum() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 8c080f28c37a4ada4f3605123a357666881fa3a0) --- source3/rpcclient/cmd_srvsvc.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/source3/rpcclient/cmd_srvsvc.c b/source3/rpcclient/cmd_srvsvc.c index 9c000608463..a9de4608fb9 100644 --- a/source3/rpcclient/cmd_srvsvc.c +++ b/source3/rpcclient/cmd_srvsvc.c @@ -643,9 +643,13 @@ static WERROR cmd_srvsvc_net_file_enum(struct rpc_pipe_client *cli, TALLOC_CTX *mem_ctx, int argc, const char **argv) { - uint32_t info_level = 3; - struct srvsvc_NetFileInfoCtr info_ctr; - struct srvsvc_NetFileCtr3 ctr3; + struct srvsvc_NetFileCtr3 ctr3 = { 0 }; + struct srvsvc_NetFileInfoCtr info_ctr = { + .level = 3, + .ctr = { + .ctr3 = &ctr3, + }, + }; WERROR result; NTSTATUS status; uint32_t preferred_len = 0xffff; @@ -658,14 +662,9 @@ static WERROR cmd_srvsvc_net_file_enum(struct rpc_pipe_client *cli, return WERR_OK; } - if (argc == 2) - info_level = atoi(argv[1]); - - ZERO_STRUCT(info_ctr); - ZERO_STRUCT(ctr3); - - info_ctr.level = info_level; - info_ctr.ctr.ctr3 = &ctr3; + if (argc == 2) { + info_ctr.level = atoi(argv[1]); + } status = dcerpc_srvsvc_NetFileEnum(b, mem_ctx, cli->desthost, -- 2.20.1 From 91c98ef74bbccf9b66847fab3c3521f73021b0ff Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 11 May 2020 11:09:02 +0200 Subject: [PATCH 7/9] rpcclient: Make netfileenum cmd print the path names Needed for the next commit testing netfileenum Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 1d40cc01c2d7f14704c1d9b4b7c42c4cf3450da9) --- source3/rpcclient/cmd_srvsvc.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source3/rpcclient/cmd_srvsvc.c b/source3/rpcclient/cmd_srvsvc.c index a9de4608fb9..478afc68cd1 100644 --- a/source3/rpcclient/cmd_srvsvc.c +++ b/source3/rpcclient/cmd_srvsvc.c @@ -684,6 +684,14 @@ static WERROR cmd_srvsvc_net_file_enum(struct rpc_pipe_client *cli, goto done; } + if (info_ctr.level == 3) { + struct srvsvc_NetFileCtr3 *ret = info_ctr.ctr.ctr3; + uint32_t i; + + for (i=0; icount; i++) { + printf("%s\n", ret->array[i].path); + } + } done: return result; } -- 2.20.1 From 866b20e0c1bf65122ac8426d7e9df9ae1e339901 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 11 May 2020 11:08:54 +0200 Subject: [PATCH 8/9] test: Show that netfileenum is broken Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 8e4583f730abd1a210ec52d5a060dddc4ad850bb) --- selftest/knownfail.d/netfileenum | 1 + source3/script/tests/test_netfileenum.sh | 73 ++++++++++++++++++++++++ source3/selftest/tests.py | 9 +++ 3 files changed, 83 insertions(+) create mode 100644 selftest/knownfail.d/netfileenum create mode 100755 source3/script/tests/test_netfileenum.sh diff --git a/selftest/knownfail.d/netfileenum b/selftest/knownfail.d/netfileenum new file mode 100644 index 00000000000..2d3b6fae117 --- /dev/null +++ b/selftest/knownfail.d/netfileenum @@ -0,0 +1 @@ +^samba3.blackbox.netfileenum.netfileenum\(simpleserver:local\) diff --git a/source3/script/tests/test_netfileenum.sh b/source3/script/tests/test_netfileenum.sh new file mode 100755 index 00000000000..e917ad42862 --- /dev/null +++ b/source3/script/tests/test_netfileenum.sh @@ -0,0 +1,73 @@ +#!/bin/bash +# +# Test rpcclient netfileenum +# +# Copyright (C) 2020 Volker Lendecke + +if [ $# -lt 5 ]; then + echo Usage: $0 \ + SMBCLIENT RPCCLIENT NET SERVER SHARE +exit 1 +fi + +SMBCLIENT="$1"; shift 1 +RPCCLIENT="$1"; shift 1 +NET="$1"; shift 1 +SERVER="$1"; shift 1 +SHARE="$1"; shift 1 + +incdir=$(dirname $0)/../../../testprogs/blackbox +. $incdir/subunit.sh + +failed=0 + +rm -f smbclient-stdin smbclient-stdout smbclient-stderr +mkfifo smbclient-stdin smbclient-stdout smbclient-stderr + +CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE + +${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ + < smbclient-stdin > smbclient-stdout 2>smbclient-stderr & +CLIENT_PID=$! + +sleep 1 + +exec 100>smbclient-stdin 101&100 + +sleep 1 + +testit "Create builtin\\administrators group" \ + "${NET}" groupmap add \ + sid=S-1-5-32-544 unixgroup="${USER}"-group type=builtin || \ + failed=$((failed+1)) +testit "Add ${USER} to builtin\\administrators" \ + "${NET}" groupmap addmem S-1-5-32-544 \ + $("${NET}" lookup name "${USER}" | cut -d' ' -f1) || \ + failed=$((failed+1)) + +"${RPCCLIENT}" "${SERVER}" -U"${USER}"%"${PASSWORD}" -c netfileenum | + grep "$FILE"\$ +RC=$? +testit "netfileenum" test $RC = 0 || failed=$((failed+1)) + +kill ${CLIENT_PID} +rm -f smbclient-stdin smbclient-stdout smbclient-stderr + +testit "Remove ${USER} from builtin\\administrators" \ + "${NET}" groupmap delmem S-1-5-32-544 \ + $("${NET}" lookup name "${USER}" | cut -d' ' -f1) || \ + failed=$((failed+1)) +testit "Remove builtin\\administrators group" \ + "${NET}" groupmap delete \ + sid=S-1-5-32-544 || \ + failed=$((failed+1)) + +testok $0 $failed diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index c00e69f61cc..7134d3d2bdd 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -827,6 +827,15 @@ plantestsuite("samba3.blackbox.open-eintr", "simpleserver:local", '$SERVER_IP', "error_inject"]) +plantestsuite("samba3.blackbox.netfileenum", "simpleserver:local", + [os.path.join(samba3srcdir, + "script/tests/test_netfileenum.sh"), + os.path.join(bindir(), "smbclient"), + os.path.join(bindir(), "rpcclient"), + os.path.join(bindir(), "net"), + '$SERVER_IP', + 'tmp']) + plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local", [os.path.join(samba3srcdir, "script/tests/test_net_tdb.sh"), smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD', -- 2.20.1 From 3712ee8dd9d5a740e8f3cc77c5cedeee7dc3ad02 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:54:25 +0200 Subject: [PATCH 9/9] srvsvc: Move brl_get_locks() out of enum_file_fn() With share_infos.tdb this is a locking order violation: share_infos.tdb is level 4, brlock.tdb is level 2. Avoid this by first walking the share_infos.tdb and then fetching all the brlock entries. Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu May 14 22:06:32 UTC 2020 on sn-devel-184 (cherry picked from commit 01db877c7766387984ef32914eca0b2e817c4c6a) --- selftest/knownfail.d/netfileenum | 1 - source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 30 +++++++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) delete mode 100644 selftest/knownfail.d/netfileenum diff --git a/selftest/knownfail.d/netfileenum b/selftest/knownfail.d/netfileenum deleted file mode 100644 index 2d3b6fae117..00000000000 --- a/selftest/knownfail.d/netfileenum +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.netfileenum.netfileenum\(simpleserver:local\) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index 9bdd494c2f9..0b68b60c29e 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -91,9 +91,6 @@ static int enum_file_fn(struct file_id id, struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; struct file_id *fids = NULL; - files_struct fsp; - struct byte_range_lock *brl; - int num_locks = 0; char *fullpath = NULL; uint32_t permissions; const char *username; @@ -131,15 +128,6 @@ static int enum_file_fn(struct file_id id, fids[ctr3->count] = id; fenum->fids = fids; - /* need to count the number of locks on a file */ - - fsp = (struct files_struct) { .file_id = id, }; - - if ( (brl = brl_get_locks(talloc_tos(), &fsp)) != NULL ) { - num_locks = brl_num_locks(brl); - TALLOC_FREE(brl); - } - if ( strcmp(d->base_name, "." ) == 0 ) { fullpath = talloc_asprintf( fenum->ctx, @@ -167,7 +155,6 @@ static int enum_file_fn(struct file_id id, .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id), .permissions = permissions, - .num_locks = num_locks, .path = fullpath, .user = username, }; @@ -188,11 +175,28 @@ static WERROR net_enum_files(TALLOC_CTX *ctx, struct file_enum_count f_enum_cnt = { .ctx = ctx, .username = username, .ctr3 = *ctr3, }; + uint32_t i; share_entry_forall(enum_file_fn, (void *)&f_enum_cnt ); *ctr3 = f_enum_cnt.ctr3; + /* need to count the number of locks on a file */ + + for (i=0; i<(*ctr3)->count; i++) { + struct files_struct fsp = { .file_id = f_enum_cnt.fids[i], }; + struct byte_range_lock *brl = NULL; + + brl = brl_get_locks(ctx, &fsp); + if (brl == NULL) { + continue; + } + + (*ctr3)->array[i].num_locks = brl_num_locks(brl); + + TALLOC_FREE(brl); + } + return WERR_OK; } -- 2.20.1