rfc/patch: user-agent distro-description for debuginfod http traffic

rfc/patch: user-agent distro-description for debuginfod http traffic

[Frank Ch. Eigler]

Hi -

commit 53aac40e9577168b886dfeecb77e5a72d2666fc9 (HEAD -> fche/debuginfod-useragent, origin/fche/debuginfod-useragent)
Author: Frank Ch. Eigler <fche@redhat.com>
Date:   Mon Jan 6 04:29:21 2020 -0500

    debuginfod: pass a distro-summary User-Agent request header
    
    It may be useful for a debuginfod server operator to know what kinds
    of clients make webapi requests.  This is mainly as a
    telemetry/diagnostic (though the data cannot be really trusted).  It
    may also be useful to automate downloading of distro packages to a
    debuginfod server in the case of an unknown hex buildid.  doc/testing
    not affected as these are diagnostics.

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 1582eba5bc0e..bed3ad7814ff 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,10 @@
+2020-01-06  Frank Ch. Eigler  <fche@redhat.com>
+
+	* Makefile.am: Create a build-time header with lsb/uname info.
+	* debuginfod-client.c: Pass above as additional User-Agent request header.
+	* debuginfod.cxx (conninfo): Print this and X-Forwarded-For, as per
+	apache httpd reverse-proxy chain.
+
 2019-12-22  Frank Ch. Eigler  <fche@redhat.com>
 
 	* debuginfod.cxx (*_rpm_*): Rename to *_archive_* throughout.
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 52ead30aebf8..20a70545755a 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -67,6 +67,20 @@ debuginfod_find_LDADD = $(libeu) $(libdebuginfod)
 noinst_LIBRARIES = libdebuginfod.a
 noinst_LIBRARIES += libdebuginfod_pic.a
 
+debuginfod-client-useragent.h:
+	if type uname 2>/dev/null; then \
+		echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 "'`uname -sm | sed -e 's, ,/,g'`'"' > $@; \
+	else \
+		echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 ""' > $@; \
+	fi
+	if type lsb_release 2>/dev/null; then \
+		echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 "'`lsb_release -sir | sed -e 's, ,/,g'`'"' >> $@; \
+	else \
+		echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 ""' > $@; \
+	fi
+	echo '#define DEBUGINFOD_CLIENT_USERAGENT DEBUGINFOD_CLIENT_USERAGENT_1 "," DEBUGINFOD_CLIENT_USERAGENT_2' >> $@
+
+libdebuginfod: debuginfod-client-useragent.h
 libdebuginfod_a_SOURCES = debuginfod-client.c
 libdebuginfod_pic_a_SOURCES = debuginfod-client.c
 am_libdebuginfod_pic_a_OBJECTS = $(libdebuginfod_a_SOURCES:.c=.os)
@@ -98,7 +112,7 @@ uninstall: uninstall-am
 
 EXTRA_DIST = libdebuginfod.map
 MOSTLYCLEANFILES = $(am_libdebuginfod_pic_a_OBJECTS) libdebuginfod.so.$(VERSION)
-CLEANFILES += $(am_libdebuginfod_pic_a_OBJECTS) libdebuginfod.so
+CLEANFILES += $(am_libdebuginfod_pic_a_OBJECTS) libdebuginfod.so debuginfod-client-useragent.h
 
 # automake std-options override: arrange to pass LD_LIBRARY_PATH
 installcheck-binPROGRAMS: $(bin_PROGRAMS)
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 9a4a0e0fc421..3c3610023e46 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -40,6 +40,7 @@
 
 #include "config.h"
 #include "debuginfod.h"
+#include "debuginfod-client-useragent.h"
 #include "system.h"
 #include <assert.h>
 #include <dirent.h>
@@ -514,7 +515,7 @@ debuginfod_query_server (debuginfod_client *c,
       curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1);
       curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1);
       curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, "");
