From ad18012c46724aa097f37015a8036a4343206efe Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Fri, 7 Dec 2018 12:47:14 +0100

Subject: [PATCH] journal-remote: verify entry length from header

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Calling mhd_respond(), which ulimately calls MHD_queue_response() is

ineffective at point, becuase MHD_queue_response() immediately returns

MHD_NO signifying an error, because the connection is in state

MHD_CONNECTION_CONTINUE_SENT.

As Christian Grothoff kindly explained:

> You are likely calling MHD_queue_repsonse() too late: once you are

> receiving upload_data, HTTP forces you to process it all. At this time,

> MHD has already sent "100 continue" and cannot take it back (hence you

> get MHD_NO!).

>

> In your request handler, the first time when you are called for a

> connection (and when hence *upload_data_size == 0 and upload_data ==

> NULL) you must check the content-length header and react (with

> MHD_queue_response) based on this (to prevent MHD from automatically

> generating 100 continue).

If we ever encounter this kind of error, print a warning and immediately

abort the connection. (The alternative would be to keep reading the data,

but ignore it, and return an error after we get to the end of data.

That is possible, but of course puts additional load on both the

sender and reciever, and doesn't seem important enough just to return

a good error message.)

Note that sending of the error does not work (the connection is always aborted

when MHD_queue_response is used with MHD_RESPMEM_MUST_FREE, as in this case)

with libµhttpd 0.59, but works with 0.61:

https://src.fedoraproject.org/rpms/libmicrohttpd/pull-request/1

(cherry-picked from commit 7fdb237f5473cb8fc2129e57e8a0039526dcb4fd)

Related: #1664977

---

 src/journal-remote/journal-remote-main.c | 34 +++++++++++++++++-------

 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c

index 8fda9d149..e9b3702e8 100644

--- a/src/journal-remote/journal-remote-main.c

+++ b/src/journal-remote/journal-remote-main.c

@@ -210,16 +210,14 @@ static int process_http_upload(

                                    journal_remote_server_global->seal);

                 if (r == -EAGAIN)

                         break;

-                else if (r < 0) {

-                        log_warning("Failed to process data for connection %p", connection);

+                if (r < 0) {

                         if (r == -E2BIG)

-                                return mhd_respondf(connection,

-                                                    r, MHD_HTTP_PAYLOAD_TOO_LARGE,

-                                                    "Entry is too large, maximum is " STRINGIFY(DATA_SIZE_MAX) " bytes.");

+                                log_warning_errno(r, "Entry is too above maximum of %u, aborting connection %p.",

+                                                  DATA_SIZE_MAX, connection);

                         else

-                                return mhd_respondf(connection,

-                                                    r, MHD_HTTP_UNPROCESSABLE_ENTITY,

-                                                    "Processing failed: %m.");

+                                log_warning_errno(r, "Failed to process data, aborting connection %p: %m",

+                                                  connection);

+                        return MHD_NO;

                 }

         }

@@ -253,6 +251,7 @@ static int request_handler(

         const char *header;

         int r, code, fd;

         _cleanup_free_ char *hostname = NULL;

+        size_t len;

         assert(connection);

         assert(connection_cls);

@@ -272,12 +271,27 @@ static int request_handler(

         if (!streq(url, "/upload"))

                 return mhd_respond(connection, MHD_HTTP_NOT_FOUND, "Not found.");

-        header = MHD_lookup_connection_value(connection,

-                                             MHD_HEADER_KIND, "Content-Type");

+        header = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "Content-Type");

         if (!header || !streq(header, "application/vnd.fdo.journal"))

                 return mhd_respond(connection, MHD_HTTP_UNSUPPORTED_MEDIA_TYPE,

                                    "Content-Type: application/vnd.fdo.journal is required.");

+        header = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "Content-Length");

+        if (!header)

+                return mhd_respond(connection, MHD_HTTP_LENGTH_REQUIRED,

+                                   "Content-Length header is required.");

+        r = safe_atozu(header, &len;;

+        if (r < 0)

+                return mhd_respondf(connection, r, MHD_HTTP_LENGTH_REQUIRED,

+                                    "Content-Length: %s cannot be parsed: %m", header);

+

+        if (len > ENTRY_SIZE_MAX)

+                /* When serialized, an entry of maximum size might be slightly larger,

+                 * so this does not correspond exactly to the limit in journald. Oh well.

+                 */

+                return mhd_respondf(connection, 0, MHD_HTTP_PAYLOAD_TOO_LARGE,

+                                    "Payload larger than maximum size of %u bytes", ENTRY_SIZE_MAX);

+

         {

                 const union MHD_ConnectionInfo *ci;