From 48df352c7a5d295fd095a2a9f5517b1df3df3365 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 15:45:06 +0200 Subject: [PATCH 01/25] CVE-2021-44141: selftest: remove POSIX test from planned tests for ad_dc_ntvfs environ Just don't run the tests instead of retrofitting them to the skiplist. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back-ported from master commit 5161edbdb28fcfe44abac8c0caf5ad592c638405) Reviewed-by: Jeremy Allison --- selftest/skip | 11 ----------- source3/selftest/tests.py | 1 - 2 files changed, 12 deletions(-) diff --git a/selftest/skip b/selftest/skip index d13cf7cd18b..f9fc7b4b411 100644 --- a/selftest/skip +++ b/selftest/skip @@ -40,19 +40,8 @@ ^samba3.smbtorture_s3.plain.DIR1\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.DIR-CREATETIME\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.DELETE-LN\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.UID-REGRESSION-TEST\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.SHORTNAME-TEST\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-APPEND\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-SYMLINK-ACL\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-SYMLINK-EA\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-OFD-LOCK\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-STREAM-DELETE\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-MKDIR\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-ACL-OPLOCK\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-ACL-SHAREROOT\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server -^samba3.smbtorture_s3.plain.WINDOWS-BAD-SYMLINK\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.RENAME-ACCESS\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.OWNER-RIGHTS\(ad_dc_ntvfs\) # Don't test against the s4 ntvfs server anymore ^samba3.smbtorture_s3.plain.PIDHIGH\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 47e914b1009..42b722dc00a 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -220,7 +220,6 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", for t in posix_tests: plantestsuite("samba3.smbtorture_s3.plain.%s" % t, "nt4_dc_smb1", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) plantestsuite("samba3.smbtorture_s3.crypt.%s" % t, "nt4_dc_smb1", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "-e", "-l $LOCAL_PATH"]) - plantestsuite("samba3.smbtorture_s3.plain.%s" % t, "ad_dc_ntvfs", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) local_tests = [ "LOCAL-SUBSTITUTE", -- 2.30.2 From 0d345ea5637015dfec5746771a96a500bd4cf11a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 12:32:53 +0200 Subject: [PATCH 02/25] CVE-2021-44141: s3/torture: add torture_conn_set_sockopt() wrapper BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit 626b4e5724efe6bde49c112dc31a171854edd180 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/torture/proto.h | 1 + source3/torture/torture.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 18e686089ed..aab53605b29 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -73,6 +73,7 @@ bool torture_close_connection(struct cli_state *c); bool torture_ioctl_test(int dummy); bool torture_chkpath_test(int dummy); NTSTATUS torture_setup_unix_extensions(struct cli_state *cli); +void torture_conn_set_sockopt(struct cli_state *cli); /* The following definitions come from torture/utable.c */ diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 5e263797730..7bf056af2e2 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -452,6 +452,10 @@ bool torture_close_connection(struct cli_state *c) return ret; } +void torture_conn_set_sockopt(struct cli_state *cli) +{ + smbXcli_conn_set_sockopt(cli->conn, sockops); +} /* check if the server produced the expected dos or nt error code */ static bool check_both_error(int line, NTSTATUS status, -- 2.30.2 From 54e153526f89b9305d409fdf545ac4fa1dc5c8bd Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 15:11:20 +0200 Subject: [PATCH 03/25] CVE-2021-44141: s3/torture: add POSIX-LS-WILDCARD test BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit 0ccd24b41c5c474435031d6d7bc8abbffb898050 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- selftest/knownfail.d/samba3.smbtorture_s3 | 2 + source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_posix.c | 251 ++++++++++++++++++++++ source3/torture/torture.c | 4 + source3/wscript_build | 1 + 6 files changed, 260 insertions(+) create mode 100644 selftest/knownfail.d/samba3.smbtorture_s3 create mode 100644 source3/torture/test_posix.c diff --git a/selftest/knownfail.d/samba3.smbtorture_s3 b/selftest/knownfail.d/samba3.smbtorture_s3 new file mode 100644 index 00000000000..f56cf2327e9 --- /dev/null +++ b/selftest/knownfail.d/samba3.smbtorture_s3 @@ -0,0 +1,2 @@ +^samba3.smbtorture_s3.plain.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.crypt.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 42b722dc00a..472ae28a841 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -215,6 +215,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-BLOCKING-LOCK", "POSIX-ACL-OPLOCK", "POSIX-ACL-SHAREROOT", + "POSIX-LS-WILDCARD", ] for t in posix_tests: diff --git a/source3/torture/proto.h b/source3/torture/proto.h index aab53605b29..ba0cb9a0d17 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -85,6 +85,7 @@ bool torture_casetable(int dummy); */ bool run_posix_append(int dummy); +bool run_posix_ls_wildcard_test(int dummy); bool run_case_insensitive_create(int dummy); bool run_nbench2(int dummy); diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c new file mode 100644 index 00000000000..109b83a7dd8 --- /dev/null +++ b/source3/torture/test_posix.c @@ -0,0 +1,251 @@ +/* + Unix SMB/CIFS implementation. + Copyright (C) Ralph Boehme 2020 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "torture/proto.h" +#include "libcli/security/security.h" +#include "libsmb/libsmb.h" +#include "libsmb/clirap.h" +#include "libsmb/proto.h" + +struct posix_test_entry { + const char *name; + const char *target; + const char *expected; + uint64_t returned_size; + bool ok; +}; + +static NTSTATUS posix_ls_fn(const char *mnt, + struct file_info *finfo, + const char *name, + void *_state) +{ + struct posix_test_entry *state = + (struct posix_test_entry *)_state; + + for (; state->name != NULL; state++) { + if (strequal(finfo->name, state->expected)) { + state->ok = true; + state->returned_size = finfo->size; + break; + } + } + + return NT_STATUS_OK; +} + +static void posix_test_entries_reset(struct posix_test_entry *state) +{ + for (; state->name != NULL; state++) { + state->ok = false; + } +} + +static bool posix_test_entry_check(struct posix_test_entry *state, + const char *name, + bool expected, + uint64_t expected_size) +{ + bool result = false; + + for (; state->name != NULL; state++) { + if (strequal(name, state->name)) { + result = state->ok; + break; + } + } + if (state->name == NULL) { + printf("test failed, unknown name: %s\n", name); + return false; + } + + if (expected == result) { + return true; + } + + printf("test failed, %s: %s\n", + expected ? "missing" : "unexpected", + name); + + return false; +} + +/* + Test non-POSIX vs POSIX ls * of symlinks + */ +bool run_posix_ls_wildcard_test(int dummy) +{ + TALLOC_CTX *frame = NULL; + struct cli_state *cli_unix = NULL; + struct cli_state *cli_win = NULL; + uint16_t fnum = (uint16_t)-1; + NTSTATUS status; + const char *file = "file"; + const char *symlnk_dangling = "dangling"; + const char *symlnk_dst_dangling = "xxxxxxx"; + const char *symlnk_in_share = "symlnk_in_share"; + const char *symlnk_dst_in_share = file; + const char *symlnk_outside_share = "symlnk_outside_share"; + const char *symlnk_dst_outside_share = "/etc/passwd"; + struct posix_test_entry state[] = { + { + .name = symlnk_dangling, + .target = symlnk_dst_dangling, + .expected = symlnk_dangling, + }, { + .name = symlnk_in_share, + .target = symlnk_dst_in_share, + .expected = symlnk_in_share, + }, { + .name = symlnk_outside_share, + .target = symlnk_dst_outside_share, + .expected = symlnk_outside_share, + }, { + .name = NULL, + } + }; + int i; + bool correct = false; + + frame = talloc_stackframe(); + + printf("Starting POSIX-LS-WILDCARD test\n"); + + if (!torture_open_connection(&cli_unix, 0)) { + TALLOC_FREE(frame); + return false; + } + + if (!torture_open_connection(&cli_win, 0)) { + TALLOC_FREE(frame); + return false; + } + + torture_conn_set_sockopt(cli_unix); + torture_conn_set_sockopt(cli_win); + + status = torture_setup_unix_extensions(cli_unix); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + status = cli_posix_open(cli_unix, + file, + O_RDWR|O_CREAT, + 0666, + &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_open of %s failed error %s\n", + file, + nt_errstr(status)); + goto out; + } + + status = cli_close(cli_unix, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + for (i = 0; state[i].name != NULL; i++) { + status = cli_posix_symlink(cli_unix, + state[i].target, + state[i].name); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX symlink of %s failed (%s)\n", + symlnk_dangling, nt_errstr(status)); + goto out; + } + } + + printf("Doing Windows ls *\n"); + + status = cli_list(cli_win, "*", 0, posix_ls_fn, state); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + + if (!posix_test_entry_check(state, symlnk_dangling, false, 0)) { + goto out; + } + if (!posix_test_entry_check(state, symlnk_outside_share, false, 0)) { + goto out; + } + if (!posix_test_entry_check(state, symlnk_in_share, true, 0)) { + goto out; + } + + posix_test_entries_reset(state); + + printf("Doing POSIX ls *\n"); + + status = cli_list(cli_unix, "*", 0, posix_ls_fn, state); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + + if (!posix_test_entry_check(state, + symlnk_dangling, + true, + strlen(symlnk_dst_dangling))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_outside_share, + true, + strlen(symlnk_dst_outside_share))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_in_share, + true, + strlen(symlnk_dst_in_share))) { + goto out; + } + + printf("POSIX-LS-WILDCARD test passed\n"); + correct = true; + +out: + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + if (!torture_close_connection(cli_unix)) { + correct = false; + } + if (!torture_close_connection(cli_win)) { + correct = false; + } + + TALLOC_FREE(frame); + return correct; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 7bf056af2e2..05d4fa933da 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14614,6 +14614,10 @@ static struct { .name = "POSIX-ACL-SHAREROOT", .fn = run_posix_acl_shareroot_test, }, + { + .name = "POSIX-LS-WILDCARD", + .fn = run_posix_ls_wildcard_test, + }, { .name = "WINDOWS-BAD-SYMLINK", .fn = run_symlink_open_test, diff --git a/source3/wscript_build b/source3/wscript_build index d86a9fcadbf..30b894d0eb1 100644 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -1178,6 +1178,7 @@ bld.SAMBA3_BINARY('smbtorture' + bld.env.suffix3, torture/test_async_echo.c torture/test_addrchange.c torture/test_posix_append.c + torture/test_posix.c torture/test_nttrans_create.c torture/test_nttrans_fsctl.c torture/test_case_insensitive.c -- 2.30.2 From bcc6da3574a4d64faec2c8b7c398826141132e5b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 15:24:11 +0200 Subject: [PATCH 04/25] CVE-2021-44141: s3/torture: add POSIX-LS-SINGLE test Note that uses SMB2 for the "Windows client" (aka non-POSIX) connection as SMB1 directory listing code translates a directory listing with a search mask that matches an existing file to a CREATE which won't cut it for our test as we're targetting the directory listing code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit c8a2530b8db0e5c9b204577430fe8399bb8ff694 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- selftest/knownfail.d/samba3.smbtorture_s3 | 2 + source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_posix.c | 187 ++++++++++++++++++++++ source3/torture/torture.c | 4 + 5 files changed, 195 insertions(+) diff --git a/selftest/knownfail.d/samba3.smbtorture_s3 b/selftest/knownfail.d/samba3.smbtorture_s3 index f56cf2327e9..388980c2c74 100644 --- a/selftest/knownfail.d/samba3.smbtorture_s3 +++ b/selftest/knownfail.d/samba3.smbtorture_s3 @@ -1,2 +1,4 @@ ^samba3.smbtorture_s3.plain.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.plain.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.crypt.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 472ae28a841..165e43adaf2 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -216,6 +216,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-ACL-OPLOCK", "POSIX-ACL-SHAREROOT", "POSIX-LS-WILDCARD", + "POSIX-LS-SINGLE", ] for t in posix_tests: diff --git a/source3/torture/proto.h b/source3/torture/proto.h index ba0cb9a0d17..87d80d930e4 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -86,6 +86,7 @@ bool torture_casetable(int dummy); bool run_posix_append(int dummy); bool run_posix_ls_wildcard_test(int dummy); +bool run_posix_ls_single_test(int dummy); bool run_case_insensitive_create(int dummy); bool run_nbench2(int dummy); diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c index 109b83a7dd8..e7e65c7b64c 100644 --- a/source3/torture/test_posix.c +++ b/source3/torture/test_posix.c @@ -22,6 +22,10 @@ #include "libsmb/libsmb.h" #include "libsmb/clirap.h" #include "libsmb/proto.h" +#include "../libcli/smb/smbXcli_base.h" + +extern struct cli_credentials *torture_creds; +extern fstring host, workgroup, share, password, username, myname; struct posix_test_entry { const char *name; @@ -249,3 +253,186 @@ out: TALLOC_FREE(frame); return correct; } + +/* + Test non-POSIX vs POSIX ls single of symlinks + */ +bool run_posix_ls_single_test(int dummy) +{ + TALLOC_CTX *frame = NULL; + struct cli_state *cli_unix = NULL; + struct cli_state *cli_win = NULL; + uint16_t fnum = (uint16_t)-1; + NTSTATUS status; + const char *file = "file"; + const char *symlnk_dangling = "dangling"; + const char *symlnk_dst_dangling = "xxxxxxx"; + const char *symlnk_in_share = "symlnk_in_share"; + const char *symlnk_dst_in_share = file; + const char *symlnk_outside_share = "symlnk_outside_share"; + const char *symlnk_dst_outside_share = "/etc/passwd"; + struct posix_test_entry state[] = { + { + .name = symlnk_dangling, + .target = symlnk_dst_dangling, + .expected = symlnk_dangling, + }, { + .name = symlnk_in_share, + .target = symlnk_dst_in_share, + .expected = symlnk_in_share, + }, { + .name = symlnk_outside_share, + .target = symlnk_dst_outside_share, + .expected = symlnk_outside_share, + }, { + .name = NULL, + } + }; + int i; + bool correct = false; + + frame = talloc_stackframe(); + + printf("Starting POSIX-LS-SINGLE test\n"); + + if (!torture_open_connection(&cli_unix, 0)) { + TALLOC_FREE(frame); + return false; + } + + if (!torture_init_connection(&cli_win)) { + TALLOC_FREE(frame); + return false; + } + + status = smbXcli_negprot(cli_win->conn, + cli_win->timeout, + lp_client_min_protocol(), + lp_client_max_protocol()); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + TALLOC_FREE(frame); + return false; + } + + status = cli_session_setup_creds(cli_win, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_sesssetup returned %s\n", nt_errstr(status)); + TALLOC_FREE(frame); + return false; + } + + status = cli_tree_connect(cli_win, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + TALLOC_FREE(frame); + return false; + } + torture_conn_set_sockopt(cli_unix); + torture_conn_set_sockopt(cli_win); + + status = torture_setup_unix_extensions(cli_unix); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + status = cli_posix_open(cli_unix, + file, + O_RDWR|O_CREAT, + 0666, + &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_open of %s failed error %s\n", + file, + nt_errstr(status)); + goto out; + } + + status = cli_close(cli_unix, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + for (i = 0; state[i].name != NULL; i++) { + status = cli_posix_symlink(cli_unix, + state[i].target, + state[i].name); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX symlink of %s failed (%s)\n", + symlnk_dangling, nt_errstr(status)); + goto out; + } + } + + printf("Doing Windows ls single\n"); + + cli_list(cli_win, symlnk_dangling, 0, posix_ls_fn, state); + cli_list(cli_win, symlnk_outside_share, 0, posix_ls_fn, state); + cli_list(cli_win, symlnk_in_share, 0, posix_ls_fn, state); + + if (!posix_test_entry_check(state, symlnk_dangling, false, 0)) { + goto out; + } + if (!posix_test_entry_check(state, symlnk_outside_share, false, 0)) { + goto out; + } + if (!posix_test_entry_check(state, symlnk_in_share, true, 0)) { + goto out; + } + + posix_test_entries_reset(state); + + printf("Doing POSIX ls single\n"); + + cli_list(cli_unix, symlnk_dangling, 0, posix_ls_fn, state); + cli_list(cli_unix, symlnk_outside_share, 0, posix_ls_fn, state); + cli_list(cli_unix, symlnk_in_share, 0, posix_ls_fn, state); + + if (!posix_test_entry_check(state, + symlnk_dangling, + true, + strlen(symlnk_dst_dangling))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_outside_share, + true, + strlen(symlnk_dst_outside_share))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_in_share, + true, + strlen(symlnk_dst_in_share))) { + goto out; + } + + printf("POSIX-LS-SINGLE test passed\n"); + correct = true; + +out: + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + if (!torture_close_connection(cli_unix)) { + correct = false; + } + if (!torture_close_connection(cli_win)) { + correct = false; + } + + TALLOC_FREE(frame); + return correct; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 05d4fa933da..090dd2b923a 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14618,6 +14618,10 @@ static struct { .name = "POSIX-LS-WILDCARD", .fn = run_posix_ls_wildcard_test, }, + { + .name = "POSIX-LS-SINGLE", + .fn = run_posix_ls_single_test, + }, { .name = "WINDOWS-BAD-SYMLINK", .fn = run_symlink_open_test, -- 2.30.2 From a5ed4ae5833be089a6de83c240ecc85c119afd35 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 15:32:34 +0200 Subject: [PATCH 05/25] CVE-2021-44141: s3/torture: add POSIX-READLINK test BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit 562ae8eb2369741e96c85fd22a5b65eb8c1863a4 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_posix.c | 144 +++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 4 + 4 files changed, 150 insertions(+) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 165e43adaf2..2ff2d6723ba 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -217,6 +217,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-ACL-SHAREROOT", "POSIX-LS-WILDCARD", "POSIX-LS-SINGLE", + "POSIX-READLINK", ] for t in posix_tests: diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 87d80d930e4..01ef479a8e9 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -87,6 +87,7 @@ bool torture_casetable(int dummy); bool run_posix_append(int dummy); bool run_posix_ls_wildcard_test(int dummy); bool run_posix_ls_single_test(int dummy); +bool run_posix_readlink_test(int dummy); bool run_case_insensitive_create(int dummy); bool run_nbench2(int dummy); diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c index e7e65c7b64c..b84123907cc 100644 --- a/source3/torture/test_posix.c +++ b/source3/torture/test_posix.c @@ -436,3 +436,147 @@ out: TALLOC_FREE(frame); return correct; } + +/* + Test POSIX readlink of symlinks + */ +bool run_posix_readlink_test(int dummy) +{ + TALLOC_CTX *frame = NULL; + struct cli_state *cli_unix = NULL; + uint16_t fnum = (uint16_t)-1; + NTSTATUS status; + const char *file = "file"; + const char *symlnk_dangling = "dangling"; + const char *symlnk_dst_dangling = "xxxxxxx"; + const char *symlnk_in_share = "symlnk_in_share"; + const char *symlnk_dst_in_share = file; + const char *symlnk_outside_share = "symlnk_outside_share"; + const char *symlnk_dst_outside_share = "/etc/passwd"; + struct posix_test_entry state[] = { + { + .name = symlnk_dangling, + .target = symlnk_dst_dangling, + .expected = symlnk_dangling, + }, { + .name = symlnk_in_share, + .target = symlnk_dst_in_share, + .expected = symlnk_in_share, + }, { + .name = symlnk_outside_share, + .target = symlnk_dst_outside_share, + .expected = symlnk_outside_share, + }, { + .name = NULL, + } + }; + int i; + bool correct = false; + + frame = talloc_stackframe(); + + printf("Starting POSIX-READLINK test\n"); + + if (!torture_open_connection(&cli_unix, 0)) { + TALLOC_FREE(frame); + return false; + } + + torture_conn_set_sockopt(cli_unix); + + status = torture_setup_unix_extensions(cli_unix); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + status = cli_posix_open(cli_unix, + file, + O_RDWR|O_CREAT, + 0666, + &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_open of %s failed error %s\n", + file, + nt_errstr(status)); + goto out; + } + + status = cli_close(cli_unix, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + for (i = 0; state[i].name != NULL; i++) { + status = cli_posix_symlink(cli_unix, + state[i].target, + state[i].name); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX symlink of %s failed (%s)\n", + symlnk_dangling, nt_errstr(status)); + goto out; + } + } + + for (i = 0; state[i].name != NULL; i++) { + char *target = NULL; + + status = cli_posix_readlink(cli_unix, + state[i].name, + talloc_tos(), + &target); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX readlink on %s failed (%s)\n", + state[i].name, nt_errstr(status)); + goto out; + } + if (strequal(target, state[i].target)) { + state[i].ok = true; + state[i].returned_size = strlen(target); + } + } + + if (!posix_test_entry_check(state, + symlnk_dangling, + true, + strlen(symlnk_dst_dangling))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_outside_share, + true, + strlen(symlnk_dst_outside_share))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_in_share, + true, + strlen(symlnk_dst_in_share))) { + goto out; + } + + printf("POSIX-READLINK test passed\n"); + correct = true; + +out: + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + if (!torture_close_connection(cli_unix)) { + correct = false; + } + + TALLOC_FREE(frame); + return correct; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 090dd2b923a..fe84a274bbd 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14622,6 +14622,10 @@ static struct { .name = "POSIX-LS-SINGLE", .fn = run_posix_ls_single_test, }, + { + .name = "POSIX-READLINK", + .fn = run_posix_readlink_test, + }, { .name = "WINDOWS-BAD-SYMLINK", .fn = run_symlink_open_test, -- 2.30.2 From 1617128413889ea6b4ee819120685d066de7a8ca Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 15 Oct 2020 15:36:42 +0200 Subject: [PATCH 06/25] CVE-2021-44141: s3/torture: add POSIX-STAT test BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit a63a39729489be073e0fe882a1f470f82dddfce6 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_posix.c | 141 +++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 4 + 4 files changed, 147 insertions(+) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 2ff2d6723ba..cab49783438 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -218,6 +218,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-LS-WILDCARD", "POSIX-LS-SINGLE", "POSIX-READLINK", + "POSIX-STAT", ] for t in posix_tests: diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 01ef479a8e9..521c662e2fb 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -88,6 +88,7 @@ bool run_posix_append(int dummy); bool run_posix_ls_wildcard_test(int dummy); bool run_posix_ls_single_test(int dummy); bool run_posix_readlink_test(int dummy); +bool run_posix_stat_test(int dummy); bool run_case_insensitive_create(int dummy); bool run_nbench2(int dummy); diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c index b84123907cc..4a7f12e2812 100644 --- a/source3/torture/test_posix.c +++ b/source3/torture/test_posix.c @@ -580,3 +580,144 @@ out: TALLOC_FREE(frame); return correct; } + +/* + Test POSIX stat of symlinks + */ +bool run_posix_stat_test(int dummy) +{ + TALLOC_CTX *frame = NULL; + struct cli_state *cli_unix = NULL; + uint16_t fnum = (uint16_t)-1; + NTSTATUS status; + const char *file = "file"; + const char *symlnk_dangling = "dangling"; + const char *symlnk_dst_dangling = "xxxxxxx"; + const char *symlnk_in_share = "symlnk_in_share"; + const char *symlnk_dst_in_share = file; + const char *symlnk_outside_share = "symlnk_outside_share"; + const char *symlnk_dst_outside_share = "/etc/passwd"; + struct posix_test_entry state[] = { + { + .name = symlnk_dangling, + .target = symlnk_dst_dangling, + .expected = symlnk_dangling, + }, { + .name = symlnk_in_share, + .target = symlnk_dst_in_share, + .expected = symlnk_in_share, + }, { + .name = symlnk_outside_share, + .target = symlnk_dst_outside_share, + .expected = symlnk_outside_share, + }, { + .name = NULL, + } + }; + int i; + bool correct = false; + + frame = talloc_stackframe(); + + printf("Starting POSIX-STAT test\n"); + + if (!torture_open_connection(&cli_unix, 0)) { + TALLOC_FREE(frame); + return false; + } + + torture_conn_set_sockopt(cli_unix); + + status = torture_setup_unix_extensions(cli_unix); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + status = cli_posix_open(cli_unix, + file, + O_RDWR|O_CREAT, + 0666, + &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_open of %s failed error %s\n", + file, + nt_errstr(status)); + goto out; + } + + status = cli_close(cli_unix, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_close failed %s\n", nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + for (i = 0; state[i].name != NULL; i++) { + status = cli_posix_symlink(cli_unix, + state[i].target, + state[i].name); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX symlink of %s failed (%s)\n", + symlnk_dangling, nt_errstr(status)); + goto out; + } + } + + for (i = 0; state[i].name != NULL; i++) { + SMB_STRUCT_STAT sbuf; + + status = cli_posix_stat(cli_unix, + state[i].name, + &sbuf); + if (!NT_STATUS_IS_OK(status)) { + printf("POSIX stat on %s failed (%s)\n", + state[i].name, nt_errstr(status)); + continue; + } + state[i].ok = true; + state[i].returned_size = sbuf.st_ex_size; + } + + if (!posix_test_entry_check(state, + symlnk_dangling, + true, + strlen(symlnk_dst_dangling))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_outside_share, + true, + strlen(symlnk_dst_outside_share))) + { + goto out; + } + if (!posix_test_entry_check(state, + symlnk_in_share, + true, + strlen(symlnk_dst_in_share))) { + goto out; + } + + printf("POSIX-STAT test passed\n"); + correct = true; + +out: + cli_posix_unlink(cli_unix, file); + cli_posix_unlink(cli_unix, symlnk_dangling); + cli_posix_unlink(cli_unix, symlnk_in_share); + cli_posix_unlink(cli_unix, symlnk_outside_share); + + if (!torture_close_connection(cli_unix)) { + correct = false; + } + + TALLOC_FREE(frame); + return correct; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index fe84a274bbd..18844d49f8d 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14626,6 +14626,10 @@ static struct { .name = "POSIX-READLINK", .fn = run_posix_readlink_test, }, + { + .name = "POSIX-STAT", + .fn = run_posix_stat_test, + }, { .name = "WINDOWS-BAD-SYMLINK", .fn = run_symlink_open_test, -- 2.30.2 From 791eef50e65f85401fd7ba0a2d2d85faf59ad04b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 16 Oct 2020 14:35:10 +0200 Subject: [PATCH 07/25] CVE-2021-44141: smbd: add vfs_stat() Deals with POSIX paths and either calls lstat() for POSIX or stat(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Back ported from commit 9d075d8072210a9806141021c7758575a411ffcf Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/smbd/proto.h | 2 ++ source3/smbd/vfs.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index ae5f82c2de5..ebe42304f2f 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1323,6 +1323,8 @@ NTSTATUS check_reduced_name(connection_struct *conn, NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, const struct smb_filename *smb_fname, struct smb_request *smbreq); +int vfs_stat(struct connection_struct *conn, + struct smb_filename *smb_fname); int vfs_stat_smb_basename(struct connection_struct *conn, const struct smb_filename *smb_fname_in, SMB_STRUCT_STAT *psbuf); diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index c4978fe5212..5b9ae17768c 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1498,6 +1498,18 @@ NTSTATUS check_reduced_name(connection_struct *conn, return NT_STATUS_OK; } +/* + * Ensure LSTAT is called for POSIX paths. + */ +int vfs_stat(struct connection_struct *conn, + struct smb_filename *smb_fname) +{ + if (smb_fname->flags & SMB_FILENAME_POSIX_PATH) { + return SMB_VFS_LSTAT(conn, smb_fname); + } + return SMB_VFS_STAT(conn, smb_fname); +} + /** * XXX: This is temporary and there should be no callers of this once * smb_filename is plumbed through all path based operations. -- 2.30.2 From 4fb6d6bebce0e4ebeb73d173009405f8629261a0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 23 Nov 2021 21:10:10 -0800 Subject: [PATCH 08/25] CVE-2021-44141: s3: smbd: dir.c: Fix directory listing to use vfs_stat() calls. If we're in SMB1+POSIX mode, we can see symlinks in the directory entries (as we're now using vfs_stat()). Instead of openat_pathref_fsp() which we don't have in 4.13, use check_name() to hide regular file entries that under Windows are symlinks outside the share. Remove samba3.smbtorture_s3.crypt.POSIX-LS-WILDCARD.smbtorture knownfail. samba3.smbtorture_s3.crypt.POSIX-LS-SINGLE.smbtorture still fails but we'll address that in the next commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/samba3.smbtorture_s3 | 2 - source3/smbd/dir.c | 51 +++++++++++++++++++++-- source3/smbd/trans2.c | 2 +- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/selftest/knownfail.d/samba3.smbtorture_s3 b/selftest/knownfail.d/samba3.smbtorture_s3 index 388980c2c74..1cae8cb37b8 100644 --- a/selftest/knownfail.d/samba3.smbtorture_s3 +++ b/selftest/knownfail.d/samba3.smbtorture_s3 @@ -1,4 +1,2 @@ -^samba3.smbtorture_s3.plain.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.crypt.POSIX-LS-WILDCARD.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.plain.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 51cd22d6c5a..7d82a3c4489 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -493,9 +493,11 @@ static char *dptr_ReadDirName(TALLOC_CTX *ctx, smb_fname_base = (struct smb_filename) { .base_name = pathreal, .twrp = dptr->smb_dname->twrp, + .flags = dptr->smb_dname->flags, }; - if (SMB_VFS_STAT(dptr->conn, &smb_fname_base) == 0) { + ret = vfs_stat(dptr->conn, &smb_fname_base); + if (ret == 0) { *pst = smb_fname_base.st; name = talloc_strdup(ctx, dptr->wcard); goto clean; @@ -872,6 +874,7 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx, .base_name = pathreal, .st = sbuf, .twrp = dirptr->smb_dname->twrp, + .flags = dirptr->smb_dname->flags, }; ok = mode_fn(ctx, private_data, &smb_fname, get_dosmode, &mode); @@ -891,6 +894,42 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx, continue; } + /* Ensure we have a VALID_STAT. */ + if (!VALID_STAT(smb_fname.st)) { + int ret; + if (mode & FILE_ATTRIBUTE_REPARSE_POINT) { + /* + * An MSDFS link. mode_fn found it, + * but doesn't set smb_fname.st. + */ + ret = SMB_VFS_LSTAT(conn, &smb_fname); + } else { + ret = vfs_stat(conn, &smb_fname); + } + if (ret == -1) { + TALLOC_FREE(dname); + TALLOC_FREE(fname); + TALLOC_FREE(pathreal); + continue; + } + } + + if (!S_ISLNK(smb_fname.st.st_ex_mode) && !isdots) { + /* + * If it's not a symlink, and not one of + * the expected dot files, then the resolved + * name must be under the share definition. + */ + NTSTATUS status; + status = check_name(conn, &smb_fname); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(dname); + TALLOC_FREE(fname); + TALLOC_FREE(pathreal); + continue; + } + } + if (ask_sharemode && !S_ISDIR(smb_fname.st.st_ex_mode)) { struct timespec write_time_ts; struct file_id fileid; @@ -1009,7 +1048,9 @@ static bool smbd_dirptr_8_3_mode_fn(TALLOC_CTX *ctx, connection_struct *conn = (connection_struct *)private_data; if (!VALID_STAT(smb_fname->st)) { - if ((SMB_VFS_STAT(conn, smb_fname)) != 0) { + int ret; + ret = vfs_stat(conn, smb_fname); + if (ret != 0) { DEBUG(5,("smbd_dirptr_8_3_mode_fn: " "Couldn't stat [%s]. Error " "= %s\n", @@ -1265,7 +1306,7 @@ bool is_visible_file(connection_struct *conn, NULL, pst, dir_path->twrp, - 0); + dir_path->flags); if (smb_fname_base == NULL) { ret = false; goto out; @@ -1276,7 +1317,9 @@ bool is_visible_file(connection_struct *conn, * checks *might* have passed if the file was present. */ if (!VALID_STAT(*pst)) { - if (SMB_VFS_STAT(conn, smb_fname_base) != 0) { + int valret; + valret = vfs_stat(conn, smb_fname_base); + if (valret != 0) { ret = true; goto out; } diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 7acde285a90..57385b8199e 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -1786,7 +1786,7 @@ static bool smbd_dirptr_lanman2_mode_fn(TALLOC_CTX *ctx, bool ms_dfs_link = false; uint32_t mode = 0; - if (INFO_LEVEL_IS_UNIX(state->info_level)) { + if (smb_fname->flags & SMB_FILENAME_POSIX_PATH) { if (SMB_VFS_LSTAT(state->conn, smb_fname) != 0) { DEBUG(5,("smbd_dirptr_lanman2_mode_fn: " "Couldn't lstat [%s] (%s)\n", -- 2.30.2 From ccc9abe3c46b8639e1d48724bf3fc5a18f56e5c8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 23 Nov 2021 21:17:05 -0800 Subject: [PATCH 09/25] CVE-2021-44141: s3: smbd: findfirst. Fix POSIX-LS-WILDCARD test. This one is subtle. When the directory listing code for SMB1+POSIX was written, it was expected that all info levels sent when SMB1+POSIX was negotiaged would be SMB_FIND_FILE_UNIX or SMB_FIND_FILE_UNIX2. For the Linux SMB1+POSIX cifsfs client it uses those UNIX or UNIX2 info levels, but for our smbclient and smbtorture tests we never bothered to convert, so we still use the Windows info levels when we call cli_list() even after negotiating SMB1+POSIX. This means that when we send a wildcard match name of "dir/to/list/bad_symlink", where bad_symlink is a symlink that points outside the share, the filename_convert() call calls check_name("dir/to/list/bad_symlink") - i.e. it doesn't first remove the last component wildcard. So check_name() returns ACCESS_DENIED for the complete path (it's outside the share). Fix this by moving the flag set UCF_UNIX_NAME_LOOKUP from the UNIX info level check to always be set if the broad "req->posix_pathnames" bool is true, which means this is guarenteed to be SMB1+POSIX findfirst call. The UCF_UNIX_NAME_LOOKUP flag being set avoids the check_name() on the full directory path plus the wildcard inside filename_convert() or filename_convert_with_privilege(). check_name() is *still* called on the directory path without the wildcard, as described below. "bad_symlink" is then choppd off and used as the directory list mask, as it is intended to be. We are still protected from "dir/to/list" being a symlink pointing outside the share as it goes through the full SMB_VFS_CREATE_FILE() path, which rejects all out of share directory opens. Remove selftest/knownfail.d/samba3.smbtorture_s3 as we now pass POSIX-LS-WILDCARD and all the POSIX tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/samba3.smbtorture_s3 | 2 -- source3/smbd/trans2.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.smbtorture_s3 diff --git a/selftest/knownfail.d/samba3.smbtorture_s3 b/selftest/knownfail.d/samba3.smbtorture_s3 deleted file mode 100644 index 1cae8cb37b8..00000000000 --- a/selftest/knownfail.d/samba3.smbtorture_s3 +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.crypt.POSIX-LS-SINGLE.smbtorture\(nt4_dc_smb1\) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 57385b8199e..812a652d65c 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2793,7 +2793,6 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da reply_nterror(req, NT_STATUS_INVALID_LEVEL); goto out; } - ucf_flags |= UCF_UNIX_NAME_LOOKUP; break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -2801,6 +2800,7 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da } if (req->posix_pathnames) { + ucf_flags |= UCF_UNIX_NAME_LOOKUP; srvstr_get_path_wcard_posix(talloc_tos(), params, req->flags2, -- 2.30.2 From da4ccd6dbfebe1b2f02ff6be06db4d589b6d580f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:54:47 -0800 Subject: [PATCH 10/25] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() to use torture_result() instead of printf. I think this test pre-dates torture_result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 60 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index c7bb88a6d53..083eef47771 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -315,7 +315,11 @@ static bool test_one_file(struct torture_context *tctx, fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { - printf("ERROR: open of %s failed (%s)\n", fname, smbcli_errstr(cli->tree)); + torture_result(tctx, + TORTURE_FAIL, + __location__"ERROR: open of %s failed (%s)\n", + fname, + smbcli_errstr(cli->tree)); ret = false; goto done; } @@ -342,9 +346,11 @@ static bool test_one_file(struct torture_context *tctx, } if (!NT_STATUS_IS_OK(levels[i].status)) { - printf("search level %s(%d) failed - %s\n", - levels[i].name, (int)levels[i].level, - nt_errstr(levels[i].status)); + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) failed - %s\n", + levels[i].name, (int)levels[i].level, + nt_errstr(levels[i].status)); ret = false; continue; } @@ -362,7 +368,9 @@ static bool test_one_file(struct torture_context *tctx, expected_status = STATUS_NO_MORE_FILES; } if (!NT_STATUS_EQUAL(status, expected_status)) { - printf("search level %s(%d) should fail with %s - %s\n", + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) should fail with %s - %s\n", levels[i].name, (int)levels[i].level, nt_errstr(expected_status), nt_errstr(status)); @@ -399,8 +407,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if ((s->sname1.field1) != (v.sname2.out.field2)) { \ - printf("(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ + __location__, \ #sname1, #field1, (int)s->sname1.field1, \ #sname2, #field2, (int)v.sname2.out.field2); \ ret = false; \ @@ -411,8 +421,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != (~1 & nt_time_to_unix(v.sname2.out.field2))) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, timestring(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -423,8 +435,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != v.sname2.out.field2) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, nt_time_string(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -435,8 +449,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (!s->sname1.field1 || strcmp(s->sname1.field1, v.sname2.out.field2.s)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -449,8 +465,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, v.sname2.out.field2.s) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -463,8 +481,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, fname) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ fname); \ ret = false; \ @@ -476,8 +496,10 @@ static bool test_one_file(struct torture_context *tctx, if (s) { \ if (!s->sname1.field1 || \ strcmp(s->sname1.field1, fname)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ fname); \ ret = false; \ -- 2.30.2 From e89feda6f4ae4d829263f061e5fdf3de841fe54b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:18:47 -0800 Subject: [PATCH 11/25] CVE-2021-44141: s4: torture: In raw.search:test_one_file() remove the leading '\\' in the test filenames. We'll soon be using this under SMB1+POSIX and neither Windows or POSIX need a leading '\\' (and SMB1+POSIX sees the '\\' as part of the name). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 083eef47771..b18c25968ae 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -304,8 +304,8 @@ static bool test_one_file(struct torture_context *tctx, { bool ret = true; int fnum; - const char *fname = "\\torture_search.txt"; - const char *fname2 = "\\torture_search-NOTEXIST.txt"; + const char *fname = "torture_search.txt"; + const char *fname2 = "torture_search-NOTEXIST.txt"; NTSTATUS status; int i; union smb_fileinfo all_info, alt_info, name_info, internal_info; @@ -580,20 +580,20 @@ static bool test_one_file(struct torture_context *tctx, short_name, alt_info, alt_name_info, fname, STR_UNICODE); } - CHECK_NAME("STANDARD", standard, name, fname+1, 0); - CHECK_NAME("EA_SIZE", ea_size, name, fname+1, 0); - CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("STANDARD", standard, name, fname, 0); + CHECK_NAME("EA_SIZE", ea_size, name, fname, 0); + CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname, STR_TERMINATE_ASCII); if (name_info_supported) { - CHECK_NAME("NAME_INFO", name_info, name, fname+1, + CHECK_NAME("NAME_INFO", name_info, name, fname, STR_TERMINATE_ASCII); } - CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname, STR_TERMINATE_ASCII); if (internal_info_supported) { CHECK_VAL("ID_FULL_DIRECTORY_INFO", id_full_directory_info, -- 2.30.2 From f7bffde6d361f61a8e7567160b3ff695b6908188 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 11:48:42 -0800 Subject: [PATCH 12/25] CVE-2021-44141: s3: smbd: Tighten up info level checks for SMB1+POSIX to make sure POSIX was negotiated first. Add knownfail file knownfail.d/posix_infolevel_fails for tests that don't currently negotiate SMB1+POSIX before using SMB1+POSIX calls. These are: samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) samba3.smbtorture_s3.crypt.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.raw.search.one\ file\ search.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 8 +++ source3/smbd/trans2.c | 65 +++++++++++++++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails new file mode 100644 index 00000000000..ee4d6318201 --- /dev/null +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -0,0 +1,8 @@ +^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.crypt.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) +^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* +^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* +^samba3.unix.info2.info2\(nt4_dc_smb1\) +^samba3.raw.search.one\ file\ search.* diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 812a652d65c..6ec3fbbee22 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2793,6 +2793,10 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da reply_nterror(req, NT_STATUS_INVALID_LEVEL); goto out; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + goto out; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -3322,6 +3326,10 @@ resume_key = %d resume name = %s continue=%d level = %d\n", reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -5215,8 +5223,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, uint32_t access_mask = 0; size_t len = 0; - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - return NT_STATUS_INVALID_LEVEL; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + return NT_STATUS_INVALID_LEVEL; + } + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } } DEBUG(5,("smbd_do_qfilepathinfo: %s (%s) level=%d max_data=%u\n", @@ -6049,9 +6062,15 @@ static void call_trans2qfilepathinfo(connection_struct *conn, DEBUG(3,("call_trans2qfilepathinfo: TRANSACT2_QFILEINFO: level = %d\n", info_level)); - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - reply_nterror(req, NT_STATUS_INVALID_LEVEL); - return; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } /* Initial check for valid fsp ptr. */ @@ -6148,6 +6167,10 @@ static void call_trans2qfilepathinfo(connection_struct *conn, reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } if (info_level == SMB_QUERY_FILE_UNIX_BASIC || info_level == SMB_QUERY_FILE_UNIX_INFO2 || info_level == SMB_QUERY_FILE_UNIX_LINK || @@ -8995,8 +9018,13 @@ NTSTATUS smbd_do_setfilepathinfo(connection_struct *conn, *ret_data_size = 0; - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - return NT_STATUS_INVALID_LEVEL; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + return NT_STATUS_INVALID_LEVEL; + } + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } } if (!CAN_WRITE(conn)) { @@ -9308,6 +9336,17 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,2); + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + smb_fname = cp_smb_filename(talloc_tos(), fsp->fsp_name); if (smb_fname == NULL) { reply_nterror(req, NT_STATUS_NO_MEMORY); @@ -9385,6 +9424,18 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,0); + + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + if (req->posix_pathnames) { srvstr_get_path_posix(req, params, -- 2.30.2 From 4128ac4edd00e1657888ae386c5b2e18932c99fc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 20 Nov 2021 20:17:11 -0800 Subject: [PATCH 13/25] CVE-2021-44141: s3: smbclient: Give a message if we try and use any POSIX command without negotiating POSIX first. Ensure we only use a POSIX command if POSIX is set up. Issue the message: Command "posix" must be issued before the "XXXX" command can be used. After the parameter parsing has been done. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/client/client.c | 79 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/source3/client/client.c b/source3/client/client.c index 8c7ceb644aa..d7ead9055a5 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2798,6 +2798,11 @@ static int cmd_posix_open(void) d_printf("posix_open 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_open\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", popt_get_cmdline_auth_info(), @@ -2857,6 +2862,11 @@ static int cmd_posix_mkdir(void) d_printf("posix_mkdir 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_mkdir\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", popt_get_cmdline_auth_info(), @@ -2889,6 +2899,11 @@ static int cmd_posix_unlink(void) d_printf("posix_unlink \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_unlink\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -2932,6 +2947,11 @@ static int cmd_posix_rmdir(void) d_printf("posix_rmdir \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_rmdir\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -3130,6 +3150,12 @@ static int cmd_lock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"lock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_lock(cli, fnum, start, len, true, lock_type); @@ -3166,6 +3192,12 @@ static int cmd_unlock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"unlock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_unlock(cli, fnum, start, len); @@ -3189,6 +3221,12 @@ static int cmd_posix_whoami(void) bool guest = false; uint32_t i; + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_whoami\" command can be used.\n"); + return 1; + } + status = cli_posix_whoami(cli, ctx, &uid, @@ -3322,6 +3360,12 @@ static int cmd_link(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"link\" command can be used.\n"); + return 1; + } + status = cli_posix_hardlink(targetcli, targetname, newname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s linking files (%s -> %s)\n", @@ -3373,6 +3417,12 @@ static int cmd_readlink(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"readlink\" command can be used.\n"); + return 1; + } + status = cli_posix_readlink(targetcli, name, talloc_tos(), &linkname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s readlink on file %s\n", @@ -3411,6 +3461,11 @@ static int cmd_symlink(void) link_target = buf; if (SERVER_HAS_UNIX_CIFS(cli)) { + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"symlink\" command can be used.\n"); + return 1; + } newname = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), buf2); if (!newname) { @@ -3491,6 +3546,12 @@ static int cmd_chmod(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chmod\" command can be used.\n"); + return 1; + } + status = cli_posix_chmod(targetcli, targetname, mode); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chmod file %s 0%o\n", @@ -3653,6 +3714,12 @@ static int cmd_getfacl(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"getfacl\" command can be used.\n"); + return 1; + } + status = cli_unix_extensions_version(targetcli, &major, &minor, &caplow, &caphigh); if (!NT_STATUS_IS_OK(status)) { @@ -3946,6 +4013,12 @@ static int cmd_stat(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"stat\" command can be used.\n"); + return 1; + } + status = cli_posix_stat(targetcli, targetname, &sbuf); if (!NT_STATUS_IS_OK(status)) { d_printf("%s stat file %s\n", @@ -4058,6 +4131,12 @@ static int cmd_chown(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chown\" command can be used.\n"); + return 1; + } + status = cli_posix_chown(targetcli, targetname, uid, gid); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chown file %s uid=%d, gid=%d\n", -- 2.30.2 From c97828c1565b04561768f05c19c01a2904a8f4f4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:44:05 -0800 Subject: [PATCH 14/25] CVE-2021-44141: s4: torture: In raw.search:test_one_file() add a second connection. Change from torture_suite_add_1smb_test() to torture_suite_add_2smb_test(). Not yet used. We will need this to do SMB1+POSIX search calls on a connection on which we have negotiated SMB1+POSIX. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index b18c25968ae..7e641e17be2 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -299,8 +299,9 @@ static union smb_search_data *find(const char *name) /* basic testing of all RAW_SEARCH_* calls using a single file */ -static bool test_one_file(struct torture_context *tctx, - struct smbcli_state *cli) +static bool test_one_file(struct torture_context *tctx, + struct smbcli_state *cli, + struct smbcli_state *cli_unix) { bool ret = true; int fnum; @@ -1570,7 +1571,7 @@ struct torture_suite *torture_raw_search(TALLOC_CTX *mem_ctx) { struct torture_suite *suite = torture_suite_create(mem_ctx, "search"); - torture_suite_add_1smb_test(suite, "one file search", test_one_file); + torture_suite_add_2smb_test(suite, "one file search", test_one_file); torture_suite_add_1smb_test(suite, "many files", test_many_files); torture_suite_add_1smb_test(suite, "sorted", test_sorted); torture_suite_add_1smb_test(suite, "modify search", test_modify_search); -- 2.30.2 From 954310b754adff4a9c8d8e614b9de6af8b57bc72 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:48:20 -0800 Subject: [PATCH 15/25] CVE-2021-44141: s4: torture: raw.search: Add setup_smb1_posix(). Call it on the second connection in test_one_file(). Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 7e641e17be2..3534af25b44 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -296,6 +296,55 @@ static union smb_search_data *find(const char *name) return NULL; } +/* + * Negotiate SMB1+POSIX. + */ + +static NTSTATUS setup_smb1_posix(struct torture_context *tctx, + struct smbcli_state *cli_unix) +{ + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; + uint32_t cap = cli_unix->transport->negotiate.capabilities; + + if ((cap & CAP_UNIX) == 0) { + /* + * Server doesn't support SMB1+POSIX. + * The caller will skip the UNIX info + * level anyway. + */ + torture_comment(tctx, + "Server doesn't support SMB1+POSIX\n"); + return NT_STATUS_OK; + } + + /* Setup POSIX on this connection. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + return smb_raw_trans2(cli_unix->tree, tctx, &tp); +} + /* basic testing of all RAW_SEARCH_* calls using a single file */ @@ -314,6 +363,16 @@ static bool test_one_file(struct torture_context *tctx, internal_info_supported; union smb_search_data *s; + status = setup_smb1_posix(tctx, cli_unix); + if (!NT_STATUS_IS_OK(status)) { + torture_result(tctx, + TORTURE_FAIL, + __location__"setup_smb1_posix() failed (%s)\n", + nt_errstr(status)); + ret = false; + goto done; + } + fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { torture_result(tctx, -- 2.30.2 From b58150999f7f83a6df1743559ae04025f7ff7ece Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:51:39 -0800 Subject: [PATCH 16/25] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() by using the SMB1+POSIX connection for POSIX info levels. Remove the following entry in knownfail.d/posix_infolevel_fails. ^samba3.raw.search.one\ file\ search.* from knownfail.d/posix_infolevel_fails BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source4/torture/raw/search.c | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index ee4d6318201..394aa711ee5 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -5,4 +5,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* ^samba3.unix.info2.info2\(nt4_dc_smb1\) -^samba3.raw.search.one\ file\ search.* diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 3534af25b44..06cc7dfbdbd 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -388,10 +388,19 @@ static bool test_one_file(struct torture_context *tctx, for (i=0;itransport->negotiate.capabilities; + struct smbcli_state *cli_search = cli; torture_comment(tctx, "Testing %s\n", levels[i].name); - levels[i].status = torture_single_search(cli, tctx, fname, + if (levels[i].data_level == RAW_SEARCH_DATA_UNIX_INFO) { + /* + * For an SMB1+POSIX info level, use the cli_unix + * connection. + */ + cli_search = cli_unix; + } + + levels[i].status = torture_single_search(cli_search, tctx, fname, levels[i].level, levels[i].data_level, 0, @@ -415,7 +424,7 @@ static bool test_one_file(struct torture_context *tctx, continue; } - status = torture_single_search(cli, tctx, fname2, + status = torture_single_search(cli_search, tctx, fname2, levels[i].level, levels[i].data_level, 0, -- 2.30.2 From 4b6d5201de8e26f3cd140999abfaa2ba899b07fa Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:15:06 -0800 Subject: [PATCH 17/25] CVE-2021-44141: s4: torture: Fix unix.info2 test to actually negotiate SMB1+POSIX before using POSIX calls. Cope with the minor difference in wildcard search return when we're actually using SMB1+POSIX on the server (SMB1+POSIX treats all directory search paths as wildcards). Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source4/torture/unix/unix_info2.c | 42 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 394aa711ee5..d6d239b4014 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -4,4 +4,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* -^samba3.unix.info2.info2\(nt4_dc_smb1\) diff --git a/source4/torture/unix/unix_info2.c b/source4/torture/unix/unix_info2.c index 069a5818e13..dafe9562ad7 100644 --- a/source4/torture/unix/unix_info2.c +++ b/source4/torture/unix/unix_info2.c @@ -19,6 +19,7 @@ #include "includes.h" #include "libcli/libcli.h" +#include "libcli/raw/raw_proto.h" #include "torture/util.h" #include "torture/unix/proto.h" #include "lib/cmdline/popt_common.h" @@ -53,6 +54,10 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) const char *share = torture_setting_string(tctx, "share", NULL); struct smbcli_options options; struct smbcli_session_options session_options; + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; lpcfg_smbcli_options(tctx->lp_ctx, &options); lpcfg_smbcli_session_options(tctx->lp_ctx, &session_options); @@ -72,6 +77,33 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) return NULL; } + /* Setup POSIX on the server. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + + status = smb_raw_trans2(cli->tree, tctx, &tp); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_OK, + "doing SMB_SET_CIFS_UNIX_INFO"); + return cli; } @@ -245,8 +277,14 @@ static bool find_single_info2(void *mem_ctx, torture_assert_int_equal(torture, search.t2ffirst.out.count, 1, "expected exactly one result"); - torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 1, - "expected end_of_search to be true"); + /* + * In smbd directory listings using POSIX extensions + * always treat the search pathname as a wildcard, + * so don't expect end_of_search to be set here. Wildcard + * searches always need a findnext to end the search. + */ + torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 0, + "expected end_of_search to be false"); return check_unix_info2(torture, info2); } -- 2.30.2 From be6fcad37ccf506bb3121c0f9d59638f739dabdc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:12:36 -0800 Subject: [PATCH 18/25] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.inherit_owner test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entry in knownfail.d/posix_infolevel_fails. samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/script/tests/test_inherit_owner.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index d6d239b4014..b940bff19de 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -3,4 +3,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* -^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh index 7e1333787aa..9783235883c 100755 --- a/source3/script/tests/test_inherit_owner.sh +++ b/source3/script/tests/test_inherit_owner.sh @@ -79,7 +79,7 @@ unix_owner_id_is() { local fname=$2 local expected_id=$3 local actual_id - actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') + actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') if ! test "x$actual_id" = "x$expected_id" ; then echo "Actual uid of $share/$fname is [$actual_id] expected [$expected_id]" exit 1 -- 2.30.2 From 2704a27c7cb78608765728da45f7eeebec04bafc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 00:05:35 -0800 Subject: [PATCH 19/25] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.acl_xattr test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 3 --- source3/script/tests/test_acl_xattr.sh | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index b940bff19de..dc3c4e185a3 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -1,5 +1,2 @@ ^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) -^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh index f134ff79c91..8abd7476244 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -55,9 +55,9 @@ nt_affects_posix() { local b4 local af local fname="$share.$$" - b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/READ" 2>/dev/null || exit 1 - af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "before: $b4" echo "after: $af" echo "${b4}" | grep -q "^# owner:" || exit 1 @@ -90,12 +90,12 @@ nt_affects_chown() { #basic sanity... test "$b4_expected != $af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# owner:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# owner: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/FULL" || exit 1 $SMBCACLS //$SERVER/$share $fname -U force_user%$PASSWORD -C force_user 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# owner:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p') echo "before: $b4_actual" @@ -124,11 +124,11 @@ nt_affects_chgrp() { #basic sanity... test "$b4_expected" != "$af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# group:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -G domadmins 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# group:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# group: (.*)/\1/p') echo "before: $b4_actual" -- 2.30.2 From 51eddff581fbb65d3bf4d78fb06533bbfe9c1c06 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 12:16:44 -0800 Subject: [PATCH 20/25] CVE-2021-44141: s3: smbtorture3: Fix POSIX-BLOCKING-LOCK to actually negotiate SMB1+POSIX before using POSIX calls. This must be done before doing POSIX calls on a connection. Remove the final entry in knownfail.d/posix_infolevel_fails samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) And remove the file knownfail.d/posix_infolevel_fails itself. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 2 -- source3/torture/torture.c | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails deleted file mode 100644 index dc3c4e185a3..00000000000 --- a/selftest/knownfail.d/posix_infolevel_fails +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.crypt.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 18844d49f8d..e5ce9dd2042 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8896,6 +8896,11 @@ static bool run_posix_blocking_lock(int dummy) return false; } + status = torture_setup_unix_extensions(cli2); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + cli_setatr(cli1, fname, 0, 0); cli_posix_unlink(cli1, fname); -- 2.30.2 From ad99af76917237db329405103f253bb623133b0e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 16:57:18 -0800 Subject: [PATCH 21/25] CVE-2021-44141: s3: smbtorture3: Add test POSIX-SYMLINK-ERRORCODE to ensure we always get NT_STATUS_OBJECT_NAME_NOT_FOUND for dangling or unavailable symlinks. Creates 3 symlinks inside the share: 1). Within share pointing to non-existing target. "nonexist_errorcode_symlink" -> "nonexist_errorcode_target" 2). Outside share but to existing target. "tmp_symlink" -> "/tmp" 3). Outside share but to non-existing target. "tmp_noexist_symlink" -> "/tmp/XXX" It then tries to SMB1+POSIX stat these objects, all should succeed. It then tries to open via 3 methods: 1). SMB1+POSIX cli_posix_open() 2). SMB1 Windows cli_ntcreate() 3). SMB2 Windows cli_ntcreate() Ensure we always get NT_STATUS_OBJECT_NAME_NOT_FOUND for all return codes. Add knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_symlink_errorcode | 2 + source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_posix.c | 300 +++++++++++++++++++ source3/torture/torture.c | 4 + 5 files changed, 308 insertions(+) create mode 100644 selftest/knownfail.d/posix_symlink_errorcode diff --git a/selftest/knownfail.d/posix_symlink_errorcode b/selftest/knownfail.d/posix_symlink_errorcode new file mode 100644 index 00000000000..e2e5d28be5c --- /dev/null +++ b/selftest/knownfail.d/posix_symlink_errorcode @@ -0,0 +1,2 @@ +^samba3.smbtorture_s3.plain.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index cab49783438..31e0604c375 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -219,6 +219,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-LS-SINGLE", "POSIX-READLINK", "POSIX-STAT", + "POSIX-SYMLINK-ERRORCODE", ] for t in posix_tests: diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 521c662e2fb..3f80566e99f 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -90,6 +90,7 @@ bool run_posix_ls_single_test(int dummy); bool run_posix_readlink_test(int dummy); bool run_posix_stat_test(int dummy); bool run_case_insensitive_create(int dummy); +bool run_posix_symlink_errorcode_test(int dummy); bool run_nbench2(int dummy); bool run_async_echo(int dummy); diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c index 4a7f12e2812..f19a4361fe7 100644 --- a/source3/torture/test_posix.c +++ b/source3/torture/test_posix.c @@ -721,3 +721,303 @@ out: TALLOC_FREE(frame); return correct; } + +/* + Ensure we get the same error code of + NT_STATUS_OBJECT_NAME_NOT_FOUND for dangling + symlinks, no matter where they point or how we + try and access them. + */ + +/* Needed for SMB2 connection. */ +extern fstring host, workgroup, share, password, username, myname; +extern struct cli_credentials *torture_creds; + +bool run_posix_symlink_errorcode_test(int dummy) +{ + TALLOC_CTX *frame = NULL; + struct cli_state *cli_unix = NULL; + struct cli_state *cli_smb1_noposix = NULL; + struct cli_state *cli_smb2 = NULL; + NTSTATUS status; + struct stat statbuf; + bool correct = false; + size_t i; + int ret; + SMB_STRUCT_STAT sbuf; + uint16_t fnum_smb1 = (uint16_t)-1; + uint16_t fnum_smb2 = (uint16_t)-1; + const char *namearray_symlinks[3] = { + "nonexist_errorcode_symlink", + "tmp_symlink", + "tmp_noexist_symlink"}; + /* Can't use const herer as the third entry is dynamic. */ + char *namearray_target[3] = { + discard_const_p(char, "nonexist_errorcode_target"), + discard_const_p(char, "/tmp"), + NULL}; + + frame = talloc_stackframe(); + + printf("Starting POSIX-SYMLINK-ERRORCODE test\n"); + + /* Ensure /tmp exists. */ + ret = lstat("/tmp", &statbuf); + if (ret != 0) { + printf("lstat of /tmp failed error %s\n", + strerror(errno)); + TALLOC_FREE(frame); + return false; + } + /* + * Try 10 times to get a target name out of share we know doesn't exist. + */ + for (i = 0; i < 10; i++) { + namearray_target[2] = talloc_asprintf(talloc_tos(), + "/tmp/XXX%x%x", + (unsigned)i, + (unsigned)random()); + if (namearray_target[2] == NULL) { + TALLOC_FREE(frame); + return false; + } + ret = lstat(namearray_target[2], &statbuf); + if (ret == 0) { + TALLOC_FREE(namearray_target[2]); + continue; + } + if (ret != -1 && errno != ENOENT) { + TALLOC_FREE(namearray_target[2]); + continue; + } + break; + } + if (i == 10) { + printf("Can't find a temporary nonexistent name in /tmp\n"); + TALLOC_FREE(frame); + return false; + } + + /* Open an SMB1+POSIX connection. */ + if (!torture_open_connection(&cli_unix, 0)) { + printf("torture_open_connection SMB1+POSIX failed !\n"); + TALLOC_FREE(frame); + return false; + } + torture_conn_set_sockopt(cli_unix); + status = torture_setup_unix_extensions(cli_unix); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_setup_unix_extensions failed !\n"); + TALLOC_FREE(frame); + return false; + } + + /* + * Start with a clean slate. + * Ensure + * "nonexist_errorcode_target" + * "nonexist_errorcode_symlink", + * "tmp_symlink", + * "tmp_noexist_symlink" + * don't exist. + */ + cli_posix_unlink(cli_unix, namearray_target[0]); + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + cli_posix_unlink(cli_unix, namearray_symlinks[i]); + } + + /* + * Create all 3 symlinks. + * 1). "nonexist_errorcode_symlink" -> "nonexist_errorcode_target". + * 2). "tmp_symlink" -> "/tmp" + * 3). "tmp_noexist_symlink" -> "/tmp/XXX". + */ + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_posix_symlink(cli_unix, + namearray_target[i], + namearray_symlinks[i]); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_symlink of %s -> %s failed " + "error %s\n", + namearray_symlinks[i], + namearray_target[i], + nt_errstr(status)); + goto out; + } + } + + /* We should be able to stat all 3. */ + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_posix_stat(cli_unix, namearray_symlinks[i], &sbuf); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_stat of %s failed error %s\n", + namearray_symlinks[i], + nt_errstr(status)); + goto out; + } + } + + /* + * But Opening all 3 via SMB1+POSIX should get + * NT_STATUS_OBJECT_NAME_NOT_FOUND. + */ + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_posix_open(cli_unix, + namearray_symlinks[i], + O_RDONLY, + 0600, + &fnum_smb1); + if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + printf("cli_posix_open of %s got error %s " + "should be NT_STATUS_OBJECT_NAME_NOT_FOUND.\n", + namearray_symlinks[i], + nt_errstr(status)); + goto out; + } + } + + /* + * Opening all 3 via SMB1 without POSIX should also get + * NT_STATUS_OBJECT_NAME_NOT_FOUND. + */ + /* Open an SMB1 connection - without POSIX. */ + if (!torture_open_connection(&cli_smb1_noposix, 0)) { + printf("torture_open_connection SMB1 without POSIX failed !\n"); + TALLOC_FREE(frame); + return false; + } + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_ntcreate(cli_smb1_noposix, /* cli. */ + namearray_symlinks[i], /* fname */ + 0, /* CreatFlags */ + READ_CONTROL_ACCESS, /* DesiredAccess */ + 0, /* FileAttributes */ + FILE_SHARE_READ| + FILE_SHARE_WRITE| + FILE_SHARE_DELETE, /* ShareAccess */ + FILE_OPEN, /* CreateDisposition */ + 0, /* CreateOptions */ + 0x0, /* SecurityFlags */ + &fnum_smb2, /* fnum. */ + NULL); /* cr */ + if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + printf("cli_ntcreate of %s got error %s " + "should be NT_STATUS_OBJECT_NAME_NOT_FOUND.\n", + namearray_symlinks[i], + nt_errstr(status)); + goto out; + } + } + /* + * Trying to do a cli_posix_stat() via SMB1 without POSIX should + * get NT_STATUS_INVALID_LEVEL. + */ + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_posix_stat(cli_smb1_noposix, + namearray_symlinks[i], + &sbuf); + if (!NT_STATUS_EQUAL(status, NT_STATUS_INVALID_LEVEL)) { + printf("cli_posix_stat of %s got error %s " + "should be NT_STATUS_INVALID_LEVEL.\n", + namearray_symlinks[i], + nt_errstr(status)); + goto out; + } + } + + /* Open an SMB2 connection. */ + if (!torture_init_connection(&cli_smb2)) { + printf("torture_init_connection for SMB2 failed\n"); + goto out; + } + + status = smbXcli_negprot(cli_smb2->conn, + cli_smb2->timeout, + PROTOCOL_SMB2_02, + PROTOCOL_SMB2_02); + if (!NT_STATUS_IS_OK(status)) { + printf("SMB2 smbXcli_negprot returned %s\n", + nt_errstr(status)); + goto out; + } + + status = cli_session_setup_creds(cli_smb2, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("SMB2 cli_session_setup returned %s\n", + nt_errstr(status)); + goto out; + } + + status = cli_tree_connect(cli_smb2, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("SMB2 cli_tree_connect returned %s\n", + nt_errstr(status)); + goto out; + } + + /* + * Opening all 3 via SMB2 should get + * NT_STATUS_OBJECT_NAME_NOT_FOUND. + */ + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + status = cli_ntcreate(cli_smb2, /* cli. */ + namearray_symlinks[i], /* fname */ + 0, /* CreatFlags */ + READ_CONTROL_ACCESS, /* DesiredAccess */ + 0, /* FileAttributes */ + FILE_SHARE_READ| + FILE_SHARE_WRITE| + FILE_SHARE_DELETE, /* ShareAccess */ + FILE_OPEN, /* CreateDisposition */ + 0, /* CreateOptions */ + 0x0, /* SecurityFlags */ + &fnum_smb2, /* fnum. */ + NULL); /* cr */ + if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + printf("cli_ntcreate of %s got error %s " + "should be NT_STATUS_OBJECT_NAME_NOT_FOUND.\n", + namearray_symlinks[i], + nt_errstr(status)); + goto out; + } + } + + printf("POSIX-SYMLINK-ERRORCODE test passed\n"); + correct = true; + +out: + if (fnum_smb1 != (uint16_t)-1) { + cli_close(cli_unix, fnum_smb1); + } + if (fnum_smb2 != (uint16_t)-1) { + cli_close(cli_smb2, fnum_smb2); + } + /* Ensure + * "nonexist_errorcode_target" + * "nonexist_errorcode_symlink", + * "tmp_symlink", + * "tmp_noexist_symlink" + * don't exist. + */ + cli_posix_unlink(cli_unix, namearray_target[0]); + for (i = 0; i < ARRAY_SIZE(namearray_symlinks); i++) { + cli_posix_unlink(cli_unix, namearray_symlinks[i]); + } + if (cli_unix != NULL) { + if (!torture_close_connection(cli_unix)) { + correct = false; + } + } + if (cli_smb1_noposix != NULL) { + if (!torture_close_connection(cli_smb1_noposix)) { + correct = false; + } + } + if (cli_smb2 != NULL) { + if (!torture_close_connection(cli_smb2)) { + correct = false; + } + } + TALLOC_FREE(frame); + return correct; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index e5ce9dd2042..65d41c071e6 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14651,6 +14651,10 @@ static struct { .name = "UID-REGRESSION-TEST", .fn = run_uid_regression_test, }, + { + .name = "POSIX-SYMLINK-ERRORCODE", + .fn = run_posix_symlink_errorcode_test, + }, { .name = "SHORTNAME-TEST", .fn = run_shortname_test, -- 2.30.2 From 3980ac8b1c567fa5297aab08b4bacf47a407b249 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 00:15:03 -0800 Subject: [PATCH 22/25] CVE-2021-44141: s3: torture: Make simple POSIX test expect NT_STATUS_OBJECT_NAME_NOT_FOUND. Add to knownfail selftest/knownfail.d/posix_symlink_errorcode for now until we fix smbd. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_symlink_errorcode | 2 ++ source3/torture/torture.c | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_symlink_errorcode b/selftest/knownfail.d/posix_symlink_errorcode index e2e5d28be5c..013f85104c8 100644 --- a/selftest/knownfail.d/posix_symlink_errorcode +++ b/selftest/knownfail.d/posix_symlink_errorcode @@ -1,2 +1,4 @@ ^samba3.smbtorture_s3.plain.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.plain.POSIX.smbtorture\(nt4_dc_smb1\) +^samba3.smbtorture_s3.crypt.POSIX.smbtorture\(nt4_dc_smb1\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 65d41c071e6..57e625288a5 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -7921,10 +7921,10 @@ static bool run_simple_posix_open_test(int dummy) printf("POSIX open of %s succeeded (should have failed)\n", sname); goto out; } else { - if (!check_both_error(__LINE__, status, ERRDOS, ERRbadpath, - NT_STATUS_OBJECT_PATH_NOT_FOUND)) { + if (!check_both_error(__LINE__, status, ERRDOS, ERRbadfile, + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { printf("POSIX open of %s should have failed " - "with NT_STATUS_OBJECT_PATH_NOT_FOUND, " + "with NT_STATUS_OBJECT_NAME_NOT_FOUND, " "failed with %s instead.\n", sname, nt_errstr(status)); goto out; -- 2.30.2 From ca3a894b01fe2c5ee0c5dfca3f0564c69554ef67 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 10:22:10 -0800 Subject: [PATCH 23/25] CVE-2021-44141: s3: tests: Get test_smbclient_s3.sh ready for change in error return for symlinks (NT_STATUS_ACCESS_DENIED -> NT_STATUS_OBJECT_NAME_NOT_FOUND). Add to selftest/knownfail.d/posix_symlink_errorcode. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_symlink_errorcode | 2 ++ source3/script/tests/test_smbclient_s3.sh | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/selftest/knownfail.d/posix_symlink_errorcode b/selftest/knownfail.d/posix_symlink_errorcode index 013f85104c8..00448349c30 100644 --- a/selftest/knownfail.d/posix_symlink_errorcode +++ b/selftest/knownfail.d/posix_symlink_errorcode @@ -2,3 +2,5 @@ ^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.plain.POSIX.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX.smbtorture\(nt4_dc_smb1\) +^samba3.blackbox.smbclient_s3.*Ensure\ widelinks\ are\ restricted.* +^samba3.blackbox.smbclient_s3.*follow\ symlinks\ =\ no.* diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 62662690415..165d1d0ff81 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1010,12 +1010,12 @@ EOF return 1 fi -# This should fail with NT_STATUS_ACCESS_DENIED - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' +# This should fail with NT_STATUS_OBJECT_NAME_NOT_FOUND + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret != 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED listing \\widelinks_share\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND listing \\widelinks_share\\source" return 1 fi } @@ -1133,11 +1133,11 @@ EOF return 1 fi - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret -ne 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND getting \\nosymlinks\\source" return 1 fi -- 2.30.2 From 909cc2e35dce7e0513be5d042faecb38c84e9d7f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 10:40:58 -0800 Subject: [PATCH 24/25] CVE-2021-44141: s3: smbd: In check_reduced_name() and check_reduced_name_with_privilege(), return NT_STATUS_OBJECT_NAME_NOT_FOUND instead of NT_STATUS_ACCESS_DENIED for symlinks we cannot reach. They are either out of share or point to nowhere. Always match the symlink error returned by openat_pathref_fsp(). This unifies the return for symlinks over SMB1, SMB1+POSIX and SMB2 when parsing a path and lays the foundation for SMB2+POSIX. Remove knownfails in selftest/knownfail.d/posix_symlink_errorcode for test_smbclient_s3.sh. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_symlink_errorcode | 2 -- source3/smbd/vfs.c | 12 ++++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/selftest/knownfail.d/posix_symlink_errorcode b/selftest/knownfail.d/posix_symlink_errorcode index 00448349c30..013f85104c8 100644 --- a/selftest/knownfail.d/posix_symlink_errorcode +++ b/selftest/knownfail.d/posix_symlink_errorcode @@ -2,5 +2,3 @@ ^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.plain.POSIX.smbtorture\(nt4_dc_smb1\) ^samba3.smbtorture_s3.crypt.POSIX.smbtorture\(nt4_dc_smb1\) -^samba3.blackbox.smbclient_s3.*Ensure\ widelinks\ are\ restricted.* -^samba3.blackbox.smbclient_s3.*follow\ symlinks\ =\ no.* diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 5b9ae17768c..61b90f72786 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1218,7 +1218,7 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, DBG_ERR("device/inode/uid/gid on directory %s changed. " "Denying access !\n", smb_fname_str_dbg(parent_name)); - status = NT_STATUS_ACCESS_DENIED; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; goto err; } @@ -1255,7 +1255,7 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, smb_fname_str_dbg(parent_name)); DEBUGADD(1, ("conn_rootdir =%s\n", conn_rootdir)); DEBUGADD(1, ("resolved_name=%s\n", resolved_name)); - status = NT_STATUS_ACCESS_DENIED; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; goto err; } } @@ -1281,7 +1281,7 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, DBG_WARNING("Last component %s is a symlink. Denying" "access.\n", smb_fname_str_dbg(file_name)); - status = NT_STATUS_ACCESS_DENIED; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; goto err; } @@ -1431,7 +1431,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, conn_rootdir, resolved_name); TALLOC_FREE(resolved_fname); - return NT_STATUS_ACCESS_DENIED; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } } @@ -1455,7 +1455,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, *p, fname); TALLOC_FREE(resolved_fname); - return NT_STATUS_ACCESS_DENIED; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } p++; @@ -1486,7 +1486,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, p); TALLOC_FREE(resolved_fname); TALLOC_FREE(new_fname); - return NT_STATUS_ACCESS_DENIED; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } } -- 2.30.2 From f95874937b82979e4fa4dcbdd3255373853d1428 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 10:55:15 -0800 Subject: [PATCH 25/25] CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND. Matches the error return from openat_pathref_fsp(). NT_STATUS_OBJECT_PATH_NOT_FOUND is for a bad component in a path, not a bad terminal symlink. Now all symlink errors are returned as NT_STATUS_OBJECT_NAME_NOT_FOUND for both SMB1, SMB1+POSIX and SMB2+. Remove knownfail file selftest/knownfail.d/posix_symlink_errorcode. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_symlink_errorcode | 4 --- source3/smbd/open.c | 27 +++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) delete mode 100644 selftest/knownfail.d/posix_symlink_errorcode diff --git a/selftest/knownfail.d/posix_symlink_errorcode b/selftest/knownfail.d/posix_symlink_errorcode deleted file mode 100644 index 013f85104c8..00000000000 --- a/selftest/knownfail.d/posix_symlink_errorcode +++ /dev/null @@ -1,4 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-ERRORCODE.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.plain.POSIX.smbtorture\(nt4_dc_smb1\) -^samba3.smbtorture_s3.crypt.POSIX.smbtorture\(nt4_dc_smb1\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 0c2c381bde5..32788097978 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -502,7 +502,8 @@ static int process_symlink_open(struct connection_struct *conn, */ link_depth++; if (link_depth >= 20) { - errno = ELOOP; + /* We want this to map to NT_STATUS_OBJECT_NAME_NOT_FOUND */ + errno = ENOENT; goto out; } @@ -553,7 +554,8 @@ static int process_symlink_open(struct connection_struct *conn, resolved_name, rootdir_len) == 0); if (!matched) { - errno = EACCES; + /* We want this to map to NT_STATUS_OBJECT_NAME_NOT_FOUND */ + errno = ENOENT; goto out; } @@ -569,7 +571,8 @@ static int process_symlink_open(struct connection_struct *conn, smb_fname->base_name = talloc_strdup(smb_fname, &resolved_name[rootdir_len+1]); } else { - errno = EACCES; + /* We want this to map to NT_STATUS_OBJECT_NAME_NOT_FOUND */ + errno = ENOENT; goto out; } @@ -721,10 +724,28 @@ static int non_widelink_open(files_struct *fsp, if (saved_errno == ELOOP || saved_errno == ENOTDIR) { if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) { /* Never follow symlinks on posix open. */ + /* + * map_nt_error_from_unix() maps ELOOP to + * NT_STATUS_OBJECT_PATH_NOT_FOUND. For + * error return consistency we need this + * to map to NT_STATUS_OBJECT_NAME_NOT_FOUND. + * + * https://bugzilla.samba.org/show_bug.cgi?id=14911 + */ + saved_errno = ENOENT; goto out; } if (!lp_follow_symlinks(SNUM(conn))) { /* Explicitly no symlinks. */ + /* + * map_nt_error_from_unix() maps ELOOP to + * NT_STATUS_OBJECT_PATH_NOT_FOUND. For + * error return consistency we need this + * to map to NT_STATUS_OBJECT_NAME_NOT_FOUND. + * + * https://bugzilla.samba.org/show_bug.cgi?id=14911 + */ + saved_errno = ENOENT; goto out; } /* -- 2.30.2