Hi Frank,
On Fri, 2020-03-27 at 09:59 -0400, Frank Ch. Eigler wrote:
> I don't think it is a scare story to explicitly say: "Note that the
> > current implementation uses libcurl, but you shouldn't rely on that
> > fact. The only supported usage of this method is for adding an
> > optional header which might or might not be passed through to the
> > server."
>
> OK, please feel free to add any such text that makes you feel more
> comfortable.
Done, as attached.
> > Sure. It is simple to write your own client code using libcurl if you
> > want to. And it might be too hard to sanity check the input. If it is,
> > too bad. But if it is easy to check then I think we should simply do
> > that to catch user mistakes.
>
> We were talking about people who read the code to work around the
> documented pattern. These would not be user mistakes. We're
> proposing protecting someone not from their mistakes, but their
> deliberate reverse-engineering.
It is indeed too hard to know exactly which standard headers libcurl
adds. But we can at least sanity check the header string form is
correct. I added a simple check to the function to see it is at least
somewhat plausible, so that a user won't be surprised if a wrongly
formatted string accidentally removes a header instead of adding one.
Cheers,
Mark
From 7ddceee2b6b0a3fe752a2e8cc5d5cfd0f45d6897 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Mon, 30 Mar 2020 00:57:30 +0200
Subject: [PATCH] debuginfod: Document and sanity check
debuginfod_add_http_header format.
Document and sanity check the format of the header string form that can
be passed to debuginfod_add_http_header. It should contain precisely
one colon, which cannot be the first or last character. And the function
should only be used to add optional headers, not replace any existing
standard ones. Anything else isn't supported.
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
debuginfod/ChangeLog | 5 +++++
debuginfod/debuginfod-client.c | 10 ++++++++++
doc/ChangeLog | 5 +++++
doc/debuginfod_find_debuginfo.3 | 10 +++++++++-
4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 9901c521..bc3bce32 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-29 Mark Wielaard <m...@klomp.org>
+
+ * debuginfod-client.c (debuginfod_add_http_header): Check header
+ contains precisely one colon that isn't the first or last char.
+
2020-03-29 Frank Ch. Eigler <f...@redhat.com>
* debuginfod-client.c (struct debuginfod_client): Add a flag field
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index fa017a84..a7dfbfb1 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1035,6 +1035,16 @@ int debuginfod_find_source(debuginfod_client *client,
/* Add an outgoing HTTP header. */
int debuginfod_add_http_header (debuginfod_client *client, const char* header)
{
+ /* Sanity check header value is of the form Header: Value.
+ It should contain exactly one colon that isn't the first or
+ last character. */
+ char *colon = strchr (header, ':');
+ if (colon == NULL
+ || colon == header
+ || *(colon + 1) == '\0'
+ || strchr (colon + 1, ':') != NULL)
+ return -EINVAL;
+
struct curl_slist *temp = curl_slist_append (client->headers, header);
if (temp == NULL)
return -ENOMEM;
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 068a1957..f598b7f2 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-29 Mark Wielaard <m...@klomp.org>
+
+ * debuginfod_find_debuginfo.3 (HTTP HEADER): Document the expected
+ header format and purpose.
+
2020-03-28 Frank Ch. Eigler <f...@redhat.com>
* debuginfod.8: Document valid --port=NUM range, excludes 0.
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index 1c7c4991..d9717d73 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -171,7 +171,15 @@ may be called with strings of the form
.BR \%"Header:\~value" .
These strings are copied by the library. A zero return value
indicates success, but out-of-memory conditions may result in
-a non-zero \fI-ENOMEM\fP.
+a non-zero \fI-ENOMEM\fP. If the string is in the wrong form
+\fI-EINVAL\fP will be returned.
+
+Note that the current debuginfod-client library implementation uses
+libcurl, but you shouldn't rely on that fact. Don't use this function
+for replacing any standard headers, except for the User-Agent mentioned
+below. The only supported usage of this function is for adding an
+optional header which might or might not be passed through to the
+server for logging purposes only.
By default, the library adds a descriptive \fIUser-Agent:\fP
header to outgoing requests. If the client application adds
--
2.18.2
