Re: patch 1/2 debuginfod client

[Mark Wielaard <mark at klomp dot org>]

Hi,

On Sat, 2019-11-16 at 13:52 -0500, Frank Ch. Eigler wrote:
> 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.

But if there are multiple server URLs it might not be clear which/where
the error came from. Since all this is done through a very simple web
api I think it is useful for the user to get informed about what the
actual request URL was that failed. Then they could debug what exactly
went wrong. Maybe my use case is not a common one. But I assume that
people who use debuginfod-find directly/by hand are doing it already
because they are interested what is really going on.

If we go with the client connection context idea for the API we could
add an extra function that would tell you the last URL tried maybe?

> > > 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.

Yes, you are right, this works on a different level than I assumed.
At the client request level it seems fine.

I still am concerned about the indexing level though. There a buggy
../.. in some debug file line table might cause the indexer to include
files which aren't really sources at all.

> > 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.

But they cannot really control that... Since they might not know (and
really shouldn't care) that libdw lazily tries to dlopen
libdebuginfod.so which then pulls in libcurl and calls that global init
function...

Could we do try to do the dlopen and global curl initialization from
libdw _init, or a ctor, to make sure it is done as early as possible? 

> > > > 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 think it is fine to just mention in the docs that resolving and
authenticating https URLs is done according to whatever the defaults
are. But I think it should be documented what those are.

> > > 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.

Are you sure? If DEBUGINFOD_TIMEOUT isn't set, then it seems it
defaults to 5 seconds:

/* Timeout for debuginfods, in seconds.
   This env var must be set for debuginfod-client to run.  */
static const char *server_timeout_envvar = DEBUGINFOD_TIMEOUT_ENV_VAR;
static int server_timeout = 5;

[...]

  if (getenv(server_timeout_envvar))
    server_timeout = atoi (getenv(server_timeout_envvar));

Cheers,

Mark