Thanks for your good analysis.. some thoughts inline..

On Thu, 26 Feb 2004, Andrew Pimlott wrote:
> - You use "getuid() == 0 || geteuid() == 0" to check whether droproot
>   will be called.  Currently, they are the same, because we call
>   setuid(getuid()).  So the code would be clearer if it just used
>   getuid().  

True, I just copied this, didn't bother changing it.

> Also, it is redundant to check uid before setting
>   chroot_dir, username, since it will be checked before they are used.

Yep.  I didn't bother changing them because this was more of a 
cleanup, but it's better to do it :)
 
> - It is really not much trouble to drop root in the setuid root case.
>   The appended patch does this.  Note that now, geteuid() is the
>   appropriate thing to check, above.

Hmm.. IMHO, the code gets a bit harder to follow: to trace whether it 
works fine you'll have to check a bunch of calls to check that all the 
seteuid()'s are really dropped properly .. this makes it harder to 
understand; that's why I have wanted to avoid this.

My argument is that setuid-tcpdump is already such a wacky corner case 
that adding code to deal with that isn't probably worth the effort. 
 
> - initgroups does not really work after chroot, because it needs to open
>   the groups file.  On my (Linux) system, it seems to fall-back to
>   setting only the give gid, however it might behave less gracefully on
>   other systems.  I think it is better to initgroups before chroot.

Good point.  Or simpler, just do 'setgroups(0, NULL)' instead of 
initgroups?  Not maybe pedantically 100% correct, but serves the 
purpose..

> Regarding the side-effects of droproot:
> 
> - The -C problem argues, perhaps, for detecting when the protocol
>   analyzers will not be used, ie, when we are just dumping.  Does anyone
>   actually use this?

Dunno.  I think we should add a warning to be printed with -C if 
username/chroot will be done.
 
> - The resolver problem appears to be serious.  I doubt there is any
>   system that can do name resolution in a chroot, at least without
>   somehow preparing beforehand.  My system appears to fall back
>   gracefully to printing numbers, but I don't think this regression is
>   acceptible.  Is it possible that if you do a gethostbyaddr before the
>   chroot, it will read/open all necessary files, so that it will still
>   work after the chroot?  If this can't be made to work on all
>   platforms, an option not to chroot is required.

Hmm.. this should be looked at, I guess.  Remember though that 
gethostbyaddr is possibly not enough as one could look up IPv6 records 
too.  

-- 
Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings



-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:[EMAIL PROTECTED]

Reply via email to