From bc9798e2e546e1b379e73079ad572a74f3a7efd7 Mon Sep 17 00:00:00 2001 From: Paul Wise Date: Fri, 29 Jan 2021 12:08:25 +0800 Subject: [PATCH] Backport net rpc share allowedusers fix --- source3/libsmb/clidfs.c | 7 +++++++ source3/libsmb/clientgen.c | 30 ++++++++++++++++++++++++++++-- source3/torture/test_smb2.c | 2 +- source3/torture/torture.c | 27 ++++++++++++++++++++++----- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c index 4342a3b1d1b..cb15d20856a 100644 --- a/source3/libsmb/clidfs.c +++ b/source3/libsmb/clidfs.c @@ -1257,6 +1257,13 @@ bool cli_check_msdfs_proxy(TALLOC_CTX *ctx, if (force_encrypt) { status = cli_cm_force_encryption_creds(cli, creds, "IPC$"); if (!NT_STATUS_IS_OK(status)) { + /* + * Failed to set up encryption. + * Disconnect the temporary IPC$ + * tcon before restoring the original + * tcon so we don't leak it. + */ + cli_tdis(cli); cli_state_restore_tcon(cli, orig_tcon); return false; } diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c index 2e4dd15ab62..eedf6b9eb6b 100644 --- a/source3/libsmb/clientgen.c +++ b/source3/libsmb/clientgen.c @@ -381,11 +381,37 @@ uint32_t cli_state_set_tid(struct cli_state *cli, uint32_t tid) struct smbXcli_tcon *cli_state_save_tcon(struct cli_state *cli) { + /* + * Note. This used to make a deep copy of either + * cli->smb2.tcon or cli->smb1.tcon, but this leaves + * the original pointer in place which will then get + * TALLOC_FREE()'d when the new connection is made on + * this cli_state. + * + * As there may be pipes open on the old connection with + * talloc'ed state allocated using the tcon pointer as a + * parent we can't deep copy and then free this as that + * closes the open pipes. + * + * This call is used to temporarily swap out a tcon pointer + * to allow a new tcon on the same cli_state. + * + * Just return the raw pointer and set the old value to NULL. + * We know we MUST be calling cli_state_restore_tcon() below + * to restore before closing the session. + * + * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=13992 + */ + struct smbXcli_tcon *tcon_ret = NULL; + if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) { - return smbXcli_tcon_copy(cli, cli->smb2.tcon); + tcon_ret = cli->smb2.tcon; + cli->smb2.tcon = NULL; /* *Not* TALLOC_FREE(). */ } else { - return smbXcli_tcon_copy(cli, cli->smb1.tcon); + tcon_ret = cli->smb1.tcon; + cli->smb1.tcon = NULL; /* *Not* TALLOC_FREE(). */ } + return tcon_ret; } void cli_state_restore_tcon(struct cli_state *cli, struct smbXcli_tcon *tcon) diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c index 094a9b84d6e..ce8e046cab4 100644 --- a/source3/torture/test_smb2.c +++ b/source3/torture/test_smb2.c @@ -187,11 +187,11 @@ bool run_smb2_basic(int dummy) cli->timeout, cli->smb2.session, cli->smb2.tcon); + cli_state_restore_tcon(cli, saved_tcon); if (!NT_STATUS_IS_OK(status)) { printf("smb2cli_tdis returned %s\n", nt_errstr(status)); return false; } - cli_state_restore_tcon(cli, saved_tcon); status = smb2cli_tdis(cli->conn, cli->timeout, diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 4e4f3760ddf..0cfb540a326 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -1353,6 +1353,7 @@ static bool run_tcon_test(int dummy) if (!NT_STATUS_IS_OK(status)) { printf("%s refused 2nd tree connect (%s)\n", host, nt_errstr(status)); + cli_state_restore_tcon(cli, orig_tcon); cli_shutdown(cli); return False; } @@ -1405,6 +1406,8 @@ static bool run_tcon_test(int dummy) status = cli_close(cli, fnum1); if (!NT_STATUS_IS_OK(status)) { printf("close failed (%s)\n", nt_errstr(status)); + cli_state_restore_tcon(cli, orig_tcon); + cli_shutdown(cli); return False; } @@ -1413,6 +1416,8 @@ static bool run_tcon_test(int dummy) status = cli_tdis(cli); if (!NT_STATUS_IS_OK(status)) { printf("secondary tdis failed (%s)\n", nt_errstr(status)); + cli_state_restore_tcon(cli, orig_tcon); + cli_shutdown(cli); return False; } @@ -9447,7 +9452,7 @@ static bool run_uid_regression_test(int dummy) int16_t old_vuid; int32_t old_cnum; bool correct = True; - struct smbXcli_tcon *orig_tcon = NULL; + struct smbXcli_tcon *tcon_copy = NULL; NTSTATUS status; printf("starting uid regression test\n"); @@ -9488,8 +9493,20 @@ static bool run_uid_regression_test(int dummy) } old_cnum = cli_state_get_tid(cli); - orig_tcon = cli_state_save_tcon(cli); - if (orig_tcon == NULL) { + /* + * This is an SMB1-only test. + * Copy the tcon, not "save/restore". + * + * In SMB1 the cli_tdis() below frees + * cli->smb1.tcon so we need a copy + * of the struct to put back for the + * second tdis call with invalid vuid. + * + * This is a test-only hack. Real client code + * uses cli_state_save_tcon()/cli_state_restore_tcon(). + */ + tcon_copy = smbXcli_tcon_copy(cli, cli->smb1.tcon); + if (tcon_copy == NULL) { correct = false; goto out; } @@ -9505,11 +9522,11 @@ static bool run_uid_regression_test(int dummy) } else { d_printf("First tdis failed (%s)\n", nt_errstr(status)); correct = false; - cli_state_restore_tcon(cli, orig_tcon); + cli->smb1.tcon = tcon_copy; goto out; } - cli_state_restore_tcon(cli, orig_tcon); + cli->smb1.tcon = tcon_copy; cli_state_set_uid(cli, old_vuid); cli_state_set_tid(cli, old_cnum); -- 2.29.2