[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#4750) libldap initialization of ~/.ldaprc and setuid



Full_Name: Quanah Gibson-Mount
Version: All
OS: NA
URL: 
Submission from: (NULL) (171.66.155.86)


The debian folks committed the following patch:

Modified: openldap/trunk-2.1/libraries/libldap/init.c
===================================================================
--- openldap/trunk-2.1/libraries/libldap/init.c	2006-11-12 10:08:20 UTC (rev
749)
+++ openldap/trunk-2.1/libraries/libldap/init.c	2006-11-14 00:54:38 UTC (rev
750)
@@ -252,6 +252,12 @@
 	char *home;
 	char *path = NULL;
 
+	if (getuid() != geteuid()) {
+		/* Caller is setuid -- don't read any per-user configs in
+		   these circumstances, as this may not be safe */
+		return;
+	}
+
 	if (file == NULL) {
 		/* no file name */
 		return;


You can see this is against 2.1, but the code is essentially the same in OL
HEAD.

The reason was:

+  * Don't check for user configuration files when the caller is setuid;
+    addresses #387467, which is a potential security hole allowing
+    libnss-ldap settings to be overridden.  Thanks to Stephen Frost for
+    bringing this to my attention.


which Howard note is not really the right way to do this.

[19:07] Howard Chu: that's the wrong fix
[19:07] Howard Chu: libnss-ldap should set NOINIT for its own usage.
[19:09] Quanah: so this patch doesn't really fix anything?
[19:09] Howard Chu: probably not.


However, one of my co-workers notes:


19:48 <rra@home> Well, I appreciate Howard's opinion here, but that's a crappy
fix because it means that every single binary that uses the LDAP libraries
    and may be setuid has to know to do this.
19:49 <rra@home> libnss-ldap should *also* set NOINIT, of course.
19:50 <rra@home> It sounded like it was reading dotfiles out of the user's home
directory, though.
19:50 <rra@home> Is that really what it's doing?
19:50 <rra@home> Something along those lines?
19:50 <Q> http://www.openldap.org/devel/cvsweb.cgi/libraries/libldap/init.c?hideattic=1&sortbydate=0
19:50 <Q> it can read .ldaprc, yes
19:50 <rra@home> Libraries reading dotfiles is all by itself pretty evil.
19:51 <rra@home> That to me violates abstraction layers.
19:51 <rra@home> Applications are not going to expect libraries to do that.
19:51 <rra@home> The *right* solution would to me be to not do that; failing
that, I think OpenLDAP should bear the responsibility to do it securely, which
    would include not reading user .ldaprc files when running as root.  (There
are other ways to prevent that, of course.)
19:55 <rra@home> Basically, my argument is more that there should be a mode that
loads user configuration, but it probably shouldn't be the default
    interface.
19:55 <rra@home> Anyway, Howard's certainly right that libnss-ldap needs to be
fixed regardless to pass that flag.


So, thoughts appreciated.

--Quanah