Ethereal-dev: RE: problems with private_data (was Re: [Ethereal-dev] [DCE RPC] Incorrect diss

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Biot Olivier <Olivier.Biot@xxxxxxxxxxx>
Date: Tue, 8 Jun 2004 17:44:31 +0200
In order to fix this and other possible issues, maybe we have to define a
*real* private_data *structure* containing the information about the
protocol (such as the protocol dissector handle?) to which it belongs and a
"pointer-to-void" as is the case today.

Maybe we should also add a call-back routine to it so we know when the
private data can be freed?

Regards,

Olivier

|-----Original Message-----
|From: Todd Sabin
|
|Ok, you're going to love this (or not).  First, applying this patch
|should solve the problem, although it is not really the right fix.
|
|Index: packet-dcerpc.h
|===================================================================
|RCS file: /cvsroot/ethereal/packet-dcerpc.h,v
|retrieving revision 1.42
|diff -u -r1.42 packet-dcerpc.h
|--- packet-dcerpc.h	9 May 2004 10:03:37 -0000	1.42
|+++ packet-dcerpc.h	8 Jun 2004 14:10:38 -0000
|@@ -228,7 +228,7 @@
|    pass transport specific information down to the dissector from the
|    dissector that parsed this encapsulated calls. */
| 
|-#define DCERPC_TRANSPORT_SMB  1
|+#define DCERPC_TRANSPORT_SMB  0x12345678
| 
| typedef struct _dcerpc_private_info {
|     int transport_type;		/* Tag */
|
|
|Now, why on earth should that help, right?
|
|The reason that your packets weren't identified as EPM packets
|is that when dcerpc_binds hashtable is consulted to find the uuid that
|goes with the context id in the rqst/rply, it can't be found.  But,
|the bind packet is there in sniff, so why can't it be found?
|
|The binds hash table is keyed on the conversation, context id, and
|(wait for it) smb_fid.  But the sniff doesn't contain any SMB traffic,
|so why should that matter?
|
|The dcerpc dissector always tries to include an smb_fid by calling
|get_smb_fid (), which looks at private_data.  And here's where things
|go south.  get_smb_fid () just assumes that private_data is a pointer
|to dcerpc_private_info (if it's not NULL).  But there's no guarantee
|of that, and it won't be true unless dcerpc has been called from the
|smb dissector.
|
|In this case, dcerpc is being called directly by the tcp dissector,
|and so private_data is actually pointing to a struct tcpinfo.  Now,
|get_smb_fid() just treats that as a dcerpc_private_info, and so the
|seq from struct tcpinfo becomes the transport_type of
|dcerpc_private_info.  (Assuming int and guint32 are the same size.)
|If the seq from struct tcpinfo happens to be 1, get_smb_fid() will
|think there's a meaningful smb_fid in the private data, and use it.
|
|This is what happens in your first sniff, which is dissected wrong.
|Because of the presence of the syn/ack packet, the (relative) seq in
|the bind packet is 1, and the dcerpc_binds key contains a totally
|bogus smb_fid.  Your second sniff doesn't contain the syn/ack, so the
|relative seq is not 1, and it's keyed properly.
|
|It's important to note that the first sniff is the way things normally
|occur in the ncacn_ip_tcp scenario, so _all_ dcerpc over tcp sessions
|are going to have this problem, and it's not just a property of having
|anonymized a sniff.  I've confirmed that my normal dcerpc over tcp
|sniffs are dissected wrong by CVS head, too.
|
|As for why this doesn't apparently happen on Mac OS X, I'm not sure,
|but suspect it has something to do with endianness being different.
|E.g. if int and guint32 are different sizes in the Apple build, that
|would explain it.  I wouldn't expect that they are, but I think it's
|probably something like that.
|
|Now, someone's probably thinking, "How did this ever work?"  It seems
|that this misuse of struct tcpinfo as dcerpc_private_info has been
|going on for a long, long time, but what used to save us is that the
|members of struct tcpinfo that were being misused as smb_fid happened
|to be 0 almost all the time.
|
|According to
|http://www.ethereal.com/cgi-bin/viewcvs.cgi/ethereal/packet-tcp.h, Guy
|checked in a change on Dec 30 last year that added nxtseq in the
|middle of struct tcpinfo.  Prior to that, the stuff immediately
|following the seq was is_reassembled, urgent, and urgent_pointer,
|which are typically zero.  nxtseq is typically _not_ zero, so we have
|this problem.
|
|That change has been in starting with 0.10.1.  I tried building that
|from source, but couldn't, due to missing files.  0.10.2 did build,
|and does in fact have this problem.  0.10.0a does not have this
|problem.  So, I'm pretty sure this is the answer.
|
|Anyway, the patch above is just a harmless band-aid to make things
|work again.  (Although, if the tcp seq happens to be 0x12345678 in a
|dcerpc bind packet, it'll still be handled wrong.)  The right fix is
|to stop the misuse of private_data.  This kind of confusion can result
|any time a dissector might be called by two different dissectors, each
|of which is trying to pass different private_data.  I have no idea
|what the right way to fix that is, but perhaps the different types of
|private data should be registered, and the type of private data
|included along with the private data.