session setup bugfix and torture test
Stefan (metze) Metzmacher
metze at samba.org
Fri Sep 20 11:41:38 CEST 2013
Hi Michael,
I think this changes are a good start, but I try to
fix this more broadly similar to this change
commit e6a58d370403e818bc2cfb8389751b78adcc14fd
Author: Stefan Metzmacher <metze at samba.org>
Date: Tue Jul 9 16:38:59 2013 +0200
s4:rpc_server: make sure we don't terminate a connection with
pending requests (bug #9820)
Sadly we may have nested event loops, which won't work correctly with
broken connections, that's why we have to do this...
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
Autobuild-Date(master): Wed Jul 10 08:47:38 CEST 2013 on sn-devel-104
The goal should be that we move any session, tcon and open to a deleted list
on the connection and just mark them as deleted like this
session->status = NT_STATUS_USER_SESSION_DELETED;
tcon->status = NT_STATUS_NETWORK_NAME_DELETED;
open->status = NT_STATUS_FILE_CLOSED;
And do through all pending request which match the closed object
and do tevent_req_cancel(smb2req->subreq);
Before we process any new incoming request, we would scan the deleted lists
and clean the objects up if they aren't used anymore.
Am 20.09.2013 08:13, schrieb Michael Adam:
> --- a/source3/smbd/smb2_sesssetup.c
> +++ b/source3/smbd/smb2_sesssetup.c
> @@ -445,23 +445,38 @@ struct smbd_smb2_session_setup_state {
>
> static int pp_self_ref_destructor(struct smbd_smb2_session_setup_state **pp_state)
> {
> - (*pp_state)->session = NULL;
> /*
> * To make things clearer, ensure the pp_self_ref
> * pointer is nulled out. We're never going to
> * access this again.
> */
> (*pp_state)->pp_self_ref = NULL;
> +
> + if ((*pp_state)->session == NULL) {
> + return 0;
> + }
> +
> + (*pp_state)->session = NULL;
> +
> return 0;
> }
I think we should remove this hunk, it doesn't change anything.
>
> + substitute_user = generate_random_str_list(tctx, 8,
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789");
> + ok = cli_credentials_set_username(broken_creds, substitute_user,
> + CRED_SPECIFIED);
> + CHECK_VAL(ok, true);
> +
Maybe we should just set a wrong password here?
> + status = smb2_session_setup_spnego(tree->session,
> + broken_creds,
> + 0 /* previous_session_id */);
> + CHECK_STATUS(status, NT_STATUS_LOGON_FAILURE);
> +
> + torture_comment(tctx, "did failed reauth\n");
> + /*
> + * now verify that the invalid session reauth has closed our session
> + */
> +
> + if (encrypted) {
> + expected = NT_STATUS_CONNECTION_DISCONNECTED;
> + } else if (using_kerberos) {
> + expected = NT_STATUS_INVALID_NETWORK_RESPONSE;
Is this correct? didn't we get that from smb2_session_setup_spnego()
I can't see why the create should fail with that status.
> + } else {
> + expected = NT_STATUS_USER_SESSION_DELETED;
> + }
> +
> + smb2_oplock_create_share(&io1, fname,
> + smb2_util_share_access(""),
> + smb2_util_oplock_level("b"));
> +
> + status = smb2_create(tree, mem_ctx, &io1);
> + CHECK_STATUS(status, expected);
metze
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130920/40bf6b9a/attachment.pgp>
More information about the samba-technical
mailing list