[release/4.x] Cherry pick: Do not enforce default parsing limits on f…

…orwarded traffic (#5803) (#5806)

.daily_canary

@@ -1,4 +1,4 @@
    -^-     _X_           ___
   (- -)   (= =)   |     Y   &  +--?
  (  V  ) /  .  \  |    +---=---'
-/--x-m- /--n-n---xXx--/--yY------>>>----<<<>>]]
+/--x-m- /--n-n---xXx--/--yY------>>>----<<<>>]]{{}}--||

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [4.0.12]
+
+[4.0.12]: https://github.com/microsoft/CCF/releases/tag/ccf-4.0.12
+
+- Lifted parser size limits on forwarded request from default values to more permissive ones. Note that the limits set out on the interface of the inbound node still apply (#5803).
+
 ## [4.0.11]
 
 [4.0.11]: https://github.com/microsoft/CCF/releases/tag/ccf-4.0.11

CMakeLists.txt

@@ -1344,6 +1344,13 @@ if(BUILD_TESTS)
                       ${CMAKE_SOURCE_DIR}/samples/apps/logging/js
     )
 
+    if(NOT SAN)
+      add_e2e_test(
+        NAME e2e_limits PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/limits.py
+        CONSENSUS cft
+      )
+    endif()
+
     add_e2e_test(
       NAME e2e_logging_http2
       PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/e2e_logging.py

include/ccf/ds/unit_strings.h

@@ -128,6 +128,11 @@ namespace ds
       value(convert_size_string(str_))
     {}
 
+    SizeString(const char* str_) :
+      UnitString(str_),
+      value(convert_size_string(str_))
+    {}
+
     inline operator size_t() const
     {
       return value;

include/ccf/http_configuration.h

@@ -9,6 +9,8 @@
 
 namespace http
 {
+  // Default parser limits, used as a DoS protection against
+  // requests that are too large.
   static const ds::SizeString default_max_body_size = {"1MB"};
   static const ds::SizeString default_max_header_size = {"16KB"};
   static const uint32_t default_max_headers_count = 256;
@@ -43,4 +45,18 @@ namespace http
     max_concurrent_streams_count,
     initial_window_size,
     max_frame_size);
+
+  // A permissive configuration, used for internally forwarded requests
+  // that have already been through application-defined limits.
+  static ParserConfiguration permissive_configuration()
+  {
+    ParserConfiguration config;
+    config.max_body_size = "1GB";
+    config.max_header_size = "100MB";
+    config.max_headers_count = 1024;
+    config.max_concurrent_streams_count = 1;
+    config.initial_window_size = "64KB";
+    config.max_frame_size = "16MB";
+    return config;
+  }
 }

src/http/http_rpc_context.h

@@ -374,8 +374,7 @@ namespace ccf
     std::shared_ptr<ccf::SessionContext> s, const std::vector<uint8_t>& packed)
   {
     http::SimpleRequestProcessor processor;
-    http::RequestParser parser(processor);
-
+    http::RequestParser parser(processor, http::permissive_configuration());
     parser.execute(packed.data(), packed.size());
 
     if (processor.received.size() != 1)

src/node/rpc/forwarder.h

@@ -237,6 +237,11 @@ namespace ccf
         return ccf::make_fwd_rpc_context(
           session, raw_request, r.first.frame_format);
       }
+      catch (const http::RequestTooLargeException& rexc)
+      {
+        LOG_FAIL_FMT("Forwarded request exceeded limit: {}", rexc.what());
+        return nullptr;
+      }
       catch (const std::exception& err)
       {
         LOG_FAIL_FMT("Invalid forwarded request");

tests/limits.py

@@ -0,0 +1,83 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the Apache 2.0 License.
+import infra.network
+import infra.e2e_args
+import infra.checker
+import infra.jwt_issuer
+import infra.proc
+import http
+import infra.clients
+import infra.crypto
+from infra.runner import ConcurrentRunner
+import copy
+
+
+def test_forward_larger_than_default_requests(network, args):
+    new_node = network.create_node(
+        infra.interfaces.HostSpec(
+            rpc_interfaces={
+                infra.interfaces.PRIMARY_RPC_INTERFACE: infra.interfaces.RPCInterface(
+                    max_http_body_size=10 * 1024 * 1024,
+                    # Deliberately large because some builds (eg. SGX Debug) take
+                    # a long time to process large requests
+                    forwarding_timeout_ms=8000,
+                )
+            }
+        )
+    )
+    network.join_node(new_node, args.package, args)
+    network.trust_node(new_node, args)
+
+    primary, _ = network.find_primary()
+
+    # Big request, but under the cap
+    with primary.client("user0") as c:
+        msg = "A" * 512 * 1024
+        r = c.post("/app/log/private", {"id": 42, "msg": msg})
+        assert r.status_code == http.HTTPStatus.OK.value, r
+
+    # Big request, over the cap for the primary
+    with primary.client("user0") as c:
+        msg = "A" * 2 * 1024 * 1024
+        r = c.post("/app/log/private", {"id": 42, "msg": msg})
+        assert r.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value, r
+
+    # Big request, over the cap for the primary, but under the cap for the new node
+    with new_node.client("user0") as c:
+        msg = "A" * 2 * 1024 * 1024
+        r = c.post("/app/log/private", {"id": 42, "msg": msg})
+        assert r.status_code == http.HTTPStatus.OK.value, r
+
+
+def run_parser_limits_checks(args):
+    new_args = copy.copy(args)
+    # Deliberately large because some builds (eg. SGX Debug) take
+    # a long time to process large requests
+    new_args.election_timeout_ms = 10000
+    new_args.host_log_level = "info"
+    new_args.enclave_log_level = "info"
+    with infra.network.network(
+        new_args.nodes,
+        new_args.binary_dir,
+        new_args.debug_nodes,
+        new_args.perf_nodes,
+        pdb=args.pdb,
+    ) as network:
+        network.start_and_open(new_args)
+
+        test_forward_larger_than_default_requests(network, new_args)
+
+
+if __name__ == "__main__":
+    cr = ConcurrentRunner()
+
+    if not cr.args.http2:
+        # No support for forwarding with HTTP/2
+        cr.add(
+            "parser_limits",
+            run_parser_limits_checks,
+            package="samples/apps/logging/liblogging",
+            nodes=infra.e2e_args.max_nodes(cr.args, f=0),
+        )
+
+    cr.run()