Ronnie Sahlberg <sahlberg@xxxxxxxxxxxxxxxx> writes:
> Hi,
>
> Looks very good.
> But i have some suggestions:
> [...]
>
> 4, in dissect_dcerpc_ntlmssp_negotiate()
> You use tvb_get_letohs() to read the 16bit bitmask with the flags.
> This is a bug and will not work if the host sending the packet is big
> endian.
> The byteorder of DCERPC types are always encoded as the native format of the
> sender.
> There are other users of these things which are not x86 based. Think NT for
> MIPS/ALPHA/PPC/...
> (dead platforms now, but they do exist).
> Instead, when reading datatypes for DCERPC you should use the accessors
> specially made for DCERPC.
> These accessors are called dissect_ndr_xxx().
I'm not sure about that. As far as the NDR is concerned, these auth
blobs are just byte arrays. Also, these same NTLMSSP blobs are used
in a lot of other contexts besides just DCERPC and they themselves
contain no indicator of endianness, so I wouldn't be at all surprised
if they're required to be little endian. Not sure, though.
That leads into my main suggestion, which is that I think you (Devin)
should create an entirely new dissector, say packet-ntlmssp.c(?),
instead of adding this to DCERPC. There are two reasons. First, as
mentioned above, NTLMSSP can be used in many other contexts (LDAP,
HTTP, Telnet, etc.), and having a separate dissector for it would make
your work usable for those, as well. Second, DCERPC auth data doesn't
have to be NTLMSSP. You need to check that the auth_type field
(hf_dcerpc_auth_type) is 10 before dissecting it as NTLMSSP. So the
best approach would be to have DCERPC do a handoff to a subdissector
based on auth-type and have the NTLMSSP dissector register with it for
type 10.
That may be more work than you're looking for, but you did ask. :)
Todd
