-
-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix TLS dialing #16
Fix TLS dialing #16
Conversation
Signed-off-by: Knut Ahlers <knut@ahlers.me>
@Xaroth this should fix the TLS issues you're having with LDAPs connections. Sadly I don't have a LDAPs server to test the change. Do you have a chance to test it? |
It works in part; however, there is a caveat: To avoid DNS hijacking, a common configuration setup is to point ldap to a fixed IP address rather than a domain name (i.e. ldaps://127.0.0.1 ). By setting ServerName to host, verification fails, because the certificate for the LDAP server is issued for 'localhost' (or 'ldap.domain.tld'), not '127.0.0.1'. Now this can be worked around by hard-locking the dns name in /etc/hosts , but a cleaner method would be to introduce a config setting server_name, where you can configure the expected server name of the LDAP host (defaulting to the host part of the server URI, possibly). |
Adding to this, it might also be an option to allow to set the InsecureSkipVerify setting, for self-signed certificates. I don't recommend people using that in production envs, but in test envs it's very common to see self-signed certs being used to test. |
Signed-off-by: Knut Ahlers <knut@ahlers.me>
I've added configuration to support both validations: Either override the hostname for the certificate validation or disable the validation. Please have another look. 🙂 |
That seems to work fine, I left one remark on the commit, but other than that it works in my test environments 👍 |
Signed-off-by: Knut Ahlers <knut@ahlers.me>
Now both parameters are taken into account. I'm not entirely sure what happens in the TLS connection when both parameters are set but they should work. If you could test the behaviour with both options set this would be great! |
InsecureSkipVerify will (if memory serves me well) cause the TLS validation step to skip, therefore not checking for cert validity, CA validity or hostname comparison, so ServerName then becomes nothing more than an indicator (and could even be wrong/missing/empty). The potential issue that it solves is that how you have it now, people can keep their config, but just add the I checked the changes, they seem to work fine 👍 |
In that case: Thank you very much for bringing up this issue and for the assistance testing the changes! 🙂 |
refs #14