uid_wrapper: Add uwrap_setresuid().

[Andrew Bartlett]

On Sat, 2011-10-08 at 10:30 +0200, Stefan (metze) Metzmacher wrote:
> Am 08.10.2011 10:01, schrieb Andrew Bartlett:
> > On Sat, 2011-10-08 at 09:44 +0200, Andreas Schneider wrote:
> >> On Saturday 08 October 2011 10:42:45 Andrew Bartlett wrote:
> >>> On Fri, 2011-10-07 at 12:05 +0200, Andreas Schneider wrote:
> >>>> - Log -----------------------------------------------------------------
> >>>> commit 4493c578b0da44ae2100cc0d4d6acc714bf39a3f
> >>>> Author: Andreas Schneider <asn at samba.org>
> >>>> Date:   Fri Oct 7 10:30:23 2011 +0200
> >>>>
> >>>>     uid_wrapper: Add uwrap_setresuid().
> >>>>     
> >>>>     Autobuild-User: Andreas Schneider <asn at cryptomilk.org>
> >>>>     Autobuild-Date: Fri Oct  7 12:04:05 CEST 2011 on sn-devel-104
> >>>
> >>> Andreas,
> >>
> >> Andrew, 
> >>
> >>> It's really great to see UID wrapper being ported into the source3 code,
> >>> but I just wanted to mention that this seems to have broken the Solaris
> >>> build.
> >>>
> >>> http://build.samba.org/build.cgi/build/2830602b39d10d5637469a8039ab51ee7329f
> >>> ad5
> >>>
> >>> I'm sure the fix is pretty simple, I just wanted to bring it to your
> >>> attention.
> >>
> >> there is more stuff broken than I thought, and I think I've fixed it now.
> >>
> >> http://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/s3-uid-wrapper
> >>
> >> I just wanted that someone looks over it before I push it.
> >>
> > 
> > Thanks.  I'm OK with these patches, except for 
> > http://git.samba.org/?p=asn/samba.git;a=commitdiff;h=17890a7e46b5227f4fa24c8326a4a13421d0edaa
> > 
> > The problem I have this this patch is if overriding the get/set uid
> > functions requires that we include a particular header, but the build
> > can continue without that header specified, we can get in a lot of
> > trouble quickly.  We should ensure that we do not obtain the system
> > prototypes for the replaced functions except via the header that has the
> > uid_wrapper replacements.
> > 
> > If we don't, we risk subtle, painful to debug issues in our test
> > environment (the same applies for socket_wrapper, and it is from that
> > experience that I give this warning).
> > 
> > I do realise this is difficult with geteuid() being defined via
> > unistd.h, but this is yet another reason why all Samba code should only
> > include system/ headers, not system headers directly.
> 
> I think Andreas should push this changes to make it work at all.
> Then we can think about a good generic strategie how to prevent such
> problems
> for all wrapper libraries.

Hmm, it seems that uid wrapper is indeed rather different to how
socket_wrapper (which I'm more familiar with) and nss_wrapper is
handled.  In these, we never include system headers directly, and always
include via system/passwd.h and system/network.h.  This ensures that we
always get the wrappers.  

In Samba4 it seems that uid handling has been so condensed that
uid_wrapper has only been included in a particular spots, and so I now
see why it hasn't been in a system/ header in the past.

The fact that the uid functions are in a very base system header
(unistd.h) does make this more challenging, but I think we have a very
good understanding of the 'right' way to do this.  I think we should
require that unistd.h is included via a system header, and put uid
wrapper hooks in each of those headers:

lib/replace/system/filesys.h:#include <unistd.h>
lib/replace/system/network.h:#include <unistd.h>
lib/replace/system/passwd.h:#include <unistd.h>

Outside self-contained tests, very few parts of Samba include unistd.h
directly, so this should not be too hard.

(Or bring the uid handling back into a small, isolated area of code, but
this seems more difficult and less reliable).

Andrew Bartlett


-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org