Specifying CA certificates for libdebuginfod

2021-05-28 Thread Magne Hov via Elfutils-devel
Hi,

I am posting here to continue a discussion from the #elfutils
libera.chat channel about whether libdebuginfod might benefit from
having a method of specifying a certificate bundle for libcurl.

Normally one would rely on the system's OpenSSL having been configured
with up-to-date certificates. However in my use-case I can't depend on
up-to-date certificates being installed on the system that I work with,
so I package certificates together with my application (which contains
libdebuginfod and its dependencies as a portable package).

Other components that my application uses already have ways of
specifying a certificate bundle. The curl tool supports custom
certificates with the CURL_CA_BUNDLE environment variable, but with
libcurl one must specify a custom certificate bundle with the
CURLOPT_CAINFO option via the API. I propose a new environment variable
DEBUGINFOD_CA_BUNDLE or similar which can be used to pass to libcurl.
Please see the attached patch below.

There is also an option of recognising CURL_CA_BUNDLE as that
environment variable is already established by the curl tool, but it
could also be good to keep the name separate to libdebuginfod.

I think having the option of specifying certificates could also be
helpful for other situations such as specifying a self-signed
certificate to use with servers under test.

Kind regards,
Magne

>From 78363eed66c8098961c84980d485f87c8b43f25c Mon Sep 17 00:00:00 2001
From: Magne Hov 
Date: Tue, 11 May 2021 16:24:51 +0100
Subject: [PATCH] libdebuginfod: specify client CA bundle with
 DEBUGINFOD_CA_BUNDLE

---
 debuginfod/debuginfod-client.c | 7 +++
 debuginfod/debuginfod.h.in | 1 +
 2 files changed, 8 insertions(+)

diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index de26af5b..b9165733 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -827,6 +827,13 @@ debuginfod_query_server (debuginfod_client *c,
   curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, "");
   curl_easy_setopt(data[i].handle, CURLOPT_HTTPHEADER, c->headers);
 
+  /* Pass SSL certificate to libcurl. */
+  const char *certfile = getenv(DEBUGINFOD_CA_BUNDLE);
+  if (certfile != NULL && strlen (certfile) > 0)
+  {
+  curl_easy_setopt(data[i].handle, CURLOPT_CAINFO, certfile);
+  }
+
   curl_multi_add_handle(curlm, data[i].handle);
   server_url = strtok_r(NULL, url_delim, &strtok_saveptr);
 }
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 559ea947..3ed32f19 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -35,6 +35,7 @@
 #define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"
 #define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS"
 #define DEBUGINFOD_VERBOSE_ENV_VAR "DEBUGINFOD_VERBOSE"
+#define DEBUGINFOD_CA_BUNDLE "DEBUGINFOD_CA_BUNDLE"
 
 /* The libdebuginfod soname.  */
 #define DEBUGINFOD_SONAME "@LIBDEBUGINFOD_SONAME@"
-- 
2.25.1



Re: Specifying CA certificates for libdebuginfod

2021-07-12 Thread Magne Hov via Elfutils-devel
Thank you for your reply, and apologies about getting back to you so late.

On Fri, May 28 2021, Frank Ch. Eigler wrote:

> Hi -
>
> On Fri, May 28, 2021 at 06:36:17PM +0100, Magne Hov via Elfutils-devel wrote:
>> [...]
>> Other components that my application uses already have ways of
>> specifying a certificate bundle. [...]
>> There is also an option of recognising CURL_CA_BUNDLE as that
>> environment variable is already established by the curl tool, but it
>> could also be good to keep the name separate to libdebuginfod.
>
> Sure; I have no strong opinion on this.  ISTM of no particular
> advantage to invent a debuginfod-oriented env var for this,
> as opposed to reusing "CURL_CA_BUNDLE" (but not "SSL_CERT_DIR"
> or "SSL_CERT_FILE", right?).
  
The exact name of the environment variable is not important to me. I
have no problem reusing "CURL_CA_BUNDLE".

I wanted to share my patch in case other people found it useful, but for
the time being I'm content with patching my own builds. I'm also not
sure what the best approach is for deciding on a name for this
environment variable. It might be best to leave this issue until the
feature is potentially wanted by more users?

>
> - FChE


Re: [patch] PR28339: debuginfod groom/scan concurrency fix

2021-11-11 Thread Magne Hov via Elfutils-devel
Hi Frank,

On Tue, Sep 14 2021, Frank Ch. Eigler via Elfutils-devel wrote:

> commit ce695bedf073127883bbbaab528dd1f2b0e955f1 (HEAD -> master)
> Author: Frank Ch. Eigler 
> Date:   Tue Sep 14 08:15:23 2021 -0400
>
> PR28339: debuginfod: fix groom/scan race condition on just-emptied queue
> 
> debuginfod's scan and groom operations (thread_main_scanner,
> thread_main_fts_source_paths) are intended to be mutually exclusive,
> as a substitute for more complicated sql transaction batching.  (This
> is because scanning / grooming involves inserting or deleting data
> from multiple related tables.)
> 
> The workq class that governs this in debuginfod.cxx has a problem: if
> the workq just becomes empty, its sole entry pulled by a scanner
> thread in response to a wait_front(), an 'idler' groomer thread is
> ALSO permitted to run, because there is no indication as to when the
> scanner thread operation finishes, only when it starts.
> 
> Extending the workq with a counter ("fronters") to track any active
> scanning activity (even if the workq is empty) lets us block idlers
> groomers a little longer.
> 
> Signed-off-by: Frank Ch. Eigler 

Thanks a lot for this fix. We've had an issue in automated testing where
`nuke orphan buildids` is executed after `rpm-buildid-intern` has run
but *before* `rpm-de-insert` has run. The result is missing BUILDIDS for
the archive, and rescans of the archive fail to re-add them because the
archive is registered as already having been scanned.

I think this fix will resovle this.