Privilege dropping for rtadvd

[l***@elandsys.com]

Hi,

I'm not sure if people might agree with this, but I'm interested
in having a dedicated user for rtadvd after it's done acquiring
the socket.

OpenBSD already does that:
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/rtadvd/rtadvd.c.diff?r1=1.35;r2=1.36

Linux's radvd goes further and implements privilege separation:

Changes since 1.0:
* Implement privilege separation on Linux: a master root process
(which is able to reconfigure interfaces) and the main process. There
is '-s' toggle to keep old behaviour.

(http://lists.litech.org/pipermail/radvd-devel-l/2008-February/000307.html)

This helped in migitation at least one security issue in the case of radvd.

From: http://www.openwall.com/lists/oss-security/2011/10/06/3:

1) A privilege escalation flaw was found in radvd, due to a buffer overflow
in the process_ra() function. ND_OPT_DNSSL_INFORMATION option parsing
"label_len" was not checked for negative values, leading to a "suffix"
buffer overflow which can lead to privilege escalation, at least if
radvd is compiled without GCC's stack protection. If radvd is invoked
without privilege separation (the -u option), this can lead to an
escalation to root privileges. Note: Red Hat Enterprise Linux starts
radvd by default with the unprivileged user. (CVE-2011-3601)

I'm not saying that netbsd's rtadvd has a security issue, but I think it would
help mitigate one if we drop privileges.

What do you guys think about this ?

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Christos Zoulas]

Post by l***@elandsys.com
Hi,
I'm not sure if people might agree with this, but I'm interested
in having a dedicated user for rtadvd after it's done acquiring
the socket.
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/rtadvd/rtadvd.c.diff?r1=1.35;r2=1.36

The problem is that after you drop privs you cannot start listening
to new interfaces that might appear, but the daemon does not do
this now, right?

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by Christos Zoulas

Post by l***@elandsys.com
Hi,
I'm not sure if people might agree with this, but I'm interested
in having a dedicated user for rtadvd after it's done acquiring
the socket.
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/rtadvd/rtadvd.c.diff?r1=1.35;r2=1.36

I don't see any reason why not.
I don't mind spending some time on this :)

Post by Christos Zoulas
The problem is that after you drop privs you cannot start listening
to new interfaces that might appear, but the daemon does not do
this now, right?

Sure it can because for IPv6 we just open a single socket not bound for
any specific interface.
We check for a valid interface though as we set IPV6_RECVPKTINFO on it.
Or should, I've not tested it though.

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[l***@elandsys.com]

Post by Roy Marples

Post by Christos Zoulas

Post by l***@elandsys.com
Hi,
I'm not sure if people might agree with this, but I'm interested
in having a dedicated user for rtadvd after it's done acquiring
the socket.
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/rtadvd/rtadvd.c.diff?r1=1.35;r2=1.36

I don't see any reason why not.
I don't mind spending some time on this :)

Well, I've already starting working on a diff. Would you be interested
in reviewing it :-) ?

Post by Roy Marples

Post by Christos Zoulas
The problem is that after you drop privs you cannot start listening
to new interfaces that might appear, but the daemon does not do
this now, right?

Sure it can because for IPv6 we just open a single socket not bound
for any specific interface.
We check for a valid interface though as we set IPV6_RECVPKTINFO on it.
Or should, I've not tested it though.
Thanks
Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by l***@elandsys.com
Well, I've already starting working on a diff. Would you be
interested
in reviewing it :-) ?

Sure!

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by l***@elandsys.com
Well, I've already starting working on a diff. Would you be
interested
in reviewing it :-) ?

Sure!

I didn't review a diff :(

Here's one I cooked up in my spare time.
Comments?

Any objections or I'll commit this soonish.

Roy

[Christos Zoulas]

-=-=-=-=-=-

Post by l***@elandsys.com
Well, I've already starting working on a diff. Would you be
interested
in reviewing it :-) ?

Sure!

