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
LDAP: Do not require START_TLS for loopback connections #854
Conversation
If the ldap uri is resolved to a loopback address then do not require START_TLS. Signed-off-by: Samuel Cabrero <scabrero@suse.de>
Can one of the admins verify this patch? |
ok to test |
@simo5 this is one of the things I don't dare to include in the project without your blessing :-) So me and @scabrero talked about this over e-mail initially. There are some people who would like to run an LDAP server on ldap://localhost. While we both agreed that supporting ldapi:// might be a better way, what do you think allowing non-encrypted auth towards localhost? I was thinking about someone listening to the traffic on the localhost, but then you need either root or at least CAP_NET_RAW/CAP_NET_ADMIN.. |
:-) More seriously I would like to understand why this is needed (after all we already have the scary hidden option to avoid TLS, why is that not sufficient ?), and why it is safe(?), noting that in the code proposed you allow any connection to any port on all localhost addresses (you are checking for IN_LOOPBACKNET not just specifically IPADDR_LOOPBACK). The ldapi:// case is not just about encryption but also to access control, you do not get to interpose a socket that root set up with the right permissions on the filesystem. In this patch though there is no way to make sure we are restricting access only to the right peer if the port is above 1024. In a setup where root decides to set up the LDAP server on a higher port (say because they were told to run the LDAP server as non-root to improve security and they find it easier to just run on a higher port as that is allowed to non-root users), an attacker that finds a way to make the ldap server crash can then run their own ldap server on that port and feed arbitrary information to SSSD (which means privilege escalation to root almost certainly at that point). So I guess what I am saying is that it might be acceptable, perhaps with limiting also ports to < 1024 ... I do note that using certs in this scenario is hard (which is why I guess @scabrero wants to avoid TLS) because no sane public authority will give you a cert signed for 127.x.x.x or the name localhost[.localdomain] as is proper. However providing a tool that root can use to create a custom, locally trusted cert is probably not too hard and will provide a much better proposition wrt security. Soo ... long story short, at the moment I do not see a convincing argument to approve this patch as is on my side, can you at least better describe the use case ? The commit and this PR description are quite ... bare. |
@simo5, as you said the idea is to avoid having to set up certificates (or undocumented options) when sssd and ldap run in the same machine. I understand the potential security issues you pointed out, so would it be acceptable after adding these additional restrictions to sdap_is_loopback_address?
IMHO it is less secure to rely on that hidden option that allows everything than the proposed changes after we agree the full list of "restrictions". I will write a more extensive commit message if you are OK with the changes :) |
@scabrero To be honest I would prefer to write a tool that simply generates a local certificate, adds it to the local machine trust store and gives it to the LDAP server. Then there is no need for exceptions on the SSSD side, and a local attacker can't impersonate the server no matter what. |
Yes, we can close this. Thanks. |
If the ldap uri is resolved to a loopback address then do not require
START_TLS.
Signed-off-by: Samuel Cabrero scabrero@suse.de