ruby-net-ldap icon indicating copy to clipboard operation
ruby-net-ldap copied to clipboard

Add support to use SNI

Open jpdasma opened this issue 3 years ago • 5 comments

Related to https://github.com/ruby-ldap/ruby-net-ldap/issues/405

jpdasma avatar Jun 07 '22 01:06 jpdasma

I'm thinking if it's better to have an option to use SNI or just always enable it?

The default openssl binary uses SNI by default.

jpdasma avatar Jun 07 '22 14:06 jpdasma

@HarlemSquirrel sorry for the direct ping, but can you share your insights regarding this? This patched work for my test, but I'm not sure if it's a good idea to just force the use of SNI for every TLS connection.

jpdasma avatar Jun 09 '22 01:06 jpdasma

Thank you for opening this issue and pull request!

This seems reasonable to me but we need to test with a few different LDAP servers and I don't have access to any that aren't public now. I was able to successfully connect to this public LDAP server:

ldap = Net::LDAP.new host: 'directory.cornell.edu', port: 636, encryption: :simple_tls, auth: { method: :anonymous }
entry = ldap.search_root_dse
entry.namingcontexts.last
# => "o=cornell university,c=us"

Looks like we also need some test updates.

HarlemSquirrel avatar Jun 09 '22 01:06 HarlemSquirrel

I'll fix the broken tests first.

jpdasma avatar Jun 09 '22 13:06 jpdasma

The failing test should be fixed now. I also tested with db.debian.org:636.

From some of my readings, SNI is an optional TLS extension, so it should be safe to be enabled by default.

Although we might still want to test it to other LDAP server with TLS.

jpdasma avatar Jun 10 '22 01:06 jpdasma

@HarlemSquirrel based on my tests, I think this should be safe to merge. I didn't encountered any issues with OpenLDAP and Windows LDAP.

jpdasma avatar Sep 02 '22 10:09 jpdasma

Thank you!

HarlemSquirrel avatar Sep 02 '22 11:09 HarlemSquirrel