-      curl_easy_setopt(data[i].handle, CURLOPT_USERAGENT, (void*) PACKAGE_STRING);
+      curl_easy_setopt(data[i].handle, CURLOPT_USERAGENT, (void*) PACKAGE_NAME "/" PACKAGE_VERSION "," DEBUGINFOD_CLIENT_USERAGENT);
 
       curl_multi_add_handle(curlm, data[i].handle);
       server_url = strtok_r(NULL, url_delim, &strtok_saveptr);
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 8bd3a3dba7a0..fc7c373c93d1 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -1,6 +1,6 @@
 /* Command-line frontend for retrieving ELF / DWARF / source files
    from the debuginfod.
-   Copyright (C) 2019 Red Hat, Inc.
+   Copyright (C) 2019-2020 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 70cb95fecd65..2c1a77876677 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1,5 +1,5 @@
 /* Debuginfo-over-http server.
-   Copyright (C) 2019 Red Hat, Inc.
+   Copyright (C) 2019-2020 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -751,7 +751,12 @@ conninfo (struct MHD_Connection * conn)
     hostname[0] = servname[0] = '\0';
   }
 
-  return string(hostname) + string(":") + string(servname);
+  // extract headers relevant to administration
+  const char* user_agent = MHD_lookup_connection_value (conn, MHD_HEADER_KIND, "User-Agent") ?: "";
+  const char* x_forwarded_for = MHD_lookup_connection_value (conn, MHD_HEADER_KIND, "X-Forwarded-For") ?: "";
+  // NB: these are untrustworthy, beware if machine-processing log files
+
+  return string(hostname) + string(":") + string(servname) + string(" UA:") + string(user_agent) + string(" XFF:") + string(x_forwarded_for);
 }
 
 

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Florian Weimer]

* Frank Ch. Eigler:

> -  return string(hostname) + string(":") + string(servname);
> +  // extract headers relevant to administration
> +  const char* user_agent = MHD_lookup_connection_value (conn, MHD_HEADER_KIND, "User-Agent") ?: "";
> +  const char* x_forwarded_for = MHD_lookup_connection_value (conn, MHD_HEADER_KIND, "X-Forwarded-For") ?: "";
> +  // NB: these are untrustworthy, beware if machine-processing log files
> +
> +  return string(hostname) + string(":") + string(servname) + string(" UA:") + string(user_agent) + string(" XFF:") + string(x_forwarded_for);
>  }
>  
>  
Should this add quoting to make the field boundaries unforgeable?

Thanks,
Florian

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Frank Ch. Eigler]

Hi -

> Should this add quoting to make the field boundaries unforgeable?

I'm thinking of adding a second output stream later on, for machine
consumption, with that info carefully quoted.

- FChE

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Mark Wielaard]

Hi Frank,

On Mon, 2020-01-06 at 04:53 -0500, Frank Ch. Eigler wrote:
> +debuginfod-client-useragent.h:
> +	if type uname 2>/dev/null; then \
> +		echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 "'`uname -sm | sed -e 's, ,/,g'`'"' > $@; \
> +	else \
> +		echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 ""' > $@; \
> +	fi
> +	if type lsb_release 2>/dev/null; then \
> +		echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 "'`lsb_release -sir | sed -e 's, ,/,g'`'"' >> $@; \
> +	else \
> +		echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 ""' > $@; \
> +	fi
> +	echo '#define DEBUGINFOD_CLIENT_USERAGENT DEBUGINFOD_CLIENT_USERAGENT_1 "," DEBUGINFOD_CLIENT_USERAGENT_2' >> $@

Eep. We really should pick up this info during runtime instead of
during build time. We do want a reproducible build. And this will most
likely produce wrong information if the package build server is on a
different OS or release than the OS/release it is producing packages
for. uname -sm might be "stable", but probably not when cross-
compiling. But where does lsb_release come from? I don't have that on
my systems.

Cheers,

Mark
> 

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Frank Ch. Eigler]

Hi -

> Eep. We really should pick up this info during runtime instead of
> during build time. 

That's what I thought at first too.  However, doing it at run time
means doing work - a popen() etc. - over and over or saving in a
locked global.  Since on a normal machine, the distro doesn't change
without elfutils also changing, it's practically a literal.

> We do want a reproducible build. 

Can you give an example?  Bootstrapping a new distro version/arch?

> And this will most likely produce wrong information if the package
> build server is on a different OS or release than the OS/release it
> is producing packages for. uname -sm might be "stable", but probably
> not when cross- compiling.

I wonder if the cross-compiled debuginfod-client case is worth
worrying about.

> But where does lsb_release come from? I don't have that on my
> systems.

BuildRequires: /usr/bin/lsb_release :-) It's a different package on
different distros.  And running at run time would make a Require:
rather than a one-time BuildRequire:.

- FChE

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

Hi Frank,

On Wed, Jan 08, 2020 at 10:11:25AM -0500, Frank Ch. Eigler wrote:
> > Eep. We really should pick up this info during runtime instead of
> > during build time. 
> 
> That's what I thought at first too.  However, doing it at run time
> means doing work - a popen() etc. - over and over or saving in a
> locked global.  Since on a normal machine, the distro doesn't change
> without elfutils also changing, it's practically a literal.

We are already opening some files to scan for cached files, open a
socket, download data, etc. I don't think one or two extra syscalls
are that much overhead.

> > We do want a reproducible build. 
> 
> Can you give an example?  Bootstrapping a new distro version/arch?

Probably best explained at https://reproducible-builds.org/ most
distributions are participating.

> > And this will most likely produce wrong information if the package
> > build server is on a different OS or release than the OS/release it
> > is producing packages for. uname -sm might be "stable", but probably
> > not when cross- compiling.
> 
> I wonder if the cross-compiled debuginfod-client case is worth
> worrying about.

I think it is.

> > But where does lsb_release come from? I don't have that on my
> > systems.
> 
> BuildRequires: /usr/bin/lsb_release :-) It's a different package on
> different distros.  And running at run time would make a Require:
> rather than a one-time BuildRequire:.

It looks like the new standard (since about 8 years) is os-release:
http://man7.org/linux/man-pages/man5/os-release.5.html

It looks like an easily parsable file format.  The attached produces
some reasonable looking identification strings on a couple of my
systems out of the box:

Linux/i686 4.19.0-6-686-pae
debian/10

Linux/x86_64 3.10.0-1062.9.1.el7.x86_64
rhel/7.7

Linux/x86_64 4.19.0-5-amd64
pureos/9.0

Maybe something like that is usable?

Cheers,

Mark

[-- Attachment #2: os.c --]
[-- Type: text/x-csrc, Size: 1171 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/utsname.h>

int
main ()
{
  struct utsname uts;
  if (uname (&uts) == 0)
    printf ("%s/%s %s\n", uts.sysname, uts.machine, uts.release);
  
  FILE *f;
  f = fopen ("/etc/os-release", "r");
  if (f == NULL)
    f = fopen ("/usr/lib/os-release", "r");

  char *id = NULL;
  char *version = NULL;
  if (f != NULL)
    {
      while (id == NULL || version == NULL)
	{
	  char buf[128];
	  char *s = &buf[0];
	  if (fgets (s, 128, f) == NULL)
	    break;

	  int len = strlen (s);
	  if (len < 3)
	    continue;

	  if (s[len - 1] == '\n')
	    {
	      s[len - 1] = '\0';
	      len--;
	    }
	  
	  char *v = strchr (s, '=');
	  if (v == NULL || strlen (v) < 2)
	    continue;

	  /* Split var and value. */
	  *v = '\0';
	  v++;

	  /* Remove optional quotes around value string. */
	  if (*v == '"' || *v == '\'')
	    {
	      v++;
	      s[len - 1] = '\0';
	    }
	  
	  if (strcmp (s, "ID") == 0)
	    id = strdup (v);
	  if (strcmp (s, "VERSION_ID") == 0)
	    version = strdup (v);
	}
      fclose (f);
    }

  printf ("%s/%s\n", id, version);

  free (id);
  free (version);

  return 0;
}

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Frank Ch. Eigler]

Hi -

> > > We do want a reproducible build. 
> > 
> > Can you give an example?  Bootstrapping a new distro version/arch?
> 
> Probably best explained at https://reproducible-builds.org/ most
> distributions are participating.

(I understood that part, just not how you thought build-time literals
would break reproducibility.)

> It looks like an easily parsable file format.  The attached produces
> some reasonable looking identification strings on a couple of my
> systems out of the box:

OK, took your code, tweaked it just a touch, confirmed it's valgrind
leak-free, and merged to master.

- FChE

Re: rfc/patch: user-agent distro-description for debuginfod http traffic

[Frank Ch. Eigler]

Hi -

> Should this add quoting to make the field boundaries unforgeable?

On second thought, added such text-filtering to the debuginfod-side
printing, and merged to master.

- FChE