jeremycx / node-LDAP

LDAP binding for node.js
MIT License
221 stars 43 forks source link

No longer can use starttls with ldap:// uri #82

Closed wdouglascampbell closed 8 years ago

wdouglascampbell commented 8 years ago

I just updated to the latest version of node which forced me to switch from LDAP v1.2.0 to the current version.

Previously I had been connecting to my ldap server using ldap://serveraddress and starttls: true. Since upgrading to the latest ldap-client module, it seems the starttls: true option is being ignored and the instructions seem to suggest that I must use ldaps://serveraddress which I am not able to do.

Am I missing something or is support for starttls with ldapi:// uri no longer provided? If not provided, can it be added back?

Thanks!

Doug

jeremycx commented 8 years ago

ldaps:// now does exactly what starttls used to do . I went a long way down the starttls road before the whole thing got really messy internally, I suspect due to a couple of bugs in the underlying libraries.

So, unless you really need way to send a few messages to the LDAP server, and THEN start up TLS mid-stream, I think ldaps:// should be all you need, (but I may be missing a good use case - educate me).

I'm surprised to hear that ldapi:// even supported TLS at all. I'd be really interested in hearing what purpose encrypting a UNIX domain socket would serve.

wdouglascampbell commented 8 years ago

Oops. I have a typo in part of my original statement even though the title is correct. The issue is that starttls does not work with ldap:// uris (standard ldap uris) when it did previously. I was not trying to get it to work with sockets (ldapi://). That was a typo.

My issue is that I only have access to a non-encrypted standard connection to the LDAP server. The LDAP server expects all connections on ldap:// and then expects starttls to be used. This seems to be the preferred method that all my searches bring up and like I said, it did work on v1.2.0. (example of what I found: http://lmorgado.com/blog/tag/ldap/, see section 2.3)

With the correction to my typo, does my question make more sense and is there a solution for me?

Here is where starttls was original added: https://github.com/jeremycx/node-LDAP/pull/53. Note the example that it is using ldap:// not ldaps://. That is what I am hoping to get back.

Doug

jeremycx commented 8 years ago

No straight-up solution, and I'd be happy to put the starttls extensions on the wishlist, but man I had trouble making it work properly across different server implementations, and I ran out of time (that I'm still a little short on... heh)

In the meantime, what if you asked for ldaps:// on the ldap:// port:

"ldaps://your.server.tld:389"
wdouglascampbell commented 8 years ago

Thanks. I would appreciate the starttls extensions being placed on the wishlist. I will figure out some other solution for now but using starttls would be my preference.

Do you think if I tried to use something in the code like was previously there that it would work for me? My hope is that if it worked for me before the update from 1.2.0 that if I inserted the lines of code in the right place that it would work for me again even if it didn't work for all setups.

Thanks again.

jeremycx commented 8 years ago

I re-added the startTLS stuff in there, but I don't have a lot of confidence in it. You need to manage the TLS yourself...

So:

 ldap = new LDAP({
            uri: 'ldap://localhost:1234',
            base: 'dc=sample,dc=com',
            attrs: '*',
            validatecert: false
        }, function(err) {
            ldap.starttls(function(err) {
                ldap.installtls();
                if (ldap.tlsactive() !== 1) process.exit(1);
            });
        });

So, you do a connect as usual (this sends an anonymous bind in the clear - not harmful but your server might not like it). Then on the connection callback, you send the startTLS packet yourself - and wait for the success message from the server. You're then good to go (I think) - you'll want to check to make sure you have TLS for sure with ldap.tlsactive()

This is in git, but not in npm, so you'll need to use the repo to install. I'll roll a new version for npmjs if you verify this all works for you.

wdouglascampbell commented 8 years ago

Thanks! I will hopefully be able to try this out this evening or tomorrow and let you know you how it turns out.

wdouglascampbell commented 8 years ago

That did it! I needed to tweak my code a little based on what you included so that my bind didn't occur before the starttls was initiated. I do notice that I have to use validatecert: false so I am guessing that it only tries to validate the cert when opening the connection and not when performing starttls as when I manually check against the server using ldapsearch and starttls a valid certificate is returned.

Anyway, this works so please do roll it into a new version.

var options = {
    uri: 'ldap://localhost:1234',
    version: 3,
    validatecert: false,
};

var ldap = new LDAP(options, function(err) {
    ldap.starttls(function(err) {
        ldap.installtls();
        if (ldap.tlsactive() !== 1) process.exit(1);
        ldap.bind({ binddn: 'cn=xxxx,dc=example,dc=org', password: 'yyyxyxy'}, function(err) {
            if (err) {
                console.log(err);
            }
        });
    });
});
jeremycx commented 8 years ago

Great! Please make sure you test it well with automatic reconnects -- when the underlying library reconnects, it will run the new LDAP() callback again, restarting TLS and doing your rebind for you. It should "just work", but you should kick it through a bunch of disconnects to make sure there's nothing weird going on.

wdouglascampbell commented 8 years ago

Seems to be working through reconnects. All is good.

jeremycx commented 8 years ago

Wicked! I'll document and push a new point release this week!