Re: [PATCH] Drop root privileges (if we have them)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

В сообщении от Среда 17 Сентябрь 2003 03:35 Zygmuntowicz Michal написал:
> I think we can use PWLib PProcess::SetUserName() and
> PProcess::SetGroupName(). Also, both uid and gid should be configurable.
> Then last problem is that if you specify username/groupname from the config
> file, then gk trace file will have uid/gid set to the original user,
> because it is created before config file is initialized. But I think it can
> be solved by writting a small routine that will read username/groupname
> from the config file before regular intialization is performed. Hint:

And what about (NOT implemented yet)?..
[Gatekeeper::Main]
LogFile=
TraceLevel=
...which also need to be loaded before config file is initialized.
If we are root, we can change ownership on logfile before dropping privileges, 
but if we are suid root it is not possible I think... 

It will be nice to have LogFileMode= option and PidFile= option, just for 
completeness. Pid-file creation procedure has the same issues on 
ownership/permissions...

--- gk.cxx.orig
+++ gk.cxx
@@
-       PFile::Remove(pidfile);
+       if (!PFile::Remove(pidfile))
+               PTRACE(2, "GK\tWARNING: Can't remove pid file " << pidfile);
@@
-       pid.WriteLine(PString(PString::Unsigned, getpid()));
+      if (!pid.WriteLine(PString(PString::Unsigned, getpid()))) {
+             PTRACE(2, "GK\tWARNING: Can't write pid file " << pidfile);
+      }

> Gatekeeper derives from PProcess, so you can use Set/GetUserName
> Set/GetGroupName directly from Gatekeeper::Main. As config variables I
> would recommend:
>
> [Gatekeeper::Main]
> # user can be specified either by his name or by his uid prefixed with '#' 
> (like '#0' for root)
> RunAsUser(or maybe DaemonUser)= 
> # group can be specified either by its name or by its gid prefixed with '#' 
> (like '#0' for root)
> RunAsGroup(or maybe DaemonGroup)=

I thing one doesn't need so "descriptive" config vars, I'd rather like them to 
be:
#
# User/Group: The name (or #number) of the user/group to run GnuGK as.
#
#   If these are commented out, the server will run as the user/group
#   that started it.  In order to change to a different user/group, you
#   MUST be root ( or have root privileges ) to start the server.
#
#    On SCO (ODT 3) use "User=nouser" and "Group=nogroup".
#
#  NOTE that some kernels refuse to setgid(Group) when the value of
#  (unsigned)Group is above 60000; don't use Group=nobody on these systems!
#
#User=
#Group=
#
as per Apache or FreeRadius...

>
> Also I would rather skip test for IsPrivilegedUser() - is it necessary?

One MUST have root privileges to switch (e.g. be setuid root), but it is not 
necessary to check if we are 'root' as PProcess::SetUserName / SetGroupName() 
returns FALSE on failure.

--
Best regards,
Andrey S Pankov.

