Re: There seems to be a problem with dissect_nt_create_andx_response in epan/dissectors/packet_smb.c

[Richard Sharpe <realrichardsharpe () gmail com>]

On Sat, May 26, 2012 at 8:34 PM, Richard Sharpe
<realrichardsharpe () gmail com> wrote:
> Hi folks,
>
> I have noticed that there are some undissected bytes when I look at
> NTCreate&X response, and I guess I have to accept the blame for that.
> However, when I look at the code in Samba
> (source3/nttrans.c:reply_ntcreate_and_X I see the following:
>
>        SOFF_T(p, 0, SMB_VFS_GET_ALLOC_SIZE(conn,fsp,&smb_fname->st));
>        p += 8;
>        SOFF_T(p,0,file_len);
>        p += 8;
>
> This matches up with the following in the dissector:
>
>        /* allocation size */
>        proto_tree_add_item(tree, hf_smb_alloc_size64, tvb, offset, 8,
> ENC_LITTLE_ENDIAN);
>        offset += 8;
>
>        /* end of file */
>        /* We store the end of file */
>        if (fid_info) {
>                fid_info->end_of_file=tvb_get_letoh64(tvb, offset);
>        }
>        proto_tree_add_item(tree, hf_smb_end_of_file, tvb, offset, 8,
> ENC_LITTLE_ENDIAN);
>        offset += 8;
>
> However, then Samba does:
>
>       if (flags & EXTENDED_RESPONSE_REQUIRED) {
>                uint16_t file_status = (NO_EAS|NO_SUBSTREAMS|NO_REPARSETAG);
>                size_t num_names = 0;
>                unsigned int num_streams = 0;
>                struct stream_struct *streams = NULL;
>
>                /* Do we have any EA's ? */
>                status = get_ea_names_from_file(ctx, conn, fsp,
>                                smb_fname->base_name, NULL, &num_names);
>                if (NT_STATUS_IS_OK(status) && num_names) {
>                        file_status &= ~NO_EAS;
>                }
>                status = vfs_streaminfo(conn, NULL, smb_fname->base_name, ctx,
>                        &num_streams, &streams);
>                /* There is always one stream, ::$DATA. */
>                if (NT_STATUS_IS_OK(status) && num_streams > 1) {
>                        file_status &= ~NO_SUBSTREAMS;
>                }
>                TALLOC_FREE(streams);
>                SSVAL(p,2,file_status);
>        }
>        p += 4;
>        SCVAL(p,0,fsp->is_directory ? 1 : 0);
>
>        if (flags & EXTENDED_RESPONSE_REQUIRED) {
>                uint32 perms = 0;
>                p += 25;
>                if (fsp->is_directory ||
>                    can_write_to_file(conn, smb_fname)) {
>                        perms = FILE_GENERIC_ALL;
>                } else {
>                        perms = FILE_GENERIC_READ|FILE_EXECUTE;
>                }
>                SIVAL(p,0,perms);
>        }
>
> Which inserts an int and then a further 29 bytes if
> EXTENDED_RESPONSE_REQUIRED is in the incoming flags, and sticks a one
> byte is_directoy indicator in between.
>
> However, in the dissector we have (syncing up with the offset += 8 above):
>
>        offset += 8;
>
>        /* File Type */
>        ftype=tvb_get_letohs(tvb, offset);
>        proto_tree_add_item(tree, hf_smb_file_type, tvb, offset, 2,
> ENC_LITTLE_ENDIAN);
>        offset += 2;
>
>        /* IPC State */
>        offset = dissect_ipc_state(tvb, tree, offset, FALSE);
>
>        /* is directory */
>        isdir=tvb_get_guint8(tvb, offset);
>        proto_tree_add_item(tree, hf_smb_is_directory, tvb, offset, 1,
> ENC_LITTLE_ENDIAN);
>        offset += 1;
>
> Which will be wrong if EXTENDED_RESPONSES_REQUIRED ...
>
> I will have a look at the MS documents on this and try to prepare an update :-)

Having looked at [MS-SMB].pdf I find, at long last, that we can answer
some questions embedded in the code (I am the RJS who put comments in
there):

2.2.4.9.1
Client Request Extensions
An SMB_COM_NT_CREATE_ANDX request is sent by a client to open a file
or device on the server.
This extension adds the following:
�X�nAn additional flag bit is added to the Flags field of the
SMB_COM_NT_CREATE_ANDX request.
The additional flag, NT_CREATE_REQUEST_EXTENDED_RESPONSE, is used to request an
extended response from the server.
�X�nAn additional parameter value is added to the ImpersonationLevel field.
SECURITY_DELEGATION is added to allow the server to call other servers
while impersonating
the original client.

and ...

2.2.4.9.2
Server Response Extensions
A successful response takes the following format. If the server
receives more than one
SMB_COM_NT_CREATE_ANDX request from a client before it sends back any
response, then the
server can respond to these requests in any order.
When a client requests extended information, then the response takes
the form described below.
Aside from the WordCount, NMPipeStatus_or_FileStatusFlags, FileId,
VolumeGUID, FileId,
MaximalAccessRights, and GuestMaximalAccessRights fields, all other
fields are as specified in
[MS-CIFS] section 2.2.4.64.2.

So, there is a bunch of extra info in the response if we saw bit 0x10
of the [Create] Flags field set.

I suspect that this can be handled by adding to struct smb_info the following:

  gboolean extended_response;   /* Did the client request extended responses */

What say ye?

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe