["Ronnie Sahlberg" <ronnie_sahlberg@xxxxxxxxxxxxxx>]

Good catch.

Hm.  we must be more careful with how we handle private_data.   similar
problems have occured previously.

I think the solution would be to add a new entry point in dcerpc :
dissect_dcerpc_over_smb() and call this handle from smb.
Then move all private_data access assuming it contains a fid into this
subroutine.
If this new entry point is called, we pick up the fid from private_data, if
the original entrypoint is called
we dont look for this in private_data.

I will check in a patch for this later today.


----- Original Message ----- 
From: "Todd Sabin"
Sent: Wednesday, June 09, 2004 1:21 AM
Subject: problems with private_data (was Re: [Ethereal-dev] [DCE
RPC]Incorrect dissection with CVS version 20040603153321)


> Jean-Baptiste Marchand writes:
>
> > Hello,
>
> Hi, answer below.  Leaving the context...
>
> > * Jean-Baptiste Marchand  [04/06/04 - 11:29]:
> >
> >> it seems that there is a dissection problem with a current version of
> >> the DCE RPC dissector.
> >>
> >> The first attached capture (epm1_anon.cap) contains 6 frames, 2 two TCP
segments
> >> (SYN-ACK, ACK) and 4 DCE RPC PDUs. The last two DCE RPC PDUs are not
> >> properly dissected as EPM operations.
> >>
> >> The second attached capture (epm2_anon.cap) is identical to the first
one,
> >> except that the first TCP segment has been removed. The last two DCE
RPC
> >> PDUs are properly dissected as EPM operations.
> >>
> >> The only difference is that in the first case, we see a SYN-ACK TCP
> >> segment and thus, this might be something related to TCP conversations?
> >>
> >> PS: attached traces have been anonymized with ipsumdump
> >> (http://www.icir.org/kohler/ipsumdump/), thus IP addresses are
different
> >> in the traces but they were both generated from the same original
trace.
> >
> > I'm a bit confused because I've tried to open these two captures on a
> > recent build of ethereal on MacOS X (CVS tree updated this morning) and
> > the two are dissected properly.
> >
> > On the other hand, I've just updated my CVS tree on my FreeBSD laptop
> > and I still have the same problem I described on ethereal-dev@.
> >
> > It does not seem to be related to a difference in my preferences, as
> > I've tried with the default ethereal preference settings (I rename the
> > ~/.ethereal directory to ~/.ethereal_old).
> >
> > Follow tethereal outputs for me (checksum is incorrect because IP
> > addresses have been modified because of anonymization with ipsumdump):
> >
> > jbm@garbarek ~> tethereal -r epm1_anon.cap
> >   1   0.000000 194.115.240.203 -> 194.115.240.48 TCP loc-srv > 1053
> > [SYN, ACK] Seq=0 Ack=1 Win=17520 [CHECKSUM INCORRECT] Len=0 MSS=1460
> >   2   0.001235 194.115.240.48 -> 194.115.240.203 TCP 1053 > loc-srv
> > [ACK] Seq=1 Ack=1 Win=17520 [CHECKSUM INCORRECT] Len=0
> >   3   0.006250 194.115.240.48 -> 194.115.240.203 DCERPC Bind: call_id: 1
> > UUID: EPM
> >   4   0.016912 194.115.240.203 -> 194.115.240.48 DCERPC Bind_ack:
> > call_id: 1 accept max_xmit: 5840 max_recv: 5840
> >   5   0.018674 194.115.240.48 -> 194.115.240.203 DCERPC Request:
> > call_id: 1 opnum: 3 ctx_id: 0
> >   6   0.021197 194.115.240.203 -> 194.115.240.48 DCERPC Response:
> > call_id: 1 ctx_id: 0
> >
> > jbm@garbarek ~> tethereal -r epm2_anon.cap
> >   1   0.000000 205.61.107.240 -> 205.61.107.6 TCP 1053 > loc-srv [ACK]
> > Seq=0 Ack=0 Win=17520 [CHECKSUM INCORRECT] Len=0
> >   2   0.005015 205.61.107.240 -> 205.61.107.6 DCERPC Bind: call_id: 1
> > UUID: EPM
> >   3   0.015677 205.61.107.6 -> 205.61.107.240 DCERPC Bind_ack: call_id:
> > 1 accept max_xmit: 5840 max_recv: 5840
> >   4   0.017439 205.61.107.240 -> 205.61.107.6 EPM Map request
> >   5   0.019962 205.61.107.6 -> 205.61.107.240 EPM Map response
> >
> >
> > As you can, frames 5 and 6 are not properly dissected in the first trace
> > whereas in the second one, frames 4 and 5 (the first frame is remove in
> > the second trace), there are properly recognized as EPM operations...
> >
> > Any ideas?
>
> 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.
>
>
> Todd
>