Allow reading http2 bodies

This allows http2 connections to now host authenticated endpoints.
Note, this work exposed that the http2 path was not calling
preparePayload() and responses were therefore missing the
Content-Length header.  preparePayload is now called, and Content-Length
is added to the unit tests.

This commit also allows a full Redfish Service Validator test to pass
entirely using HTTP2.

Tested: Unit tests pass.

Curl /redfish/v1/Managers/bmc/LogServices/Journal/Entries
(which returns a payload larger than 16kB) succeeds and returns the
data.

Manually logging in with both basic and session authentication succeeds
over http2.

A modified Redfish-Service-Validator, changed to use httpx as its
backend, (thus using http2) succeeds.

Change-Id: I956f3ff8f442e9826312c6147d7599ab136a8e7c
Signed-off-by: Ed Tanous <ed@tanous.net>

3 files changed, 103 insertions, 15 deletions

diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index ed3748059b..a698d9eb0d 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -4,6 +4,7 @@
 #include "async_resp.hpp"
 #include "authentication.hpp"
 #include "complete_response_fields.hpp"
+#include "http_body.hpp"
 #include "http_response.hpp"
 #include "http_utility.hpp"
 #include "logging.hpp"
@@ -38,6 +39,7 @@ namespace crow
 struct Http2StreamData
 {
     Request req{};
+    std::optional<bmcweb::HttpBody::reader> reqReader;
     Response res{};
     std::optional<bmcweb::HttpBody::writer> writer;
 };
@@ -108,11 +110,12 @@ class HTTP2Connection :
             stream.writer->getWithMaxSize(ec, length);
         if (ec)
         {
+            BMCWEB_LOG_CRITICAL("Failed to get buffer");
             return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
         }
-
         if (!out)
         {
+            BMCWEB_LOG_ERROR("Empty file, setting EOF");
             *dataFlags |= NGHTTP2_DATA_FLAG_EOF;
             return 0;
         }
@@ -120,20 +123,28 @@ class HTTP2Connection :
         BMCWEB_LOG_DEBUG("Send chunk of size: {}", out->first.size());
         if (length < out->first.size())
         {
+            BMCWEB_LOG_CRITICAL(
+                "Buffer overflow that should never happen happened");
             // Should never happen because of length limit on get() above
             return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
         }
-
+        boost::asio::mutable_buffer writeableBuf(buf, length);
         BMCWEB_LOG_DEBUG("Copying {} bytes to buf", out->first.size());
-        memcpy(buf, out->first.data(), out->first.size());
+        size_t copied = boost::asio::buffer_copy(writeableBuf, out->first);
+        if (copied != out->first.size())
+        {
+            BMCWEB_LOG_ERROR(
+                "Couldn't copy all {} bytes into buffer, only copied {}",
+                out->first.size(), copied);
+            return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+        }
 
         if (!out->second)
         {
-            BMCWEB_LOG_DEBUG("Setting OEF and nocopy flag");
+            BMCWEB_LOG_DEBUG("Setting EOF flag");
             *dataFlags |= NGHTTP2_DATA_FLAG_EOF;
-            //*dataFlags |= NGHTTP2_DATA_FLAG_NO_COPY;
         }
-        return static_cast<ssize_t>(out->first.size());
+        return static_cast<ssize_t>(copied);
     }
 
     nghttp2_nv headerFromStringViews(std::string_view name,
@@ -161,6 +172,7 @@ class HTTP2Connection :
 
         completeResponseFields(thisReq, thisRes);
         thisRes.addHeader(boost::beast::http::field::date, getCachedDateStr());
+        thisRes.preparePayload();
 
         boost::beast::http::fields& fields = thisRes.fields();
         std::string code = std::to_string(thisRes.resultInt());
@@ -200,6 +212,7 @@ class HTTP2Connection :
         callbacks.setOnStreamCloseCallback(onStreamCloseCallbackStatic);
         callbacks.setOnHeaderCallback(onHeaderCallbackStatic);
         callbacks.setOnBeginHeadersCallback(onBeginHeadersCallbackStatic);
+        callbacks.setOnDataChunkRecvCallback(onDataChunkRecvStatic);
 
         nghttp2_session session(callbacks);
         session.setUserData(this);
@@ -217,7 +230,17 @@ class HTTP2Connection :
             close();
             return -1;
         }
-
+        if (it->second.reqReader)
+        {
+            boost::beast::error_code ec;
+            it->second.reqReader->finish(ec);
+            if (ec)
+            {
+                BMCWEB_LOG_CRITICAL("Failed to finalize payload");
+                close();
+                return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+            }
+        }
         crow::Request& thisReq = it->second.req;
         BMCWEB_LOG_DEBUG("Handling {} \"{}\"", logPtr(&thisReq),
                          thisReq.url().encoded_path());
@@ -235,11 +258,69 @@ class HTTP2Connection :
         });
         auto asyncResp =
             std::make_shared<bmcweb::AsyncResp>(std::move(it->second.res));
