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 -0000 1.25
+++ control.c 30 Oct 2018 17:24:44 -0000
@@ -100,14 +100,6 @@ control_listen(void)
return (0);
}
-void
-control_cleanup(char *path)
-{
- event_del(&control_state.ev);
- event_del(&control_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 -0000 1.5
+++ control.h 30 Oct 2018 17:24:44 -0000
@@ -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 -u -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 -0000 1.21
+++ rde.c 30 Oct 2018 17:24:44 -0000
@@ -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 -0000 1.30
+++ ripd.c 30 Oct 2018 17:24:45 -0000
@@ -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 -0000 1.22
+++ ripe.c 30 Oct 2018 17:24:45 -0000
@@ -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 <[email protected]> wrote:
>
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer <[email protected]> wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +0000, 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?
>
