daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
ff6046
From ad18012c46724aa097f37015a8036a4343206efe Mon Sep 17 00:00:00 2001
ff6046
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
ff6046
Date: Fri, 7 Dec 2018 12:47:14 +0100
ff6046
Subject: [PATCH] journal-remote: verify entry length from header
ff6046
MIME-Version: 1.0
ff6046
Content-Type: text/plain; charset=UTF-8
ff6046
Content-Transfer-Encoding: 8bit
ff6046
ff6046
Calling mhd_respond(), which ulimately calls MHD_queue_response() is
ff6046
ineffective at point, becuase MHD_queue_response() immediately returns
ff6046
MHD_NO signifying an error, because the connection is in state
ff6046
MHD_CONNECTION_CONTINUE_SENT.
ff6046
ff6046
As Christian Grothoff kindly explained:
ff6046
> You are likely calling MHD_queue_repsonse() too late: once you are
ff6046
> receiving upload_data, HTTP forces you to process it all. At this time,
ff6046
> MHD has already sent "100 continue" and cannot take it back (hence you
ff6046
> get MHD_NO!).
ff6046
>
ff6046
> In your request handler, the first time when you are called for a
ff6046
> connection (and when hence *upload_data_size == 0 and upload_data ==
ff6046
> NULL) you must check the content-length header and react (with
ff6046
> MHD_queue_response) based on this (to prevent MHD from automatically
ff6046
> generating 100 continue).
ff6046
ff6046
If we ever encounter this kind of error, print a warning and immediately
ff6046
abort the connection. (The alternative would be to keep reading the data,
ff6046
but ignore it, and return an error after we get to the end of data.
ff6046
That is possible, but of course puts additional load on both the
ff6046
sender and reciever, and doesn't seem important enough just to return
ff6046
a good error message.)
ff6046
ff6046
Note that sending of the error does not work (the connection is always aborted
ff6046
when MHD_queue_response is used with MHD_RESPMEM_MUST_FREE, as in this case)
ff6046
with libµhttpd 0.59, but works with 0.61:
ff6046
https://src.fedoraproject.org/rpms/libmicrohttpd/pull-request/1
ff6046
ff6046
(cherry-picked from commit 7fdb237f5473cb8fc2129e57e8a0039526dcb4fd)
ff6046
ff6046
Related: #1664977
ff6046
---
ff6046
 src/journal-remote/journal-remote-main.c | 34 +++++++++++++++++-------
ff6046
 1 file changed, 24 insertions(+), 10 deletions(-)
ff6046
ff6046
diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c
ff6046
index 8fda9d1499..e9b3702e8a 100644
ff6046
--- a/src/journal-remote/journal-remote-main.c
ff6046
+++ b/src/journal-remote/journal-remote-main.c
ff6046
@@ -210,16 +210,14 @@ static int process_http_upload(
ff6046
                                    journal_remote_server_global->seal);
ff6046
                 if (r == -EAGAIN)
ff6046
                         break;
ff6046
-                else if (r < 0) {
ff6046
-                        log_warning("Failed to process data for connection %p", connection);
ff6046
+                if (r < 0) {
ff6046
                         if (r == -E2BIG)
ff6046
-                                return mhd_respondf(connection,
ff6046
-                                                    r, MHD_HTTP_PAYLOAD_TOO_LARGE,
ff6046
-                                                    "Entry is too large, maximum is " STRINGIFY(DATA_SIZE_MAX) " bytes.");
ff6046
+                                log_warning_errno(r, "Entry is too above maximum of %u, aborting connection %p.",
ff6046
+                                                  DATA_SIZE_MAX, connection);
ff6046
                         else
ff6046
-                                return mhd_respondf(connection,
ff6046
-                                                    r, MHD_HTTP_UNPROCESSABLE_ENTITY,
ff6046
-                                                    "Processing failed: %m.");
ff6046
+                                log_warning_errno(r, "Failed to process data, aborting connection %p: %m",
ff6046
+                                                  connection);
ff6046
+                        return MHD_NO;
ff6046
                 }
ff6046
         }
ff6046
 
ff6046
@@ -253,6 +251,7 @@ static int request_handler(
ff6046
         const char *header;
ff6046
         int r, code, fd;
ff6046
         _cleanup_free_ char *hostname = NULL;
ff6046
+        size_t len;
ff6046
 
ff6046
         assert(connection);
ff6046
         assert(connection_cls);
ff6046
@@ -272,12 +271,27 @@ static int request_handler(
ff6046
         if (!streq(url, "/upload"))
ff6046
                 return mhd_respond(connection, MHD_HTTP_NOT_FOUND, "Not found.");
ff6046
 
ff6046
-        header = MHD_lookup_connection_value(connection,
ff6046
-                                             MHD_HEADER_KIND, "Content-Type");
ff6046
+        header = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "Content-Type");
ff6046
         if (!header || !streq(header, "application/vnd.fdo.journal"))
ff6046
                 return mhd_respond(connection, MHD_HTTP_UNSUPPORTED_MEDIA_TYPE,
ff6046
                                    "Content-Type: application/vnd.fdo.journal is required.");
ff6046
 
ff6046
+        header = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "Content-Length");
ff6046
+        if (!header)
ff6046
+                return mhd_respond(connection, MHD_HTTP_LENGTH_REQUIRED,
ff6046
+                                   "Content-Length header is required.");
ff6046
+        r = safe_atozu(header, &len;;
ff6046
+        if (r < 0)
ff6046
+                return mhd_respondf(connection, r, MHD_HTTP_LENGTH_REQUIRED,
ff6046
+                                    "Content-Length: %s cannot be parsed: %m", header);
ff6046
+
ff6046
+        if (len > ENTRY_SIZE_MAX)
ff6046
+                /* When serialized, an entry of maximum size might be slightly larger,
ff6046
+                 * so this does not correspond exactly to the limit in journald. Oh well.
ff6046
+                 */
ff6046
+                return mhd_respondf(connection, 0, MHD_HTTP_PAYLOAD_TOO_LARGE,
ff6046
+                                    "Payload larger than maximum size of %u bytes", ENTRY_SIZE_MAX);
ff6046
+
ff6046
         {
ff6046
                 const union MHD_ConnectionInfo *ci;
ff6046