Removing geteuid() != 0 check in smbldap_open()

Jeremy Allison jra at samba.org
Fri Oct 17 21:03:10 GMT 2008


On Sat, Oct 18, 2008 at 07:50:17AM +1100, Andrew Bartlett wrote:
> I added this code originally to avoid security bugs where smbpasswd
> operations would fail (unable to open the file), but LDAP operations
> would succeed.

If we're checking at the upper levels for remote access correctly (ie. at the lanman,
samr and lsa layers), then this check is not needed.

I'm auditing the code in lanman.c, srv_lsa_nt.c and srv_samr_nt.c
right now to ensure we always do access checks first. (lanman.c
is trivially clean btw).

> But the reverse must also be true, so how do you intend to handle this
> for tdb and smbpasswd backed systems?

Don't understand what "the reverse must also be true" means here.

> The other reason to keep this is that as non-root, will we always have
> access to the ldapi socket, if so configured?  Normally it is
> world-accessible (but a sysadmin might restrict it), but if we ever add
> support for SASL EXTERNAL binds to our directory server, it will need to
> be root during that bind (so OpenLDAP can verify we are privileged via
> the getpeerid call). 

Then I'll add the become_root()/unbecome_root() pair around this
call instead.

The problem is that every single bug report we've ever had on this
code (and we've had quite a few) has involved us adding become_root()
unbecome_root() pairs around more and more areas.

This is the wrong thing to do. To be honest, we should just
add become_root()/unbecome_root() wrappers on all the passdb
operations so they'll never fail due to privilage errors and
always check at the incoming RPC/remote layer for the correct
privilage before starting the operation.

This is much closer to the way Windows operates.

Jeremy.


More information about the samba-technical mailing list