On Jul 25, 2017, at 5:58 PM, Sultan, Hassan <sultah@xxxxxxxxxx> wrote:
> I'm not sure I understand what you mean here (likely due to me still ramping up on the code).
>
> Are you saying that, assuming the blob offset/length fields are part of the blob, using FT_BYTES for the blob here is wrong b/c the 3 fields under it are not within the offset/length defined for the blob, and instead FT_NONE should be used ? Should I understand from this that FT_NONE is purely an abstract grouping without any meaning as to position/length of the fields under it ?
I'm saying that there are probably people who think of the length and offset as part of the security information, along with the blob to which they refer, and people who think of the length and offset as separate from the blob to which they refer, so there's not an "obvious" way to represent them in the protocol tree ("obvious" in the sense that "nobody would make a good argument for doing it differently").
For people in the former group, the right way to do the field would be as an FT_NONE, with the three items underneath it, and with the FT_NONE item being composed of two disconnected ranges. The blob data itself could just be dissected as NTLMSSP or GSSAPI, without an FT_BYTES field; the top-level protocol item should have all the blob's data in it.
For people in the latter group, the right way to do it would be to have separate fields for the length and offset, not under the item for the security blob, with the security blob as a separate item - which, again, could just be NTLMSSP or GSSAPI, without an FT_BYTES field.
>> If the pointer and length are *not* considered part of the security blob, then
>> they shouldn't be under smb2.security_blob.
>
> Not sure what the authoritative source would be for that, but looking at the parser in Network Monitor 3 it has them outside the blob :
>
> UINT16 SecurityBufferOffset;
> UINT16 SecurityBufferLength;
> UINT32 Reserved2;
> switch
> {
> case CurrentProtocolOffset < SecurityBufferOffset:
> BLOB(SecurityBufferOffset - CurrentProtocolOffset) BufferPadding;
> }
> SMBSecurityBlob(SecurityBufferLength) securityBlob
So NetMon treats them as separate from the item to which they refer, i.e. the second way.
Whoever wrote the Wireshark code chose something like the first way, but I don't think it's done correctly.