I didn't review a diff :(
Here's one I cooked up in my spare time.
Comments?
Any objections or I'll commit this soonish.
Roy
-=-=-=-=-=-
Index: usr.sbin/rtadvd/rtadvd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/rtadvd/rtadvd.c,v
retrieving revision 1.43
diff -u -p -r1.43 rtadvd.c
--- usr.sbin/rtadvd/rtadvd.c 28 Jun 2013 07:59:32 -0000 1.43
+++ usr.sbin/rtadvd/rtadvd.c 28 Jun 2013 11:38:35 -0000
@@ -58,6 +58,7 @@
#include <util.h>
#endif
#include <poll.h>
+#include <pwd.h>
#include "rtadvd.h"
#include "rrenum.h"
@@ -177,6 +178,7 @@ main(int argc, char *argv[])
struct timeval *timeout;
int i, ch;
int fflag = 0, logopt;
+ struct passwd *pw;
/* get command line options and arguments */
#define OPTIONS "c:dDfM:Rs"
@@ -260,6 +262,37 @@ main(int argc, char *argv[])
} else
set[1].fd = -1;
+ errno = 0;

You should not need that, getpwnam() sets errno. Anyway syslog(3) calls a
bunch of stuff that can trash errno anyway...

+ syslog(LOG_INFO, "<%s> dropping privileges to %s",
+ __func__, RTADVD_USER);

I don't think that the function name is useful in a syslog message.

+ if ((pw = getpwnam(RTADVD_USER)) == NULL) {
+ /* Preserve the old behaviour if the user does not exist */
+ if (errno == 0) {
+ syslog(LOG_WARNING,
+ "<%s> user does not exist, not dropping privileges",

IBID

+ __func__);
+ goto setsig;
+ }
+ syslog(LOG_ERR, "getpwnam: %s: %m", RTADVD_USER);
+ exit(1);
+ }
+ if (chroot(pw->pw_dir) == -1) {
+ syslog(LOG_ERR, "chroot: %s: %m", pw->pw_dir);
+ exit(1);
+ }
+ if (chdir("/") == -1) {
+ syslog(LOG_ERR, "chdir(\"/\")");

It should be (for consistency):
syslog(LOG_ERR, "chdir /: %m");

+ exit(1);
+ }
+ if (setgroups(1, &pw->pw_gid) == -1 ||
+ setgid(pw->pw_gid) == -1 ||
+ setuid(pw->pw_uid) == -1)
+ {
+ syslog(LOG_ERR, "failed to drop privileges: %m");
+ exit(1);
+ }

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jean-Yves Migeon]

Post by Roy Marples

Post by l***@elandsys.com
Well, I've already starting working on a diff. Would you be
interested
in reviewing it :-) ?

Sure!

I didn't review a diff :(
Here's one I cooked up in my spare time.
Comments?
Any objections or I'll commit this soonish.

No objection, one comment: as you are adding the _rtadvd user within
the patch, I would rather end the getpwnam test in fatal() rather than
keeping "the old way" and jump to "setsig".

Most privsep daemons log the missing user and exit right after.

[Roy Marples]

Post by Jean-Yves Migeon
No objection, one comment: as you are adding the _rtadvd user within
the patch, I would rather end the getpwnam test in fatal() rather than
keeping "the old way" and jump to "setsig".
Most privsep daemons log the missing user and exit right after.

I'm just thinking about upgraders who fail to upgrade passwd/group and
mtree, reboot and their IPv6 only network fails to come up.
Anyone else with experience of upgrading stuff like this have any
opinion?

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jean-Yves Migeon]

Post by Roy Marples

Post by Jean-Yves Migeon
No objection, one comment: as you are adding the _rtadvd user within
the patch, I would rather end the getpwnam test in fatal() rather than
keeping "the old way" and jump to "setsig".
Most privsep daemons log the missing user and exit right after.

I'm just thinking about upgraders who fail to upgrade passwd/group and
mtree, reboot and their IPv6 only network fails to come up.
Anyone else with experience of upgrading stuff like this have any opinion?

I would say that one of the first things they will look at are the logs,
and as rtadvd will exit with a non zero status this will show up on boot
too.

If they upgrade the system but forget passwd/group, hmm, they are
shooting themselves in the foot; it is part of the postinstall/etcupdate
dance. IMHO the checks in the code are not really worth it.

[Lars Schotte]

On Sat, 06 Jul 2013 23:47:46 +0200

Post by Jean-Yves Migeon
If they upgrade the system but forget passwd/group, hmm, they are
shooting themselves in the foot; it is part of the postinstall/etcupdate
dance. IMHO the checks in the code are not really worth it.

why? either it will run with the user nobody, or the system can check if the
user exists and if not, then create it. like it does when you install some
webserver or software like that, so i do not see any problem in that matter.

[Jean-Yves Migeon]

