Ricardo Mestre([email protected]) on 2018.10.28 17:26:24 +0000:
> Correct, and I'd go even further by not unveiling the socket at all. A few
> weeks ago I removed the logic of unlinking the socket when the program stops,
> for a few daemons, but left untouched the ones that don't have the main
> process
> pledged since it wouldn't make much difference.
>
> If we want to go this way then I think we also should remove the logic for
> these daemons and adding unveil just like the below.
>
> Comments? OK?
in ospfd, ospf6d (and hopefully soon bgpd) we make sure the daemon does not
run twice by checking the socket.
Since it really is bad if you run two ospfd, please keep this.
/Benno
>
> Index: control.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/control.c,v
> retrieving revision 1.45
> diff -u -p -u -r1.45 control.c
> --- control.c 29 Aug 2018 08:43:16 -0000 1.45
> +++ control.c 28 Oct 2018 17:24:46 -0000
> @@ -124,16 +124,6 @@ control_listen(void)
> return (0);
> }
>
> -void
> -control_cleanup(char *path)
> -{
> - if (path == NULL)
> - return;
> - 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/ospfd/control.h,v
> retrieving revision 1.7
> diff -u -p -u -r1.7 control.h
> --- control.h 29 Aug 2018 08:43:16 -0000 1.7
> +++ control.h 28 Oct 2018 17:24:46 -0000
> @@ -40,6 +40,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: ospfd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.100
> diff -u -p -u -r1.100 ospfd.c
> --- ospfd.c 29 Aug 2018 08:43:17 -0000 1.100
> +++ ospfd.c 28 Oct 2018 17:24:46 -0000
> @@ -288,6 +288,11 @@ main(int argc, char *argv[])
> area_del(a);
> }
>
> + if (unveil("/", "r") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ospfd_shutdown();
> @@ -308,7 +313,6 @@ ospfd_shutdown(void)
> msgbuf_clear(&iev_rde->ibuf.w);
> close(iev_rde->ibuf.fd);
>
> - control_cleanup(ospfd_conf->csock);
> while ((r = SIMPLEQ_FIRST(&ospfd_conf->redist_list)) != NULL) {
> SIMPLEQ_REMOVE_HEAD(&ospfd_conf->redist_list, entry);
> free(r);
>
> On 15:58 Sun 28 Oct , Florian Obser wrote:
> > Sorry, I'm on a phone. The diff context looks like the control FD is
> > already open at this point. Does ospfd later re-open it?
> >
> > On October 27, 2018 11:25:58 PM GMT+02:00, Remi Locherer
> > <[email protected]> wrote:
> > >On Fri, Oct 26, 2018 at 10:19:01AM -0600, Theo de Raadt wrote:
> > >> Remi Locherer <[email protected]> wrote:
> > >>
> > >> > On Fri, Oct 26, 2018 at 06:01:40PM +0200, Florian Obser wrote:
> > >> > > This breaks usage of the "include" keyword. Something that all
> > >the parse.y daemons support.
> > >> > >
> > >> >
> > >> > Oh, of course!
> > >> >
> > >> > I guess this is similar to unveil files based on a list of command
> > >line args.
> > >>
> > >> correct.
> > >>
> > >> Now that unveil is used in the tree, there are 3 types of programs
> > >>
> > >> 1) they use unveil
> > >> 2) they use pledge, heading close towards "stdio" without a "*path"
> > >> 3) they access arbitrary files based upon argv
> > >>
> > >> this is (3), except not argv, it nested inside the config file
> > >>
> > >> Well there are maybe 20 programs beyond that which aren't converted
> > >yet,
> > >> but things are looking pretty good.
> > >>
> > >
> > >Since ospfd is not suppose to write or execute files we could make the
> > >file system read only (with the exception of the control socket).
> > >
> > >(Once we can add pledge to ospfd's parent proc this will probably not
> > >make
> > >sense anymore.)
> > >
> > >
> > >
> > >cvs diff: Diffing .
> > >Index: ospfd.c
> > >===================================================================
> > >RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> > >retrieving revision 1.100
> > >diff -u -p -r1.100 ospfd.c
> > >--- ospfd.c 29 Aug 2018 08:43:17 -0000 1.100
> > >+++ ospfd.c 27 Oct 2018 07:28:58 -0000
> > >@@ -278,6 +278,13 @@ main(int argc, char *argv[])
> > > fatalx("control socket setup failed");
> > > main_imsg_compose_ospfe_fd(IMSG_CONTROLFD, 0, control_fd);
> > >
> > >+ if (unveil("/", "r") == -1)
> > >+ fatal("unveil");
> > >+ if (unveil(ospfd_conf->csock, "c") == -1)
> > >+ fatal("unveil");
> > >+ if (unveil(NULL, NULL) == -1)
> > >+ fatal("unveil");
> > >+
> > > if (kr_init(!(ospfd_conf->flags & OSPFD_FLAG_NO_FIB_UPDATE),
> > > ospfd_conf->rdomain, ospfd_conf->redist_label_or_prefix) == -1)
> > > fatalx("kr_init failed");
>
