Re: patch 1/2 debuginfod client

["Frank Ch. Eigler" <fche@redhat.com>]

Hi -

> It also seems that how you did it on the branch:

Yeah, maybe a different automake version made it work after my earlier
failing tests.  (dead horse PS.  It remains my opinion that we should
commit auto* generated files into git.)

> > Possible, but since these bits are just getting transcribed straight
> > to a debuginfod URL, an insane unclean hex will just get rejected at
> > that time.  There is no lost-error case there.
> 
> But since the user won't see the URL generated they might not notice
> what is really going on. They will just see that something wasn't
> found, won't they?

Yes, so the only benefit would be the generation of a different error
message for impossible buildids.


> > Yeah ... if a user were to set DEBUGINFOD_CACHE_DIR to $HOME, she
> > will
> > get a good thorough cleaning, or a root user were to set it to /.
> > PEBCAK IMHO; don't see easy ways of protecting themselves against it.
> 
> It might be unlikely the user accidentally set the environment
> variables to something odd, 

> but they might be tricked into running it on a debug file that was
> doctored. Which might produce lookups for odd source files. It might
> even just be a buggy compiler that created a few ../.. too many. It
> would be bad if that would cause havoc.

Source file names do not survive into the client side cache - you 
already found the "#" escaping bits.


> > > > +/* NB: these are thread-unsafe. */
> > > > +__attribute__((constructor)) attribute_hidden void libdebuginfod_ctor(void)
> > > > +{
> > > > +  curl_global_init(CURL_GLOBAL_DEFAULT);
> > > > +}
> > > 
> > > How does this interact with a program that uses libcurl itself and also
> > > links with libdebuginfod?
> > 
> > The call is harmless if repeated.  It is described merely as
> > multi-thread-unsafe.
> 
> It also says:
> 
>    This function is not thread safe.
>    You must not call it when any other thread in the program (i.e. a  thread sharing the same memory) is running. This doesn't just mean no other thread that is using libcurl.  Because curl_global_init calls functions of other libraries that are similarly thread unsafe,
>    it could conflict with any other thread that uses these other libraries.
> 
> That doesn't make me very happy.
> It looks like using libcurl from another library is not really very
> safe if the program or another library it links against are also
> libcurl users.
> 
> Do you know how other libraries that use libcurl deal with this?

I looked over some other libcurl users in fedora.  Some don't worry
about the issue at all, relying on implicit initialization, which is
only safe if single-threaded.  Others (libvirtd) do an explicitly
invoked initialization function, which is also only safe if invoked
from a single-threaded context.

I think actually our way here, running the init function from the
shared library constructor, is about the best possible.  As long as
the ld.so process is done as normal, it should be fine.  Programs that
use the elfutils trick of manual dlopen'ing libdebuginfod.so should do
so only if they are single-threaded.


> > These are the webapi URL strings.  The cache file names are not the
> > same - those are specifically scrubbed with the '#' characters.
> 
> Aha. That is done by this code:
> 
> >       /* copy the filename to suffix, s,/,#,g */
> >       for (q=0; q<sizeof(suffix)-1; q++)
> >         {
> >           if (filename[q] == '\0') break;
> >           if (filename[q] == '/' || filename[q] == '.') suffix[q] = '#';
> >           else suffix[q] = filename[q];
> >         }
> 
> Why do you also replace '.' with '#'?
> That seems unnecessary.

It was to make sure ../ names from dwarf source files definitely don't
escape the cache dir.  But yeah if we nuke "/", then "." is alone
safe, and actually that would be good.


> What about files that already contain '#' chars?
> Wouldn't something like /foo/bar#/baz clash with /foo/bar/#baz?
> Or do you just think that is just completely unlikely to ever occur?

Considered it unlikely, but will tweak the code to escape these two
without clashing.


> > > I assume libcurl handles tls certificates for https? Does that need any
> > > mention here?
> > 
> > Dunno if it's worth discussing ... the client side of https usually
> > does not deal with configurable certificates.
> 
> But the client side might or might not verify the server side ssl
> certificate. Does it do that? And if so, which trusted roots does it
> use? And can that be disabled or changed?

Whatever the compiled-in defaults are, /etc paths etc.  IMHO we
shouldn't worry about them.


> > I suppose that wouldn't make any sense.  Anything will take a nonzero
> > amount of time. :-) Maybe it could be a floating point number or
> > something; worth it?
> 
> I was more thinking zero == infinity (no timeout).

An unset environment variable should do that.

- FChE