[RFC] SMB2 Compound related chain handling when generation of FileId has failed

[Jeremy Allison]

On Wed, May 13, 2020 at 10:01:38PM +0530, Anubhav Rakshit via samba-technical wrote:
> Issue:
> We have a scenario where an application sends a Compound Related chain
> consisting of:
> SMB2_CREATE
> SMB2_IOCTL
> SMB2_SET_INFO
> SMB2_CLOSE
> 
> SMB2_CREATE failed with NT_STATUS_ACCESS_DENIED and subsequent
> requests all fail. In Samba they return NT_STATUS_FILE_CLOSED.
> 
> When I tried the same against a Win2k12 server, I noticed that all the
> failed requests of the chain would return NT_STATUS_ACCESS_DENIED.
> 
> I believe this behaviour is also mentioned in the [MS-SMB2] Specs
> 3.3.5.2.7.2: Handling Compounded Related Requests
> 
> “When the current operation requires a FileId and the previous
> operation either contains or generates a FileId, if the previous
> operation fails with an error, the server SHOULD<223> fail the current
> operation with the same error code returned by the previous
> operation.“
> 
> I have written a smbtorture test case to verify the above. I also
> propose the following fix. I am attaching code for your reference.
> Kindly let me know if the changes look reasonable.

> diff --git source3/smbd/globals.h source3/smbd/globals.h
> index e9bcf45..b85f41a 100644
> --- source3/smbd/globals.h
> +++ source3/smbd/globals.h
> @@ -776,6 +776,8 @@ struct smbd_smb2_request {
>         /* Should we encrypt? */
>         bool do_encryption;
>         bool compound_related;
> +       bool create_fail;
> +       NTSTATUS compound_create_err;

Rather than having an extra bool "create_fail" and
an NTSTATUS compound_create_err you can set
smb2req->compound_create_err = NT_STATUS_OK
on create success, and then just do:

  if (!NT_STATUS_IS_OK(smb2req->compound_create_err)) {
		return smbd_smb2_request_error(req,
			req->compound_create_err);
  }

I don't think you need the bool here.

Great detective work though !

Thanks,

Jeremy.