On Mon, Oct 07, 2019 at 11:48:12AM +0100, Stuart Henderson wrote:
> On 2019/10/07 12:30, Florian Obser wrote:
> > On Mon, Oct 07, 2019 at 11:16:22AM +0100, Stuart Henderson wrote:
> > > On 2019/10/07 09:53, Theo Buehler wrote:
> > > > $ doas /usr/sbin/unbound-anchor -v
> > > > /var/unbound/db/root.key has content
> > > > [1570433629] libunbound[28321:0] fatal error: could not open autotrust 
> > > > file for writing, /var/unbound/db/root.key.28321-0-1966ee948e00: No 
> > > > such file or directory
> > > > 
> > > > The problem is the following change that came with the update to 1.9.3:
> > > > 
> > > >         - Add hex print of trust anchor pointer to trust anchor file 
> > > > temp
> > > >           name to make it unique, for libunbound created multiple 
> > > > contexts.
> > > > 
> > > > See 
> > > > https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/unbound/validator/autotrust.c.diff?r1=1.10&r2=1.11
> > > > 
> > > > Thus, the unveil code in smallapp/unbound-anchor.c needs some
> > > > adjustment.
> > > > 
> > > >         if (asprintf(&root_anchor_tempfile, "%s.%d-0", root_anchor_file,
> > > >             getpid()) == -1) {
> > > >                 if(verb) printf("out of memory\n");
> > > >                 exit(0);
> > > >         }
> > > > 
> > > >         if (unveil(root_anchor_file, "rwc") == -1)
> > > >                 err(1, "unveil");
> > > >         if (unveil(root_anchor_tempfile, "rwc") == -1)
> > > >                 err(1, "unveil");
> > > > 
> > > > The problem is that tp used for tempfile generation is not yet known at
> > > > that point. Not sure how best to deal with this.
> > > 
> > > Just unveil the directory?
> > 
> > yes, however dirname(3) has this:
> > 
> > CAVEATS
> >      Other vendor implementations of dirname() may modify the contents of 
> > the
> >      string passed to dirname(); this should be taken into account when
> >      writing code which calls this function if portability is desired.
> > 
> > I think we want a strdup here.
> 
> done, though it is openbsd-only code (and added some frees).

True, but there is still the small risk of copying it around.  For
example, I only became aware of this because acme-client did it wrong
and someone pointed it out to me.

OK florian

> 
> Index: smallapp/unbound-anchor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/unbound/smallapp/unbound-anchor.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 unbound-anchor.c
> --- smallapp/unbound-anchor.c 10 Jan 2019 12:13:44 -0000      1.12
> +++ smallapp/unbound-anchor.c 7 Oct 2019 10:47:14 -0000
> @@ -116,6 +116,7 @@
>   */
>  
>  #include <err.h>
> +#include <libgen.h>
>  #include <unistd.h>
>  
>  #include "config.h"
> @@ -2284,7 +2285,8 @@ int main(int argc, char* argv[])
>       const char* res_conf = NULL;
>       const char* root_hints = NULL;
>       const char* debugconf = NULL;
> -     char* root_anchor_tempfile;
> +     char* root_anchor_temppath;
> +     char* s;
>       int dolist=0, ip4only=0, ip6only=0, force=0, port = HTTPS_PORT;
>       int res_conf_fallback = 0;
>       /* parse the options */
> @@ -2370,16 +2372,16 @@ int main(int argc, char* argv[])
>  
>       if(dolist) do_list_builtin();
>  
> -     if (asprintf(&root_anchor_tempfile, "%s.%d-0", root_anchor_file,
> -         getpid()) == -1) {
> +     s = strdup(root_anchor_file);
> +     if (s == NULL ||
> +         asprintf(&root_anchor_temppath, "%s", dirname(s)) == -1) {
>               if(verb) printf("out of memory\n");
>               exit(0);
>       }
> -
> -     if (unveil(root_anchor_file, "rwc") == -1)
> -             err(1, "unveil");
> -     if (unveil(root_anchor_tempfile, "rwc") == -1)
> +     if (unveil(root_anchor_temppath, "rwc") == -1)
>               err(1, "unveil");
> +     free(root_anchor_temppath);
> +     free(s);
>       if (unveil(root_cert_file, "r") == -1)
>               err(1, "unveil");
>       if (res_conf != NULL && unveil(res_conf, "r") == -1)
> 

-- 
I'm not entirely sure you are real.

Reply via email to