Re: disable fs access on ripd
reads OK for me as well
On 10:28 Sat 03 Nov , Claudio Jeker wrote:
> On Sat, Nov 03, 2018 at 10:24:23AM +0100, Remi Locherer wrote:
> > On Tue, Oct 30, 2018 at 05:31:04PM +, Ricardo Mestre wrote:
> > > clearly an oversight due to looking at too many daemons at the same
> > > time. since the only thing ripd needs to do is unlink the socket I think
> > > we can remove control_cleanup, even though I'd rather do this
> > > introducing pledge, but for now it's a great compromise.
> > >
> >
> > In addition to your diff this pledges the rde and ripe.
> >
> > OK?
>
> Reads OK. Not sure about the ripe pledge but looking at ospfd I think it
> should be right. Can't test right now.
>
> > Index: control.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ripd/control.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 control.c
> > --- control.c 17 Jan 2017 22:10:56 - 1.25
> > +++ control.c 3 Nov 2018 09:11:39 -
> > @@ -100,14 +100,6 @@ control_listen(void)
> > return (0);
> > }
> >
> > -void
> > -control_cleanup(char *path)
> > -{
> > - event_del(_state.ev);
> > - event_del(_state.evt);
> > - unlink(path);
> > -}
> > -
> > /* ARGSUSED */
> > void
> > control_accept(int listenfd, short event, void *bula)
> > Index: control.h
> > ===
> > RCS file: /cvs/src/usr.sbin/ripd/control.h,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 control.h
> > --- control.h 2 Aug 2016 16:05:32 - 1.5
> > +++ control.h 3 Nov 2018 09:11:20 -
> > @@ -39,6 +39,5 @@ int control_listen(void);
> > void control_accept(int, short, void *);
> > void control_dispatch_imsg(int, short, void *);
> > intcontrol_imsg_relay(struct imsg *);
> > -void control_cleanup(char *);
> >
> > #endif /* _CONTROL_H_ */
> > Index: rde.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 rde.c
> > --- rde.c 3 Sep 2016 10:28:08 - 1.21
> > +++ rde.c 3 Nov 2018 07:38:41 -
> > @@ -109,6 +109,9 @@ rde(struct ripd_conf *xconf, int pipe_pa
> > setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
> > fatal("can't drop privileges");
> >
> > + if (pledge("stdio", NULL) == -1)
> > + fatal("pledge");
> > +
> > event_init();
> >
> > /* setup signal handler */
> > Index: ripd.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 ripd.c
> > --- ripd.c 3 Sep 2016 10:28:08 - 1.30
> > +++ ripd.c 3 Nov 2018 09:14:38 -
> > @@ -211,6 +211,11 @@ main(int argc, char *argv[])
> > rde_pid = rde(conf, pipe_parent2rde, pipe_ripe2rde, pipe_parent2ripe);
> > ripe_pid = ripe(conf, pipe_parent2ripe, pipe_ripe2rde, pipe_parent2rde);
> >
> > + if (unveil("/", "") == -1)
> > + fatal("unveil");
> > + if (unveil(NULL, NULL) == -1)
> > + fatal("unveil");
> > +
> > event_init();
> >
> > /* setup signal handler */
> > @@ -276,7 +281,6 @@ ripd_shutdown(void)
> > if_del(i);
> > }
> >
> > - control_cleanup(conf->csock);
> > kr_shutdown();
> >
> > log_debug("waiting for children to terminate");
> > Index: ripe.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 ripe.c
> > --- ripe.c 3 Sep 2016 10:28:08 - 1.22
> > +++ ripe.c 3 Nov 2018 09:07:24 -
> > @@ -196,6 +196,9 @@ ripe(struct ripd_conf *xconf, int pipe_p
> > iface->name);
> > }
> >
> > + if (pledge("stdio inet mcast", NULL) == -1)
> > + fatal("pledge");
> > +
> > evtimer_set(>report_timer, report_timer, oeconf);
> > start_report_timer();
> >
>
> --
> :wq Claudio
Re: disable fs access on ripd
On Sat, Nov 03, 2018 at 10:24:23AM +0100, Remi Locherer wrote:
> On Tue, Oct 30, 2018 at 05:31:04PM +, Ricardo Mestre wrote:
> > clearly an oversight due to looking at too many daemons at the same
> > time. since the only thing ripd needs to do is unlink the socket I think
> > we can remove control_cleanup, even though I'd rather do this
> > introducing pledge, but for now it's a great compromise.
> >
>
> In addition to your diff this pledges the rde and ripe.
>
> OK?
Reads OK. Not sure about the ripe pledge but looking at ospfd I think it
should be right. Can't test right now.
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/control.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 control.c
> --- control.c 17 Jan 2017 22:10:56 - 1.25
> +++ control.c 3 Nov 2018 09:11:39 -
> @@ -100,14 +100,6 @@ control_listen(void)
> return (0);
> }
>
> -void
> -control_cleanup(char *path)
> -{
> - event_del(_state.ev);
> - event_del(_state.evt);
> - unlink(path);
> -}
> -
> /* ARGSUSED */
> void
> control_accept(int listenfd, short event, void *bula)
> Index: control.h
> ===
> RCS file: /cvs/src/usr.sbin/ripd/control.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 control.h
> --- control.h 2 Aug 2016 16:05:32 - 1.5
> +++ control.h 3 Nov 2018 09:11:20 -
> @@ -39,6 +39,5 @@ int control_listen(void);
> void control_accept(int, short, void *);
> void control_dispatch_imsg(int, short, void *);
> int control_imsg_relay(struct imsg *);
> -void control_cleanup(char *);
>
> #endif /* _CONTROL_H_ */
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 rde.c
> --- rde.c 3 Sep 2016 10:28:08 - 1.21
> +++ rde.c 3 Nov 2018 07:38:41 -
> @@ -109,6 +109,9 @@ rde(struct ripd_conf *xconf, int pipe_pa
> setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
> fatal("can't drop privileges");
>
> + if (pledge("stdio", NULL) == -1)
> + fatal("pledge");
> +
> event_init();
>
> /* setup signal handler */
> Index: ripd.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 ripd.c
> --- ripd.c3 Sep 2016 10:28:08 - 1.30
> +++ ripd.c3 Nov 2018 09:14:38 -
> @@ -211,6 +211,11 @@ main(int argc, char *argv[])
> rde_pid = rde(conf, pipe_parent2rde, pipe_ripe2rde, pipe_parent2ripe);
> ripe_pid = ripe(conf, pipe_parent2ripe, pipe_ripe2rde, pipe_parent2rde);
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_init();
>
> /* setup signal handler */
> @@ -276,7 +281,6 @@ ripd_shutdown(void)
> if_del(i);
> }
>
> - control_cleanup(conf->csock);
> kr_shutdown();
>
> log_debug("waiting for children to terminate");
> Index: ripe.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 ripe.c
> --- ripe.c3 Sep 2016 10:28:08 - 1.22
> +++ ripe.c3 Nov 2018 09:07:24 -
> @@ -196,6 +196,9 @@ ripe(struct ripd_conf *xconf, int pipe_p
> iface->name);
> }
>
> + if (pledge("stdio inet mcast", NULL) == -1)
> + fatal("pledge");
> +
> evtimer_set(>report_timer, report_timer, oeconf);
> start_report_timer();
>
--
:wq Claudio
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 05:31:04PM +, Ricardo Mestre wrote:
> clearly an oversight due to looking at too many daemons at the same
> time. since the only thing ripd needs to do is unlink the socket I think
> we can remove control_cleanup, even though I'd rather do this
> introducing pledge, but for now it's a great compromise.
>
In addition to your diff this pledges the rde and ripe.
OK?
Index: control.c
===
RCS file: /cvs/src/usr.sbin/ripd/control.c,v
retrieving revision 1.25
diff -u -p -r1.25 control.c
--- control.c 17 Jan 2017 22:10:56 - 1.25
+++ control.c 3 Nov 2018 09:11:39 -
@@ -100,14 +100,6 @@ control_listen(void)
return (0);
}
-void
-control_cleanup(char *path)
-{
- event_del(_state.ev);
- event_del(_state.evt);
- unlink(path);
-}
-
/* ARGSUSED */
void
control_accept(int listenfd, short event, void *bula)
Index: control.h
===
RCS file: /cvs/src/usr.sbin/ripd/control.h,v
retrieving revision 1.5
diff -u -p -r1.5 control.h
--- control.h 2 Aug 2016 16:05:32 - 1.5
+++ control.h 3 Nov 2018 09:11:20 -
@@ -39,6 +39,5 @@ int control_listen(void);
void control_accept(int, short, void *);
void control_dispatch_imsg(int, short, void *);
intcontrol_imsg_relay(struct imsg *);
-void control_cleanup(char *);
#endif /* _CONTROL_H_ */
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
retrieving revision 1.21
diff -u -p -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 - 1.21
+++ rde.c 3 Nov 2018 07:38:41 -
@@ -109,6 +109,9 @@ rde(struct ripd_conf *xconf, int pipe_pa
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("can't drop privileges");
+ if (pledge("stdio", NULL) == -1)
+ fatal("pledge");
+
event_init();
/* setup signal handler */
Index: ripd.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
retrieving revision 1.30
diff -u -p -r1.30 ripd.c
--- ripd.c 3 Sep 2016 10:28:08 - 1.30
+++ ripd.c 3 Nov 2018 09:14:38 -
@@ -211,6 +211,11 @@ main(int argc, char *argv[])
rde_pid = rde(conf, pipe_parent2rde, pipe_ripe2rde, pipe_parent2ripe);
ripe_pid = ripe(conf, pipe_parent2ripe, pipe_ripe2rde, pipe_parent2rde);
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_init();
/* setup signal handler */
@@ -276,7 +281,6 @@ ripd_shutdown(void)
if_del(i);
}
- control_cleanup(conf->csock);
kr_shutdown();
log_debug("waiting for children to terminate");
Index: ripe.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
retrieving revision 1.22
diff -u -p -r1.22 ripe.c
--- ripe.c 3 Sep 2016 10:28:08 - 1.22
+++ ripe.c 3 Nov 2018 09:07:24 -
@@ -196,6 +196,9 @@ ripe(struct ripd_conf *xconf, int pipe_p
iface->name);
}
+ if (pledge("stdio inet mcast", NULL) == -1)
+ fatal("pledge");
+
evtimer_set(>report_timer, report_timer, oeconf);
start_report_timer();
Re: disable fs access on ripd
Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> >
> > Back in the summer the consensus was that it is fine to leave the
> > sockets lying around. Furthermore the consensus was that it is worth
> > to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> > claudio, deraadt, florian and mestre where in the loop.)
> >
> > mestre went on a rampage and removed control_cleanup and cpath pledge
> > in a bunch of daemons. I think he stopped when he couldn't find any
> > more daemons where the parent process was pledged.
> >
> > This was before the
> > unveil("/", "r");
> > unveil(control_socket, "rwc");
> > pattern was discovered which is also very powerful.
> >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> >
> > I want consistency, if we go with the unveil pattern, fine, then we
> > should restore the control_cleanup code in a bunch of daemons.
> >
> > However:
> > - the control program behaviour change had been discussed in the
> > summer and the only difference discovered then was "file not found" vs.
> > "connection refused".
> > - benno's comment about only one instance of ospfd running is not
> > effected by leaving the socket behind, it checks if the socket has a
> > listener. a dead socket is no problem.
> > - daemons/routers might crash and leave a socket behind anyway. they
> > must be able to deal with this and I'm pretty sure they all do. I just
> > checked ospfd, bgpd and rad.
>
> i agree with that. In the summer i first thought it would be better to clean
> up, but the discussion back then convinced me otherwise, and florian summed
> it up nicely.
Fine, but let's try to make them consistant?
Re: disable fs access on ripd
>From my point of view, and my sole opinion, the process running as root
from a long standing daemon, and which is not under a chroot, should not have
the hability to write and/or create files (even better if it cannot even
read). But we should reach a consensus once again, and if control_cleanup()
should be brought back to the daemons I removed it from I won't be against it.
The combination of unveil/pledge, or even just by using unveil is very
powerful and if we by all means can limit the access the most we can to the
filesystem is really great against this type of attack vector.
On 21:53 Tue 30 Oct , Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> >
> > Back in the summer the consensus was that it is fine to leave the
> > sockets lying around. Furthermore the consensus was that it is worth
> > to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> > claudio, deraadt, florian and mestre where in the loop.)
> >
> > mestre went on a rampage and removed control_cleanup and cpath pledge
> > in a bunch of daemons. I think he stopped when he couldn't find any
> > more daemons where the parent process was pledged.
> >
> > This was before the
> > unveil("/", "r");
> > unveil(control_socket, "rwc");
> > pattern was discovered which is also very powerful.
> >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> >
> > I want consistency, if we go with the unveil pattern, fine, then we
> > should restore the control_cleanup code in a bunch of daemons.
> >
> > However:
> > - the control program behaviour change had been discussed in the
> > summer and the only difference discovered then was "file not found" vs.
> > "connection refused".
> > - benno's comment about only one instance of ospfd running is not
> > effected by leaving the socket behind, it checks if the socket has a
> > listener. a dead socket is no problem.
> > - daemons/routers might crash and leave a socket behind anyway. they
> > must be able to deal with this and I'm pretty sure they all do. I just
> > checked ospfd, bgpd and rad.
>
> i agree with that. In the summer i first thought it would be better to clean
> up, but the discussion back then convinced me otherwise, and florian summed
> it up nicely.
Re: disable fs access on ripd
Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > Remi Locherer wrote:
> >
> > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > Hi,
> > > >
> > > > After all files are opened ripd(8) can have the fs access disabled just
> > > > before
> > > > each process main loop. Its 2 childs already run under chroot, but
> > > > since they
> > > > are still not pledged at least they have no way to read/write/create
> > > > files within
> > > > the chroot. No loads or reloads of the config file happen through any
> > > > signal,
> > > > nor can we do it via ripctl(8).
> > > >
> > > > I was able to run a simple daemon with the example file. Comments? OK?
> > >
> > > control_cleanup() unlinks the control socket on exit. I think you should
> > > either unveil(conf->csock, "c") or remove control_cleanup().
> >
> > I don't understand this latter comment, let me ask.
> >
> > You think it is smart to leave these sockets lying around?
> >
>
> Back in the summer the consensus was that it is fine to leave the
> sockets lying around. Furthermore the consensus was that it is worth
> to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> claudio, deraadt, florian and mestre where in the loop.)
>
> mestre went on a rampage and removed control_cleanup and cpath pledge
> in a bunch of daemons. I think he stopped when he couldn't find any
> more daemons where the parent process was pledged.
>
> This was before the
> unveil("/", "r");
> unveil(control_socket, "rwc");
> pattern was discovered which is also very powerful.
>
> > I suspect there are a few oddball programs where it makes senes, but as
> > a general rule I think it is a bad idea; as stated in other threads it
> > means control programs and restart sequences have a bunch more oddball
> > behaviours which will be inconsistant.
>
> I want consistency, if we go with the unveil pattern, fine, then we
> should restore the control_cleanup code in a bunch of daemons.
>
> However:
> - the control program behaviour change had been discussed in the
> summer and the only difference discovered then was "file not found" vs.
> "connection refused".
> - benno's comment about only one instance of ospfd running is not
> effected by leaving the socket behind, it checks if the socket has a
> listener. a dead socket is no problem.
> - daemons/routers might crash and leave a socket behind anyway. they
> must be able to deal with this and I'm pretty sure they all do. I just
> checked ospfd, bgpd and rad.
i agree with that. In the summer i first thought it would be better to clean
up, but the discussion back then convinced me otherwise, and florian summed
it up nicely.
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> Remi Locherer wrote:
>
> > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > Hi,
> > >
> > > After all files are opened ripd(8) can have the fs access disabled just
> > > before
> > > each process main loop. Its 2 childs already run under chroot, but since
> > > they
> > > are still not pledged at least they have no way to read/write/create
> > > files within
> > > the chroot. No loads or reloads of the config file happen through any
> > > signal,
> > > nor can we do it via ripctl(8).
> > >
> > > I was able to run a simple daemon with the example file. Comments? OK?
> >
> > control_cleanup() unlinks the control socket on exit. I think you should
> > either unveil(conf->csock, "c") or remove control_cleanup().
>
> I don't understand this latter comment, let me ask.
>
> You think it is smart to leave these sockets lying around?
>
Back in the summer the consensus was that it is fine to leave the
sockets lying around. Furthermore the consensus was that it is worth
to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
claudio, deraadt, florian and mestre where in the loop.)
mestre went on a rampage and removed control_cleanup and cpath pledge
in a bunch of daemons. I think he stopped when he couldn't find any
more daemons where the parent process was pledged.
This was before the
unveil("/", "r");
unveil(control_socket, "rwc");
pattern was discovered which is also very powerful.
> I suspect there are a few oddball programs where it makes senes, but as
> a general rule I think it is a bad idea; as stated in other threads it
> means control programs and restart sequences have a bunch more oddball
> behaviours which will be inconsistant.
I want consistency, if we go with the unveil pattern, fine, then we
should restore the control_cleanup code in a bunch of daemons.
However:
- the control program behaviour change had been discussed in the
summer and the only difference discovered then was "file not found" vs.
"connection refused".
- benno's comment about only one instance of ospfd running is not
effected by leaving the socket behind, it checks if the socket has a
listener. a dead socket is no problem.
- daemons/routers might crash and leave a socket behind anyway. they
must be able to deal with this and I'm pretty sure they all do. I just
checked ospfd, bgpd and rad.
--
I'm not entirely sure you are real.
Re: disable fs access on ripd
clearly an oversight due to looking at too many daemons at the same
time. since the only thing ripd needs to do is unlink the socket I think
we can remove control_cleanup, even though I'd rather do this
introducing pledge, but for now it's a great compromise.
OK?
Index: control.c
===
RCS file: /cvs/src/usr.sbin/ripd/control.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 control.c
--- control.c 17 Jan 2017 22:10:56 - 1.25
+++ control.c 30 Oct 2018 17:24:44 -
@@ -100,14 +100,6 @@ control_listen(void)
return (0);
}
-void
-control_cleanup(char *path)
-{
- event_del(_state.ev);
- event_del(_state.evt);
- unlink(path);
-}
-
/* ARGSUSED */
void
control_accept(int listenfd, short event, void *bula)
Index: control.h
===
RCS file: /cvs/src/usr.sbin/ripd/control.h,v
retrieving revision 1.5
diff -u -p -u -r1.5 control.h
--- control.h 2 Aug 2016 16:05:32 - 1.5
+++ control.h 30 Oct 2018 17:24:44 -
@@ -39,6 +39,5 @@ int control_listen(void);
void control_accept(int, short, void *);
void control_dispatch_imsg(int, short, void *);
intcontrol_imsg_relay(struct imsg *);
-void control_cleanup(char *);
#endif /* _CONTROL_H_ */
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 - 1.21
+++ rde.c 30 Oct 2018 17:24:44 -
@@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
free(r);
}
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
rde_shutdown();
Index: ripd.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ripd.c
--- ripd.c 3 Sep 2016 10:28:08 - 1.30
+++ ripd.c 30 Oct 2018 17:24:45 -
@@ -251,6 +251,11 @@ main(int argc, char *argv[])
conf->rdomain) == -1)
fatalx("kr_init failed");
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripd_shutdown();
@@ -276,7 +281,6 @@ ripd_shutdown(void)
if_del(i);
}
- control_cleanup(conf->csock);
kr_shutdown();
log_debug("waiting for children to terminate");
Index: ripe.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 ripe.c
--- ripe.c 3 Sep 2016 10:28:08 - 1.22
+++ ripe.c 30 Oct 2018 17:24:45 -
@@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripe_shutdown();
On 11:15 Tue 30 Oct , Theo de Raadt wrote:
> Remi Locherer wrote:
>
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> > >
> >
> > I prefer if sockets get removed on exit. But I was not sure if this is
> > just my personal taste.
>
> I didn't speak about taste, I spoke about behaviour relative to the
> control program. Why do you propose potentially changing behaviour
> without checking first?
>
Re: disable fs access on ripd
Remi Locherer wrote: > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote: > > Remi Locherer wrote: > > > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > > > Hi, > > > > > > > > After all files are opened ripd(8) can have the fs access disabled just > > > > before > > > > each process main loop. Its 2 childs already run under chroot, but > > > > since they > > > > are still not pledged at least they have no way to read/write/create > > > > files within > > > > the chroot. No loads or reloads of the config file happen through any > > > > signal, > > > > nor can we do it via ripctl(8). > > > > > > > > I was able to run a simple daemon with the example file. Comments? OK? > > > > > > control_cleanup() unlinks the control socket on exit. I think you should > > > either unveil(conf->csock, "c") or remove control_cleanup(). > > > > I don't understand this latter comment, let me ask. > > > > You think it is smart to leave these sockets lying around? > > > > I suspect there are a few oddball programs where it makes senes, but as > > a general rule I think it is a bad idea; as stated in other threads it > > means control programs and restart sequences have a bunch more oddball > > behaviours which will be inconsistant. > > > > I prefer if sockets get removed on exit. But I was not sure if this is > just my personal taste. I didn't speak about taste, I spoke about behaviour relative to the control program. Why do you propose potentially changing behaviour without checking first?
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote: > Remi Locherer wrote: > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > > Hi, > > > > > > After all files are opened ripd(8) can have the fs access disabled just > > > before > > > each process main loop. Its 2 childs already run under chroot, but since > > > they > > > are still not pledged at least they have no way to read/write/create > > > files within > > > the chroot. No loads or reloads of the config file happen through any > > > signal, > > > nor can we do it via ripctl(8). > > > > > > I was able to run a simple daemon with the example file. Comments? OK? > > > > control_cleanup() unlinks the control socket on exit. I think you should > > either unveil(conf->csock, "c") or remove control_cleanup(). > > I don't understand this latter comment, let me ask. > > You think it is smart to leave these sockets lying around? > > I suspect there are a few oddball programs where it makes senes, but as > a general rule I think it is a bad idea; as stated in other threads it > means control programs and restart sequences have a bunch more oddball > behaviours which will be inconsistant. > I prefer if sockets get removed on exit. But I was not sure if this is just my personal taste.
Re: disable fs access on ripd
Remi Locherer wrote: > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > Hi, > > > > After all files are opened ripd(8) can have the fs access disabled just > > before > > each process main loop. Its 2 childs already run under chroot, but since > > they > > are still not pledged at least they have no way to read/write/create files > > within > > the chroot. No loads or reloads of the config file happen through any > > signal, > > nor can we do it via ripctl(8). > > > > I was able to run a simple daemon with the example file. Comments? OK? > > control_cleanup() unlinks the control socket on exit. I think you should > either unveil(conf->csock, "c") or remove control_cleanup(). I don't understand this latter comment, let me ask. You think it is smart to leave these sockets lying around? I suspect there are a few oddball programs where it makes senes, but as a general rule I think it is a bad idea; as stated in other threads it means control programs and restart sequences have a bunch more oddball behaviours which will be inconsistant.
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> Hi,
>
> After all files are opened ripd(8) can have the fs access disabled just before
> each process main loop. Its 2 childs already run under chroot, but since they
> are still not pledged at least they have no way to read/write/create files
> within
> the chroot. No loads or reloads of the config file happen through any signal,
> nor can we do it via ripctl(8).
>
> I was able to run a simple daemon with the example file. Comments? OK?
control_cleanup() unlinks the control socket on exit. I think you should
either unveil(conf->csock, "c") or remove control_cleanup().
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 rde.c
> --- rde.c 3 Sep 2016 10:28:08 - 1.21
> +++ rde.c 30 Oct 2018 15:09:44 -
> @@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
> free(r);
> }
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> rde_shutdown();
> Index: ripd.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ripd.c
> --- ripd.c3 Sep 2016 10:28:08 - 1.30
> +++ ripd.c30 Oct 2018 15:09:44 -
> @@ -251,6 +251,11 @@ main(int argc, char *argv[])
> conf->rdomain) == -1)
> fatalx("kr_init failed");
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripd_shutdown();
> Index: ripe.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 ripe.c
> --- ripe.c3 Sep 2016 10:28:08 - 1.22
> +++ ripe.c30 Oct 2018 15:09:44 -
> @@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
>
> ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripe_shutdown();
>
Re: disable fs access on ripd
ok benno@
Ricardo Mestre(ser...@helheim.mooo.com) on 2018.10.30 15:20:35 +:
> Hi,
>
> After all files are opened ripd(8) can have the fs access disabled just before
> each process main loop. Its 2 childs already run under chroot, but since they
> are still not pledged at least they have no way to read/write/create files
> within
> the chroot. No loads or reloads of the config file happen through any signal,
> nor can we do it via ripctl(8).
>
> I was able to run a simple daemon with the example file. Comments? OK?
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 rde.c
> --- rde.c 3 Sep 2016 10:28:08 - 1.21
> +++ rde.c 30 Oct 2018 15:09:44 -
> @@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
> free(r);
> }
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> rde_shutdown();
> Index: ripd.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ripd.c
> --- ripd.c3 Sep 2016 10:28:08 - 1.30
> +++ ripd.c30 Oct 2018 15:09:44 -
> @@ -251,6 +251,11 @@ main(int argc, char *argv[])
> conf->rdomain) == -1)
> fatalx("kr_init failed");
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripd_shutdown();
> Index: ripe.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 ripe.c
> --- ripe.c3 Sep 2016 10:28:08 - 1.22
> +++ ripe.c30 Oct 2018 15:09:44 -
> @@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
>
> ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripe_shutdown();
>
disable fs access on ripd
Hi,
After all files are opened ripd(8) can have the fs access disabled just before
each process main loop. Its 2 childs already run under chroot, but since they
are still not pledged at least they have no way to read/write/create files
within
the chroot. No loads or reloads of the config file happen through any signal,
nor can we do it via ripctl(8).
I was able to run a simple daemon with the example file. Comments? OK?
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 - 1.21
+++ rde.c 30 Oct 2018 15:09:44 -
@@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
free(r);
}
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
rde_shutdown();
Index: ripd.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ripd.c
--- ripd.c 3 Sep 2016 10:28:08 - 1.30
+++ ripd.c 30 Oct 2018 15:09:44 -
@@ -251,6 +251,11 @@ main(int argc, char *argv[])
conf->rdomain) == -1)
fatalx("kr_init failed");
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripd_shutdown();
Index: ripe.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 ripe.c
--- ripe.c 3 Sep 2016 10:28:08 - 1.22
+++ ripe.c 30 Oct 2018 15:09:44 -
@@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripe_shutdown();