-        handler->handle(thisReq, asyncResp);
+#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
+        thisReq.session = crow::authentication::authenticate(
+            {}, thisRes, thisReq.method(), thisReq.req, nullptr);
+        if (!crow::authentication::isOnAllowlist(thisReq.url().path(),
+                                                 thisReq.method()) &&
+            thisReq.session == nullptr)
+        {
+            BMCWEB_LOG_WARNING("Authentication failed");
+            forward_unauthorized::sendUnauthorized(
+                thisReq.url().encoded_path(),
+                thisReq.getHeaderValue("X-Requested-With"),
+                thisReq.getHeaderValue("Accept"), thisRes);
+        }
+        else
+#endif // BMCWEB_INSECURE_DISABLE_AUTHX
+        {
+            handler->handle(thisReq, asyncResp);
+        }
+        return 0;
+    }
 
+    int onDataChunkRecvCallback(uint8_t /*flags*/, int32_t streamId,
+                                const uint8_t* data, size_t len)
+    {
+        auto thisStream = streams.find(streamId);
+        if (thisStream == streams.end())
+        {
+            BMCWEB_LOG_ERROR("Unknown stream{}", streamId);
+            close();
+            return -1;
+        }
+        if (!thisStream->second.reqReader)
+        {
+            thisStream->second.reqReader.emplace(
+                thisStream->second.req.req.base(),
+                thisStream->second.req.req.body());
+        }
+        boost::beast::error_code ec;
+        thisStream->second.reqReader->put(boost::asio::const_buffer(data, len),
+                                          ec);
+        if (ec)
+        {
+            BMCWEB_LOG_CRITICAL("Failed to write payload");
+            return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+        }
         return 0;
     }
 
+    static int onDataChunkRecvStatic(nghttp2_session* /* session */,
+                                     uint8_t flags, int32_t streamId,
+                                     const uint8_t* data, size_t len,
+                                     void* userData)
+    {
+        BMCWEB_LOG_DEBUG("on_frame_recv_callback");
+        if (userData == nullptr)
+        {
+            BMCWEB_LOG_CRITICAL("user data was null?");
+            return NGHTTP2_ERR_CALLBACK_FAILURE;
+        }
+        return userPtrToSelf(userData).onDataChunkRecvCallback(flags, streamId,
+                                                               data, len);
+    }
+
     int onFrameRecvCallback(const nghttp2_frame& frame)
     {
         BMCWEB_LOG_DEBUG("frame type {}", static_cast<int>(frame.hd.type));
@@ -450,8 +531,8 @@ class HTTP2Connection :
             return;
         }
         isWriting = true;
-        adaptor.async_write_some(
-            boost::asio::buffer(data.data(), data.size()),
+        boost::asio::async_write(
+            adaptor, boost::asio::const_buffer(data.data(), data.size()),
             std::bind_front(afterWriteBuffer, shared_from_this()));
     }
 
diff --git a/http/nghttp2_adapters.hpp b/http/nghttp2_adapters.hpp
index aeb18d60ab..3235372157 100644
--- a/http/nghttp2_adapters.hpp
+++ b/http/nghttp2_adapters.hpp
@@ -84,6 +84,13 @@ struct nghttp2_session_callbacks
             ptr, afterSendFrame);
     }
 
+    void setOnDataChunkRecvCallback(
+        nghttp2_on_data_chunk_recv_callback afterDataChunkRecv)
+    {
+        nghttp2_session_callbacks_set_on_data_chunk_recv_callback(
+            ptr, afterDataChunkRecv);
+    }
+
   private:
     nghttp2_session_callbacks* get()
     {
diff --git a/test/http/http2_connection_test.cpp b/test/http/http2_connection_test.cpp
index 1c6ae6ce0d..72f985c761 100644
--- a/test/http/http2_connection_test.cpp
+++ b/test/http/http2_connection_test.cpp
@@ -127,8 +127,8 @@ TEST(http_connection, RequestPropogates)
         // Settings ACK from server to client
         "\x00\x00\x00\x04\x01\x00\x00\x00\x00"
 
-        // Start Headers frame stream 1, size 0x0346
-        "\x00\x03\x46\x01\x04\x00\x00\x00\x01"sv;
+        // Start Headers frame stream 1, size 0x034b
+        "\x00\x03\x4b\x01\x04\x00\x00\x00\x01"sv;
 
     std::string_view expectedPostfix =
         // Data Frame, Length 12, Stream 1, End Stream flag set
@@ -137,7 +137,7 @@ TEST(http_connection, RequestPropogates)
         "StringOutput"sv;
 
     std::string_view outStr;
-    constexpr size_t headerSize = 0x346;
+    constexpr size_t headerSize = 0x34b;
 
     // Run until we receive the expected amount of data
     while (outStr.size() <
@@ -149,7 +149,7 @@ TEST(http_connection, RequestPropogates)
     EXPECT_TRUE(handler.called);
 
     // check the stream output against expected
-    EXPECT_TRUE(outStr.starts_with(expectedPrefix));
+    EXPECT_EQ(outStr.substr(0, expectedPrefix.size()), expectedPrefix);
     outStr.remove_prefix(expectedPrefix.size());
     std::vector<std::pair<std::string, std::string>> headers;
     unpackHeaders(outStr.substr(0, headerSize), headers);
@@ -158,7 +158,7 @@ TEST(http_connection, RequestPropogates)
     EXPECT_THAT(
         headers,
         UnorderedElementsAre(
-            Pair(":status", "200"),
+            Pair(":status", "200"), Pair("content-length", "12"),
             Pair("strict-transport-security",
                  "max-age=31536000; includeSubdomains"),
             Pair("x-frame-options", "DENY"), Pair("pragma", "no-cache"),