>
> Would like to provide the improved patch?
>
>
> ----- Original Message -----
> From: "Chris Rankin" <rankincj@yahoo.com>
> Sent: Wednesday, September 17, 2003 12:39 AM
> Subject:  [PATCH] Drop root privileges (if we have them)
>
> > Hi,
> >
> > This patch is against 2.0.5 and has been tested on a Linux 2.4.22 box
> > running glibc-2.3.2 and compiled with gcc-3.2.3. The idea is that since
> > the GateKeeper is baring a lot of ports to the public Internet and does
> > not need to run with root privileges, then it should be able to be
> > launched at boot-time as an ordinary user and not as root.
> >
> > Basically, the GateKeeper looks up a given username and then calls
> > setgid() and setuid(). I have tested these syscalls under Linux to ensure
> > that the GateKeeper cannot later reclaim root privileges.
> >
> > I do not know how to achieve the same goal under Windows, so the WIN32
> > versions of my functions are just stubs.
> >
> > And for interest, did you know that you can remove a chunk of
> > template/inline code bloat by passing g++ the following flags?:
> >
> > -frepo -fno-implement-inlines
> >
> > The "-fno-implement-inlines" option tells the compiler not to generate a
> > static or extern version of any inline function, whereas -frepo enables
> > the compiler to eliminate duplicate template functions.
> >
> > Cheers,
> > Chris
>
> ---------------------------------------------------------------------------
>-----
>
> > --- gk.cxx.orig Wed Dec 11 07:30:52 2002
> > +++ gk.cxx Tue Sep 16 23:04:06 2003
> > @@ -203,6 +203,14 @@
> >
> >  #ifdef WIN32
> >
> > +inline bool IsPrivilegedProcess()
> > +{
> > + return false;
> > +}
> > +
> > +inline void SetProcessUser(const PString &username)
> > +{}
> > +
> >  BOOL WINAPI WinCtrlHandlerProc(DWORD dwCtrlType)
> >  {
> >   PTRACE(1, "GK\tGatekeeper shutdown");
> > @@ -211,6 +219,48 @@
> >  }
> >
> >  #else
> > +#  include <pwd.h>
> > +#  include <cstring>
> > +
> > +inline bool IsPrivilegedProcess()
> > +{
> > + return (::geteuid() == 0);
> > +}
> > +
> > +static void SetProcessUser(const PString &username)
> > +{
> > + static const size_t MAX_PASSWORD_BUFSIZE = 256;
> > +
> > + struct passwd userdata;
> > + struct passwd *userptr;
> > + char buffer[MAX_PASSWORD_BUFSIZE];
> > +
> > + int err = ::getpwnam_r(username,
> > +                        &userdata,
> > +                        buffer,
> > +                        sizeof(buffer),
> > +                        &userptr);
> > + if (userptr == NULL) {
> > + cerr << "Warning: Bad database entry for \""
> > +      << username
> > +      << "\": "
> > +      << ::strerror(err)
> > +      << endl;
> > + }
> > + else {
> > + err = ::setgid(userptr->pw_gid);
> > + if (err != 0)
> > + cerr << "Failed to set group ID: "
> > +      << ::strerror(err)
> > +      << endl;
> > +
> > + err = ::setuid(userptr->pw_uid);
> > + if (err != 0)
> > + cerr << "Failed to set user ID: "
> > +      << ::strerror(err)
> > +      << endl;
> > + }
> > +}
> >
> >  void UnixShutdownHandler(int sig)
> >  {
> > @@ -259,6 +309,7 @@
> >   "i-interface:"
> >   "l-timetolive:"
> >   "b-bandwidth:"
> > + "u-user:"
> >  #if PTRACING
> >   "t-trace."
> >   "o-output:"
> > @@ -363,6 +414,7 @@
> >   "  -i  --interface IP : The IP that the gatekeeper listen to\n"
> >   "  -l  --timetolive n : Time to live for client registration\n"
> >   "  -b  --bandwidth n  : Specify the total bandwidth\n"
> > + "  -u  --user name    : Run as this user\n"
> >  #if PTRACING
> >   "  -t  --trace        : Set trace verbosity\n"
> >   "  -o  --output file  : Write trace to this file\n"
> > @@ -394,6 +446,10 @@
> >  {
> >   PArgList & args = GetArguments();
> >   args.Parse(GetArgumentsParseString());
> > +
> > + if (args.HasOption('u') && IsPrivilegedProcess()) {
> > + SetProcessUser(args.GetOptionString('u'));
> > + }
> >
> >   PIPSocket::Address GKHome = INADDR_ANY;
>
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> List: Openh323gk-users@lists.sourceforge.net
> Archive: http://sourceforge.net/mailarchive/forum.php?forum_id=8549
> Homepage: http://www.gnugk.org/



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
List: Openh323gk-users@lists.sourceforge.net
Archive: http://sourceforge.net/mailarchive/forum.php?forum_id┘49
Homepage: http://www.gnugk.org/


[Index of Archives]     [SIP]     [Open H.323]     [Gnu Gatekeeper]     [Asterisk PBX]     [ISDN Cause Codes]     [Yosemite News]

  Powered by Linux