Wireshark · Ethereal-dev: Re: [Ethereal-dev] DCOM implementation, first try!

[Todd Sabin <tsabin@xxxxxxxxxxxxx>]

Ulf Lamping <ulf.lamping@xxxxxx> writes:

> Hi list!
> 
> Here is the first implementation of the DCOM dissection I mentioned earlier.

As a sort of pre-comment, this is a really large patch, and difficult
to consider in its entirety.  I think it might make sense to re-submit
it in stages, given that we now have an idea where you're going.
(Actually, I'm not sure I agree with your direction, but more on that
later.)

> All files for DCOM currently called something like: "packet-dcerpc-xy.c"
> (e.g. packet-dcerpc-oxid.c). I have called them "packet-dcom-xy.c", 
> as the DCOM dissectors mentioned above are sitting on top of DCOM, not on top of DCERPC,
> and of course the DCOM implementation itself is not part of DCERPC.

Err, well, this is really a matter of perspective, I think.  IMHO, the
best way to think of DCOM is as a 'discipline' for doing distributed,
reference counted, object based RPC, not as an actual network protocol
in and of itself.  The actual DCOM wire protocol happens to just be a
set of normal DCERPC interfaces used for activation, reference
counting, oxid resolution, etc. (the dcerpc-oxid, dcerpc-remact, etc
interfaces) along with whatever interfaces the objects in question
support.  The calls to those per-object interfaces are also DCERPC,
with the qualification that they all have the object flag specified
(hf_dcerpc_cn_flags_object), and the NDR for the requests starts with
an ORPC_THIS, and the replies start with an ORPC_THAT.  In other
words, DCOM is 'invisible' on the wire, and everything can be fully
described with DCERPC and NDR.  (Sometimes, the NDR is used to shovel
byte-bags around which have other, non-NDR, structure within.)

Hopefully, that made some sense, because it's the basis for most of my
comments below.  :)

> What I have done so far to add DCOM to Ethereal:

Right, on to the code.  Again, I think it'd be better to resubmit this
in easier to digest pieces.

> Changes in the existing code:
> -----------------------------
> Changes in dcerpc.c/.h:

You modified the types of dcerpc.ver and dcerpc.ver_minor from UINT8
to UINT16.  Why?  They're clearly 8 bit values on the wire.

You split each of dcerpc.cn_bind_trans_ver and ...ack_trans_ver into
major and minor versions.  Why?  The OpenGroup spec says a transfer
syntax id looks like:

     typedef struct {
         uuid_t if_uuid;
         u_int32 if_version;
     } p_syntax_id_t;

Admittedly, that spec is pretty old.  Do you have a more recent spec
with something different in it?

> new methods dissect_dcerpc_uuid() and similar

The comment about interfaces, not uuids, having versions is true, but
a bit off the mark.  What I mean is that if you're going to use a uuid
on the network, you need a version to go with it.  For DCOM, the
version is always 0 (that's specified in draft-brown-dcom-v1-spec-03).
So, I think the right thing is for dcerpc_get_uuid_name to also take a
version.  When you're looking up a uuid that has a version with it
(e.g., in Bind pdus) you'd pass that along.  If there's no version on
the wire with the uuid (presumably, some dcom scenario) then you'd
pass in 0 for the version.

Also, some uuid's will never be found in there, as they're not
associated with interfaces.  An example is the object uuid, when
present.  Looking those up doesn't hurt anything, but it does waste
time.

> Special uuid registration for DCOM subdissectors (I'm not happy with this).

I confess I'm not really clear about what you're trying to do here.  I
guess it's intended to be a way of factoring out common code from what
the per-interface dissectors will need to do.  Honestly, I'm not sure
that it (adding a separate registration method for dcom interfaces) is
worth it.  If you defined a dissect_ndr_orpc_this(...) and a
dissect_ndr_orpc_that(...), then subdissectors would just need to add
one line to each of their dissectors.  At which point, the DCOM
dissectors are just normal DCERPC dissectors that happen to have
something in common.

Ideally, all these subdissectors would be automatically generated from
IDL, anyway, which would make this irrelevant.

> Changes in dcerpc-ndr.c:
> added some simple datatypes: float, double, uuid

I'm a little worried about the possibility of the float and double
dissecting wrong, since you don't check the drep for the float format
(as you noted).  Actually, I'm not that worried about it, as I've
never seen an IEEE float on the wire, let alone a VAX one.  But I'm
not necessarily typical.  Maybe it's possible to put a "Not
implemented" message in the tree instead of doing the wrong thing if
it's not an IEEE float?

> New code implemented decoding the basic DCOM mechanisms (in WinNt4):
> --------------------------------------------------------------------
> implementation of a lot DCOM datatypes:
> -BYTE,WORD,DWORD,...
> -DATE,FILETIME,BSTR,OBJREF,DUALSTRINGARRAY,...
> -VARIANT (currently not all varianttypes)
> -SAFEARRAY (currently not all data types)

Some of these types, esp. VARIANT and SAFEARRAY, are more particular
to IDispatch than DCOM, per se.  Also, I didn't look at them that
closely, but I couldn't help but feeling that a lot of it could have
been done through calls to the existing dissect_ndr.. stuff.

> Implementation of the following DCOM-interfaces (still some lesser used methods missing):
> -IOXIDResolver (now implemented)
> -IRemoteActivation (now implemented)

Didn't look at them too closely, but I think it makes sense to leave
these as packet-dcerpc-..., for reasons above.

> -IRemUnknown (newly implemented)
> -IDispatch (newly implemented, not complete)

Didn't look at these.

> Conclusion:
> -----------
> I'm currently not satisfied with my implementation of the DCOM
> subdissector protocol registration inside packet-dcerpc.c.
> 
> I need some more example capture files, as there are still a lot of
> ToBeDone's inside the code.
> 
> Info: The code has the "ready to use" state, but is maybe not
> "production stable".

I hope you're not discouraged by my comments.  I think you've done a
lot of valuable work here, and it's just a matter of figuring out how
best to work it into ethereal.


Todd