[Richard Sharpe <rsharpe@xxxxxxxxxxxxxxxxx>]

Hi Folks,

I am doing some work around protocol fuzzing (kinda) LDAP at my new place 
of employment, and I have been looking at the LDAP RFCs and how Ethereal 
displays LDAP packets.

I have these comments.

1. It would be nice if there was a repository of LDAP captures that 
together demonstrate many of the LDAP message types.

2. The dissector code for the MS CLDAP message types has been inserted 
between the code to dissect an LDAP search request message and the LDAP 
search response messsages. This is a minor issue and can be fixed by 
moving some code around.

3. While the current LDAP dissector code is fine for people who want to 
casually look at LDAP when dealing with other issues, it seems to me that 
it suffers from a number of problems for people who want to use Ethereal 
to learn seriously about LDAP or who want to use it test their own 
implementation.

Let me elaborate, however, let me first say that these criticisms in no 
way reflect on the people who have given what we already have. They 
are a result of approaching this from a different direction.

Also, I am not sure if I will find the time to make extensive changes to 
Ethereal to incorporate some of these fixes, but I thought it would be 
good to throw these comments out there for consideration.

The problems I see are:

- The displayed LDAP messages do not really follow the structure of an 
LDAP message according to the RFC. For example, RFC2251 says:

        LDAPMessage ::= SEQUENCE {
                messageID       MessageID,
                protocolOp      CHOICE {
                        bindRequest     BindRequest,
                        bindResponse    BindResponse,
                        unbindRequest   UnbindRequest,
                        searchRequest   SearchRequest,
                        searchResEntry  SearchResultEntry,
                        searchResDone   SearchResultDone,
                        searchResRef    SearchResultReference,
                        modifyRequest   ModifyRequest,
                        modifyResponse  ModifyResponse,
                        addRequest      AddRequest,
                        addResponse     AddResponse,
                        delRequest      DelRequest,
                        delResponse     DelResponse,
                        modDNRequest    ModifyDNRequest,
                        modDNResponse   ModifyDNResponse,
                        compareRequest  CompareRequest,
                        compareResponse CompareResponse,
                        abandonRequest  AbandonRequest,
                        extendedReq     ExtendedRequest,
                        extendedResp    ExtendedResponse },
                 controls       [0] Controls OPTIONAL }

The current dissector flattens this into one structure, so it looks like 
messageID is a part of the Bind or Search protocolOp, but that's not the 
case.

In addition, if controls are not present (since they are optoinal), then 
nothing is placed in the protocol tree, which seems reasonable on 
first sight. However, from the point of view of people trying to 
understand LDAP, it might be better to put an item in the protocol tree 
for Controls with a value of Not Present.

Then, when we get to the level of, say, a search message, we see things 
like this:

00000000: 305b 0201 0263 5604 1464 633d 6d75 7365  0[...cV..dc=muse
00000010: 6375 7269 7479 2c64 633d 636f 6d0a 0102  curity,dc=com...
00000020: 0a01 0202 0100 0201 0001 0100 a02d a31b  .............-..
00000030: 040b 6f62 6a65 6374 636c 6173 7304 0c70  ..objectclass..p
00000040: 6f73 6978 4163 636f 756e 74a3 0e04 0375  osixAccount....u
00000050: 6964 0407 7273 6861 7270 6530 00         id..rsharpe0.

There are two aspects of the current dissector that I find unsatisfactory 
with respect to this sequence of ASN.1.

The first is that the last two bytes, 3000, which encodes an empty 
AttributeDescriptionList (SEQUENCE OF AttributeDescription). While the 
list is empty, it seems to me that from a protocol completeness POV, we 
should show an empty AttributeDescriptionList. If nothing else, it 
accounts for more bytes in the ASN.1 data and prevents someone from 
doubting that the dissector is right.

The second is the filter:

  a02d a31b 040b 6f62 6a65 6374 636c 6173 7304 0c70 6f73
  6978 4163 636f 756e 74a3 0e04 0375 6964 0407 7273 6861
  7270 65

The current dissector does a great job of displaying this as a filter 
string, but there is sub-structure here, as it actually consists of

    a0 length 2d
    and [0] SET OF Filter
	a3 length 1b
	equalityMatch   [3] AttributeValueAssertion
		octet string value: objectclass = posixAccount
	a3 length 0e
        equalityMatch   [3] AttributeValueAssertion
		octet string value: uid = rsharpe

Now, while I wouldn't expect this to be the default way to view the filter 
expression, it would be nice, I think, if the data was there so we could 
switch into a mode where all the ASN.1 encoded data could be explained.

Are there any comments?

Regards
-------
Richard Sharpe, rsharpe[at]richardsharpe.com, rsharpe[at]samba.org,
sharpe[at]ethereal.com, http://www.richardsharpe.com