On Sat, 06 Jul 2013 23:47:46 +0200 Jean-Yves Migeon

Post by Jean-Yves Migeon
If they upgrade the system but forget passwd/group, hmm, they are
shooting themselves in the foot; it is part of the
postinstall/etcupdate dance. IMHO the checks in the code are not
really worth it.

why? either it will run with the user nobody,

The last patch I saw was clearly checking for the presence of _rtadvd,
and if the getpwnam call failed, continue as usual (no setuid, no chroot).

Dropping to nobody is acceptable as failsafe; indeed not as good as
having a dedicated user, but ok. You still have to log for the absence
of _rtadvd though.

or the system can check if the user exists and if not, then create
it. like it does when you install some webserver or software like
that, so i do not see any problem in that
matter.

Bad idea for a daemon; it is something more suitable for the package
system rather than the daemon itself.

Cheers,

[Roy Marples]

Post by Jean-Yves Migeon

Post by Roy Marples

Post by l***@elandsys.com
Well, I've already starting working on a diff. Would you be
interested
in reviewing it :-) ?

Sure!

I didn't review a diff :(
Here's one I cooked up in my spare time.
Comments?
Any objections or I'll commit this soonish.

No objection, one comment: as you are adding the _rtadvd user within
the patch, I would rather end the getpwnam test in fatal() rather than
keeping "the old way" and jump to "setsig".
Most privsep daemons log the missing user and exit right after.

Now committed.

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Thor Lancelot Simon]

Post by Christos Zoulas
The problem is that after you drop privs you cannot start listening
to new interfaces that might appear, but the daemon does not do
this now, right?

Another alternative might be to adjust our system security policy so
that the system could be configured for a non-root user to do these
things. This is actually pretty easy to do with kauth, but figuring
out a clean userland interface to it is harder.

The clockctl device is the obvious prior art, though.

Thor

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Christos Zoulas]

On Jun 27, 1:20pm, ***@panix.com (Thor Lancelot Simon) wrote:
-- Subject: Re: Privilege dropping for rtadvd

| On Thu, Jun 27, 2013 at 03:01:18PM +0000, Christos Zoulas wrote:
| >
| > The problem is that after you drop privs you cannot start listening
| > to new interfaces that might appear, but the daemon does not do
| > this now, right?
|
| Another alternative might be to adjust our system security policy so
| that the system could be configured for a non-root user to do these
| things. This is actually pretty easy to do with kauth, but figuring
| out a clean userland interface to it is harder.
|
| The clockctl device is the obvious prior art, though.

We could start implementing a capabilities system, stored in the filesystem
and process like linux has. That would eliminate most setuid programs.

christos

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jean-Yves Migeon]

Post by Christos Zoulas
-- Subject: Re: Privilege dropping for rtadvd
| >
| > The problem is that after you drop privs you cannot start
listening
| > to new interfaces that might appear, but the daemon does not do
| > this now, right?
|
| Another alternative might be to adjust our system security policy so
| that the system could be configured for a non-root user to do these
| things. This is actually pretty easy to do with kauth, but
figuring
| out a clean userland interface to it is harder.
|
| The clockctl device is the obvious prior art, though.
We could start implementing a capabilities system, stored in the filesystem
and process like linux has. That would eliminate most setuid
programs.

That requires extended attributes support, no?

[l***@elandsys.com]

Post by Thor Lancelot Simon

Post by Christos Zoulas
The problem is that after you drop privs you cannot start listening
to new interfaces that might appear, but the daemon does not do
this now, right?

Another alternative might be to adjust our system security policy so
that the system could be configured for a non-root user to do these
things. This is actually pretty easy to do with kauth, but figuring
out a clean userland interface to it is harder.
The clockctl device is the obvious prior art, though.

I'd be more interested in porting capsicum, but that's wishful
thinking at this point :-)

Post by Thor Lancelot Simon
Thor

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Lars Schotte]

On Thu, 27 Jun 2013 10:32:23 -0700

Post by l***@elandsys.com
I'd be more interested in porting capsicum, but that's wishful
thinking at this point :-)

obviously, you are not the only one who thinks like that. dragonfly is also playing
with that idea, however, they have it easier, due to its history as a freebsd spin
off, but that problems will be overcome someday too.

however, i still do not see the need for putting rtadvd under so much
surveillance. there should not be a lot of code in programs like this and
that's still the best way to protect against some security holes.