reply_sesssetup_and_X_spnego in Samba 3.6.x allows a non-zero (V)UID for a new Session Setup

[Jeremy Allison]

On Wed, Feb 20, 2013 at 10:49:22AM -0800, Richard Sharpe wrote:
> Hi,
> 
> Samba 3.6.x fails the following smbtorture test:
> 
> smbtorture 4.1.0pre1-DEVELOPERBUILD
> Using seed 1361385618
> time: 2013-02-20 10:40:19.087230
> test: session1
> time: 2013-02-20 10:40:19.090190
> Remote OS: Unix, Lan Manager: Samba 3.6.12
> TESTING SESSION HANDLING
> create a second security context on the same transport
> create a third security context on the same transport, with given vuid
> vuid1=100 vuid2=101 vuid3=102
> time: 2013-02-20 10:40:19.689054
> failure: session1 [
> ../source4/torture/raw/context.c:123: status was NT_STATUS_OK,
> expected ERRSRV:ERRbaduid: ../source4/torture/raw/context.c:123
> ]
> 
> This seems to happen because of the following code in reply_sesssetup_and_X:
> 
>         /* Do we have a valid vuid now ? */
>         if (!is_partial_auth_vuid(sconn, vuid)) {
>                 /* No, start a new authentication setup. */
>                 vuid = register_initial_vuid(sconn);
>                 if (vuid == UID_FIELD_INVALID) {
>                         data_blob_free(&blob1);
>                         reply_nterror(req, nt_status_squash(
>                                               NT_STATUS_INVALID_PARAMETER));
>                         return;
>                 }
>         }
> 
> If we don't have a valid VUID at that point and it is not 0, we should
> reject the auth attempt with ERRSRV:ERRbaduid. At least that is what
> W2K08 does.

Try this. Allows us to pass session1, but there are still a couple
of fixes needed for the full raw.context.

Jeremy.
-------------- next part --------------
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 358d051..c59bd39 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1516,7 +1516,7 @@ static connection_struct *switch_message(uint8 type, struct smb_request *req, in
 			DEBUG(0, ("Error: Could not change to user. Removing "
 				"deferred open, mid=%llu.\n",
 				(unsigned long long)req->mid));
-			reply_force_doserror(req, ERRSRV, ERRbaduid);
+			reply_nterror(req, NT_STATUS_INVALID_HANDLE);
 			return conn;
 		}
 
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index a708fd8..4d20c5f 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2081,6 +2081,10 @@ void reply_ulogoffX(struct smb_request *req)
 	if(vuser == NULL) {
 		DEBUG(3,("ulogoff, vuser id %d does not map to user.\n",
 			 req->vuid));
+		END_PROFILE(SMBulogoffX);
+		req->vuid = UID_FIELD_INVALID;
+		reply_force_doserror(req, ERRSRV, ERRbaduid);
+		return;
 	}
 
 	/* in user level security we are supposed to close any files
diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
index 75c2a15..cdd69f2 100644
--- a/source3/smbd/sesssetup.c
+++ b/source3/smbd/sesssetup.c
@@ -1149,6 +1149,11 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req)
 
 	/* Do we have a valid vuid now ? */
 	if (!is_partial_auth_vuid(sconn, vuid)) {
+		if (vuid != 0) {
+			data_blob_free(&blob1);
+			reply_force_doserror(req, ERRSRV, ERRbaduid);
+			return;
+		}
 		/* No, start a new authentication setup. */
 		vuid = register_initial_vuid(sconn);
 		if (vuid == UID_FIELD_INVALID) {