Quanah Gibson-Mount quanah@stanford.edu writes:
Russ Allbery rra@debian.org wrote:
So, what does it do, then? How doesn't it work? What would work instead given the above constraint?
I'll note I opened ITS#4750 upstream on this issue. Howard has said that if a good security argument can be made, it could be committed into the current (2.3+ releases).
The security argument is made in the Debian bug:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387467
specifically:
| > I'm curious about this, are you sure a libldap would read an "ldaprc" | > file when run from a setuid program? | | Yep: | | # ls -l /usr/bin/passwd | -rwsr-xr-x 1 root root 32296 2006-08-25 19:49 /usr/bin/passwd | # mv /etc/ldap/ldap.conf /etc/ldap/ldap.conf.away | | $ cd /tmp | $ passwd testuser | passwd: unknown user testuser | $ echo 'TLS_CACERT /etc/ssl/certs/ldapca.pem' > ldaprc | $ passwd testuser | passwd: You may not view or modify password information for testuser. | | > Or that it'd read the | > current-directory ldaprc in that situation? Can you provide an strace | > showing this happening? | | The interesting fragments: | | execve("/usr/bin/passwd", ["passwd", "testuser"], [/* 16 vars */]) = 0 | [...] | access("/etc/suid-debug", F_OK) = -1 ENOENT (No such file or directory) | [...] | getuid32() = 1000 | [...] | geteuid32() = 0 | [...] | open("/etc/libnss-ldap.conf", O_RDONLY) = 3 | [...] | open("/etc/ldap/ldap.conf", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory) | open("ldaprc", O_RDONLY|O_LARGEFILE) = 3 | | There is no path component in the last open(), so ldaprc is always read | from the current directory. | | > Also, the user would have to have access to more than the ldaprc file, | > no? Since the user couldn't control what server is being connected to | > without more control on the system, or control over the DNS, etc. | | True, but DNS poisoning is trivial (especially in a large local network | with lots of not-so-trusted machines). If I'd trust the network I'd have | no need for TLS at all...
Clearly the user should have an /etc/ldap/ldap.conf under any normal circumstance, and a configuration that does not is most likely broken. However, my argument is that while there may be cases where a user creating a security vulnerability through a broken configuration is simply the user's problem, I believe this is a case where the action taken has no intuitive connection to security. This was initially noticed because a user was putting all their LDAP options in /etc/libnss-ldap.conf rather than in /etc/ldap/ldap.conf, which may be broken but which is not something that is obviously broken from a *security* standpoint.
Having a library attempt to open files in the current directory and read them for configuration is highly non-intuitive and unexpected. Having it read configuration out of the user's home directory is less so, but in the case of setuid programs is still rather disturbing, particularly since the setuid program has no direct control over this behavior.
So, I see two potential problems here:
* A user can override the LDAP configuration of a setuid binary and cause it to, for instance, trust LDAP servers that it wouldn't otherwise trust, which is a potential attack (albeit a difficult one to exploit). If the setuid binary knows that it's using LDAP, it can avoid this via an environment variable, but frequently given such things as NSS plugins and PAM modules the binary isn't going to know that LDAP is involved. The primary problem here is that said setuid binary will read the configuration file based on HOME from the enviroment (it appears to me from the code) or the current directory (the above example), which violates one of the standard secure programming practices for setuid binaries (never trust the process environment including working directory that you're handed by the user).
* The library may fall back to reading a file out of the current directory. I don't know if this behavior persists in 2.3; the above example is from 2.1. If it does persist, it opens other potential problems that don't involve setuid but instead involve creating files in the current working directory of a process one wishes to attack. Again, due to the plugin nature of current operating systems, the process being attacked may not even be aware that it's using LDAP. I think this exploit is unlikely to occur in practice since it requires some attacker control over the home directory of the process being attacked, but it's theoretically present.
Steve's patch addresses the first issue but doesn't do much about the second, so it's an improvement but I'm not sure it's a complete solution. Unfortunately, this support is also important for the current functionality of the command-line tools and doubtless other programs that use the LDAP libraries.
What we ideally need is some way for a user of the LDAP libraries to say, through the API, that user configuration files should not be loaded but that isn't a process-wide flag that interferes with the expected behavior of other uses of LDAP elsewhere in the same process. And as mentioned, I don't see an obvious way to do that without changing the API, which is, of course, a pain.
In the meantime, the patch isn't exactly something I'd want to take upstream either, but it at least addresses the most obvious problem, and more problematically I don't see a better way of addressing it other than saying "well, anyone without a system-wide ldap.conf loses." And I wouldn't be comfortable trying to defend that position.
I suppose another possible workaround specific to the NSS module (and probably the PAM module as well) would be to proactively check whether there's a system-wide ldap.conf file and fail immediately if there isn't. That leaves the problem open for other setuid uses of the LDAP libraries, but I don't expect there are a lot of those and what ones there may be are more likely to be able to use the LDAPNOINIT flag.
Russ Allbery wrote:
In the meantime, the patch isn't exactly something I'd want to take upstream either, but it at least addresses the most obvious problem, and more problematically I don't see a better way of addressing it other than saying "well, anyone without a system-wide ldap.conf loses." And I wouldn't be comfortable trying to defend that position.
I suppose another possible workaround specific to the NSS module (and probably the PAM module as well) would be to proactively check whether there's a system-wide ldap.conf file and fail immediately if there isn't. That leaves the problem open for other setuid uses of the LDAP libraries, but I don't expect there are a lot of those and what ones there may be are more likely to be able to use the LDAPNOINIT flag.
This whole line of reasoning appears to be based on the assumption that OpenLDAP's ldap.conf and pam/nss_ldap's ldap.conf are equivalent, which is false. pam/nss_ldap use their own config file, which is not the OpenLDAP default config file. A properly configured pam/nss_ldap fully specifies all of the options that pam/nss needs. In this situation, the presence of other ldap.conf and ldaprc files is irrelevant, since they can only be used to supply defaults when the calling application does not specify particular arguments. E.g., the URI in <openldap>/ldap.conf will only be used by libldap if the calling API doesn't specify a URI. DNS poisoning and other such bizarre attacks of course can apply regardless, but that's completely orthogonal to the question of where the URI was specified.
Apps that intend to use libldap and intend to be installed setuid have the responsibility to insure that they are properly and adequately configured, such that arbitrary defaults outside the app's awareness are never used. That's just common sense.
Howard Chu hyc@symas.com writes:
This whole line of reasoning appears to be based on the assumption that OpenLDAP's ldap.conf and pam/nss_ldap's ldap.conf are equivalent, which is false.
Well, no, my line of reasoning is not based on this. My line of reasoning is based on the fact that a user who doesn't understand the full list of parameters that must be specified in their local configuration opens themselves to a security vulnerability that would allow an attacker to spoof NSS information by changing the trust parameters for their LDAP configuration so that the system would trust an LDAP server under the control of an attacker. This attack is very similar to the sorts of attacks that can be launched against NIS; avoiding it is one of the reasons why Stanford, at the least, moved away from NIS and towards LDAP for NSS information since remote LDAP servers can be authenticated.
pam/nss_ldap use their own config file, which is not the OpenLDAP default config file. A properly configured pam/nss_ldap fully specifies all of the options that pam/nss needs. In this situation, the presence of other ldap.conf and ldaprc files is irrelevant, since they can only be used to supply defaults when the calling application does not specify particular arguments. E.g., the URI in <openldap>/ldap.conf will only be used by libldap if the calling API doesn't specify a URI.
So your suggestion is that nss-ldap should verify that every possible parameter with security sensitivity (definitely including, in this case, the configuration of trusted certificate authorities) is set explicitly in its configuration and, if not, abort? That's similar, although somewhat more complex, than the workaround that I suggested.
Apps that intend to use libldap and intend to be installed setuid have the responsibility to insure that they are properly and adequately configured, such that arbitrary defaults outside the app's awareness are never used. That's just common sense.
Libraries should not attempt to open configuration files in the current working directory. That's just common sense.
Since, unfortunately, we live in a world that doesn't operate entirely by the dictates of common sense for a variety of historic and hard-to-resolve reasons, I'm more interested in how to fix the problem going forward so that we aren't creating a situation where users can easily open a security vulnerability by accident than arguing about our personal conceptions of common sense. As I'm sure you'd agree, being able to override the system's concept of trusted certificate authorities pretty much defeats the entire purpose of using TLS to authenticate a remote LDAP server.
So, there's a couple of options that the NSS module should verify are set, obviously (the URI and the two TLS certificate authority paths). Probably BASE and TLS_REQCERT as well. Now, though, I'm not sure what the security implications are of not setting several other ones. From the descriptions, I can't immediately rule out possible interference with the system security policy via TLS_CALCHECK, TLS_CIPHER_SUITE, SASL_SECPROPS, SASL_MECH, SASL_REALM, SASL_AUTHCID, SASL_AUTHZID, DEREF, and REFERRALS, although for the most part I'd expect fiddling with them to fail.
That's a lot to verify is set, some of which many users will not *want* to set. Can you provide more information about exactly what parameters the NSS module needs to provide to ensure that none of the information in a local ldaprc file will be used? I'm not sure that I can picture what the code should look like to ensure this.
Russ Allbery wrote:
Howard Chu hyc@symas.com writes:
This whole line of reasoning appears to be based on the assumption that OpenLDAP's ldap.conf and pam/nss_ldap's ldap.conf are equivalent, which is false.
Well, no, my line of reasoning is not based on this. My line of reasoning is based on the fact that a user who doesn't understand the full list of parameters that must be specified in their local configuration opens themselves to a security vulnerability that would allow an attacker to spoof NSS information by changing the trust parameters for their LDAP configuration so that the system would trust an LDAP server under the control of an attacker. This attack is very similar to the sorts of attacks that can be launched against NIS; avoiding it is one of the reasons why Stanford, at the least, moved away from NIS and towards LDAP for NSS information since remote LDAP servers can be authenticated.
"users" = sysadmins here. Sysadmins have a responsibility to understand what they're doing with their systems, that's what sysadmins do.
So, there's a couple of options that the NSS module should verify are set, obviously (the URI and the two TLS certificate authority paths). Probably BASE and TLS_REQCERT as well. Now, though, I'm not sure what the security implications are of not setting several other ones. From the descriptions, I can't immediately rule out possible interference with the system security policy via TLS_CALCHECK, TLS_CIPHER_SUITE, SASL_SECPROPS, SASL_MECH, SASL_REALM, SASL_AUTHCID, SASL_AUTHZID, DEREF, and REFERRALS, although for the most part I'd expect fiddling with them to fail.
That's a lot to verify is set, some of which many users will not *want* to set. Can you provide more information about exactly what parameters the NSS module needs to provide to ensure that none of the information in a local ldaprc file will be used? I'm not sure that I can picture what the code should look like to ensure this.
Have a look in _nss_ldap_init_config in nss_ldap/util.c (there's a similar function in pam_ldap.c)...
BASE generally isn't useful in nss_ldap since you must configure the SSDs for each of the maps that nss_ldap will use. Likewise for pam_ldap, since it just uses the passwd SSD. So setting it or ignoring it is irrelevant.
BINDDN/BINDPW - you can't set a bindpw in ldaprc. Omitting these settings means you're using anonymous. The worst thing that can happen is that someone specifies a BINDDN but again, since bindpw is missing, it will still be anonymous.
saslid - ignored unless you set usesasl. If you enable sasl without setting a saslid, it's possible for some arbitrary ID to be configured. But again, without a password, such a setting is usually useless. If you're using a mech like GSSAPI or EXTERNAL that doesn't use passwords, it may connect successfully, with that ID's privileges. Whether the ID can see the relevant info that pam/nss needs would determine what happens next.
rootbinddn/rootbindpw - obviously no such thing exists in ldap.conf/ldaprc.
version - hardcoded to LDAP_VERSION3, pam/nss setting will override anything in ldap.conf/ldaprc. timelimit, bind_timelimit, ssl_on, referrals, restart - ditto
tls_checkpeer - this is a vulnerability, as the pam/nss default is to not set it and use the library default. Probably pam/nss should be patched to use an explicit setting here.
tls_cacertfile/cacertdir - generally one of these must be specified for TLS to work at all. If you omit these then yes some other trusted certs may get used. It may be legitimate to omit these settings if you have turned checkpeer off, but obviously that's an insecure configuration to begin with.
tls_ciphers - the default is ALL, so the only consequence of omitting this setting is that someone might set a more restrictive set of ciphers.
tls_cert / tls_key - if not specified, none is used. If left blank, at worst you'll get a client cert used where none was needed.
tls_randfile - ignored on Linux and any platform that provides /dev/urandom. On systems without it, it would be possible to point this at a source of constant numbers, thus allowing you to predict the keys of all TLS sessions.
sasl_secprops - it would be possible to specify weaker props if this value is not set.
The pam/nss config has no option to control CRL checking, so that can only come ldap.conf.
Howard Chu hyc@symas.com writes:
Have a look in _nss_ldap_init_config in nss_ldap/util.c (there's a similar function in pam_ldap.c)...
[...]
Thank you! This was extremely helpful to me. It looks like the only real issues from an NSS (and PAM) perspective are:
tls_checkpeer - this is a vulnerability, as the pam/nss default is to not set it and use the library default. Probably pam/nss should be patched to use an explicit setting here.
tls_cacertfile/cacertdir - generally one of these must be specified for TLS to work at all. If you omit these then yes some other trusted certs may get used. It may be legitimate to omit these settings if you have turned checkpeer off, but obviously that's an insecure configuration to begin with.
since everything else is either irrelevant, being set already, or requires ridiculously tortured logic to become an exploit.
I assume from the ldap.conf documentation that if tls_cacertfile is set, tls_cacertdir is irrelevant? Or are both explored for a root cert to validate the remote server?
I think that if both the NSS and PAM modules deal with those variables, that removes most of my concern. I'd still feel generally better with a safety net in the library for setuid processes on the principle of defense in depth and because safely using the LDAP library in such a situation requires thinking more about configuration initialization than I think some users may realize, but I'll freely admit that my concern at that point is theoretical.
Howard Chu wrote:
saslid - ignored unless you set usesasl. If you enable sasl without setting a saslid, it's possible for some arbitrary ID to be configured. But again, without a password, such a setting is usually useless. If you're using a mech like GSSAPI or EXTERNAL that doesn't use passwords, it may connect successfully, with that ID's privileges. Whether the ID can see the relevant info that pam/nss needs would determine what happens next.
The version of nss_ldap I'm looking at has GSSAPI hardcoded, so much of this is moot. You'll have to configure a credential cache, and ldap.conf can't provide that.
sasl_secprops - it would be possible to specify weaker props if this value is not set.
The worst you could do is turn off the security layer, which nss_ldap turns off by default anyway.
Russ Allbery wrote:
I assume from the ldap.conf documentation that if tls_cacertfile is set, tls_cacertdir is irrelevant? Or are both explored for a root cert to validate the remote server?
Both will get used.
I think that if both the NSS and PAM modules deal with those variables, that removes most of my concern. I'd still feel generally better with a safety net in the library for setuid processes on the principle of defense in depth and because safely using the LDAP library in such a situation requires thinking more about configuration initialization than I think some users may realize, but I'll freely admit that my concern at that point is theoretical.
I'm not totally convinced yet, will think about it. The patch would have to be #ifdef'd (HAVE_GETEUID or something) since it would not be relevant on Windows and some other obscure platforms.
-
Howard Chu -
Russ Allbery