twisted / ldaptor

LDAP server, client and utilities, using Twisted Python
MIT License
160 stars 53 forks source link

Malformed LDAP packets due to missing LDAPSearchResultReference implementation #47

Open flesser opened 8 years ago

flesser commented 8 years ago

The missing __str__() implementation of LDAPSearchResultReference can lead to malformed LDAP packets.

I tried running proxybase.py as a transparent LDAP proxy for (GitLab-)user authentication against a Microsoft Active Directory server and ended up with "Unsupported object type: id=60" failures originating somewere deep in GitLab's BER parser.

Investigating the Proxy -> Client communication with Wireshark showed several [Malformed Packet: LDAP] errors where the byte stream contained stuff like <instance object at 0x7fba757ed2d8> (with < as ASCII char 60 apparently being the cause for the "Unsupported object type: id=60" message).

On the Server -> Proxy stream, Wireshark showed searchResRef LDAP messages instead. Since I don't need those for my use case, I created a workaround by ignoring them:

    def handleProxiedResponse(self, response, request, controls):
        if isinstance(response, pureldap.LDAPSearchResultReference):
            return
        ...

But strictly speaking this is not a transparent proxy anymore.

psi29a commented 8 years ago

Thank you for reporting this. Looks like LDAPSearchResultReference is currently just a stub, as indicated by the comment. If you are able to implement a real __str__() in a pull request, I'd be happy to review it and add it in.

I'm currently in maintainer mode with Ldaptor, but I hope to pick back up on this sometime this year.

schlenk commented 8 years ago

A LDAPSearchResultReference would probably need special consideration in a proxy setting.

The upstream server tells you to contact a different LDAP server, so either the proxy should do that on its own (hiding the fact from the client) or it would pass that info on and the client might get references he cannot resolve (due to servers not reachable from his point in the network).

The implementation of the class should be rather simple (as it is just a list of LDAP URIs encoded in an ASN.1 BER Sequence):

SearchResultReference ::= [APPLICATION 19] SEQUENCE
                              SIZE (1..MAX) OF uri URI

Something along these lines:

   def __init__(self, uris):
         self.uris = uris
         ....

   def __str__(self):
         return str(BERSequence([BEROctetString(uri) for uri in self.uris] ,tag=self.tag))
psi29a commented 8 years ago

If someone would like to investigate this and perhaps also write a unit-test for it, I'd merge the PR. :)

cwaldbieser commented 8 years ago

I've been thinking about this-- it seems like the issue is that the LDAP client internal to the proxy is not chasing the reference itself. I'll take a look and see if there is some way to make the client do that.

I can see use cases for both having the proxy chase the reference, or having it return the reference and having the original client chase the reference. So the server should be able to handle this kind of result, but I think I'd like to see an ldaptor client be able to chase the reference, too.

If that all pans out, there should probably be some additional examples/variations in the cookbook documentation.