Ronnie Sahlberg <[EMAIL PROTECTED]> 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
