Stuart Henderson <stu <at> spacehopper.org> writes:
>
> On 2009-01-07, patrick keshishian <pkeshish <at> gmail.com> wrote:
>
> >>
> >>
> >
http://git.ozlabs.org/?p=ppp.git;a=commit;h=a00baab063b349591289cbde22ab40cf80b8f0af
> >>
> >> We changed to use setresuid() rather than setuid(), but this didn't change
> >> behaviour here. I guess many people will run pppd as root so they won't
> >> notice the problem (afaict, it only happens if you start as a non-root
> >> member of the group "network").
> >
> >
> > As discussed (off-line) this was changed in -r1.45 by d...@.
> > Since no one else is chiming in, I'm asking if someone will
> > looking into updating this, or was there a valid reason why
> > it was decided to prevent scripts to run with root privilege
> > in our local copy of pppd?
>
> you misinterpreted this,
>
> >> We changed to use setresuid() rather than setuid(), but this didn't change
> ^^^^^^^^^^^^^^^^^^^^^^
> >> behaviour here.
> ^^^^^^^^^^^^^^
>
> upstream's old code: drop privileges
>
> >> - (void) chdir ("/"); /* no current directory. */
> >> - setuid(geteuid());
I don't know how to use git, nor am I finding their web-interface very
intuitive, but from the link you provided, looking at the "diff" for
main.c, upstream never had setuid(geteuid()) as far as I can see:
/* Leave the current location */
- (void) setsid(); /* No controlling tty. */
+ (void) setsid(); /* No controlling tty. */
(void) umask (S_IRWXG|S_IRWXO);
- (void) chdir ("/"); /* no current directory. */
+ (void) chdir ("/"); /* no current directory. */
+ setuid(0); /* set real UID = root */
setgid(getegid());
They only added the setuid(0).
> upstream's new code: setuid(root) (even though it's already running
> as root..?)
Effective uid may be root, assuming they are installing it like OpenBSD
is, set-user-ID bit set, but real uid is that of the user who invoked
pppd.
> >> + (void) chdir ("/"); /* no current directory. */
> >> + setuid(0); /* set real UID = root */
>
> the setuid() -> setresuid() change made in OpenBSD did not affect this.
> the old version in OpenBSD and upstream both dropped privileges for
> these scripts.
OpenBSD's copy changed this -r1.44 -r1.45 main.c
@@ -1195,8 +1203,14 @@ run_program(prog, args, must_exist)
(void) setsid(); /* No controlling tty. */
(void) umask (S_IRWXG|S_IRWXO);
(void) chdir ("/"); /* no current directory. */
- setuid(geteuid());
- setgid(getegid());
+
+ /* revoke privs */
+ uid = getuid();
+ gid = getgid();
+ if (setresgid(gid, gid, gid) == -1 || setresuid(uid, uid, uid) == -1) {
+ syslog(LOG_ERR, "revoke privileges: %s", strerror(errno));
+ _exit(1);
+ }
setuid(geteuid()) is equivalent to setuid(0) since set-user-ID pppd
will have effective uid of 0 (file owner is root). This was changed
to setuid(getuid()) which changes the uid to the userid invoking
pppd.
Which is causing the problem of ip-up not being able to update the
routes: insufficient privileges.
--patrick
