From b7b3514bb1f71cdad552ba712f683b4d427c4aec Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Sat, 14 Oct 2023 06:11:01 -0400 Subject: [PATCH 1/3] Introduce HTTP request body limit for definition uploads The default is 20 MiB, which is enough to upload a definition file with 200K queues, a few virtual host and a few users. In other words, it should accomodate a lot of environments. --- deps/rabbitmq_management/BUILD.bazel | 3 ++- deps/rabbitmq_management/Makefile | 3 ++- .../include/rabbit_mgmt.hrl | 2 ++ .../priv/schema/rabbitmq_management.schema | 17 +++++++++++++ .../src/rabbit_mgmt_util.erl | 24 ++++++++++++++----- .../src/rabbit_mgmt_wm_definitions.erl | 11 +++++++-- 6 files changed, 50 insertions(+), 10 deletions(-) diff --git a/deps/rabbitmq_management/BUILD.bazel b/deps/rabbitmq_management/BUILD.bazel index 2de82451af..91d8e346cb 100644 --- a/deps/rabbitmq_management/BUILD.bazel +++ b/deps/rabbitmq_management/BUILD.bazel @@ -35,7 +35,8 @@ APP_ENV = """[ {cors_allow_origins, []}, {cors_max_age, 1800}, - {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"} + {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"}, + {max_http_body_size, 20000000} ]""" genrule( diff --git a/deps/rabbitmq_management/Makefile b/deps/rabbitmq_management/Makefile index dea06b0000..0160d48dcc 100644 --- a/deps/rabbitmq_management/Makefile +++ b/deps/rabbitmq_management/Makefile @@ -12,7 +12,8 @@ define PROJECT_ENV {cors_allow_origins, []}, {cors_max_age, 1800}, - {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"} + {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"}, + {max_http_body_size, 20000000} ] endef diff --git a/deps/rabbitmq_management/include/rabbit_mgmt.hrl b/deps/rabbitmq_management/include/rabbit_mgmt.hrl index a9a7a44217..d1358920ce 100644 --- a/deps/rabbitmq_management/include/rabbit_mgmt.hrl +++ b/deps/rabbitmq_management/include/rabbit_mgmt.hrl @@ -11,3 +11,5 @@ -define(MANAGEMENT_PG_SCOPE, rabbitmq_management). -define(MANAGEMENT_PG_GROUP, management_db). + +-define(MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE, 20000000). diff --git a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema index 30a8bc84c0..8374b43d44 100644 --- a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema +++ b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema @@ -20,6 +20,23 @@ {mapping, "management.http_log_dir", "rabbitmq_management.http_log_dir", [{datatype, string}]}. +%% Max HTTP body limit + +{mapping, "management.http.max_body_size", "rabbitmq_management.max_http_body_size", + [{datatype, integer}, {validators, ["non_negative_integer"]}]}. + +{translation, "rabbitmq_management.max_http_body_size", +fun(Conf) -> + case cuttlefish:conf_get("management.http.max_body_size", Conf, undefined) of + %% 20 MiB allows for about 200K queues across a small (single digit) number of virtual hosts with + %% an equally small number of users. MK. + undefined -> 20000000; + Val when is_integer(Val) -> Val; + Other -> cuttlefish:invalid("management.http.max_body_size must be set to a positive integer") + end +end}. + + %% HTTP (TCP) listener options ======================================================== %% HTTP listener consistent with Web STOMP and Web MQTT. diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_util.erl b/deps/rabbitmq_management/src/rabbit_mgmt_util.erl index 9ae365aff2..abfb6ae786 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_util.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_util.erl @@ -686,15 +686,27 @@ id(Key, ReqData) -> read_complete_body(Req) -> read_complete_body(Req, <<"">>). -read_complete_body(Req0, Acc) -> - case cowboy_req:read_body(Req0) of - {ok, Data, Req} -> {ok, <>, Req}; - {more, Data, Req} -> read_complete_body(Req, <>) +read_complete_body(Req, Acc) -> + BodySizeLimit = application:get_env(rabbitmq_management, max_http_body_size, ?MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE), + read_complete_body(Req, Acc, BodySizeLimit). +read_complete_body(Req0, Acc, BodySizeLimit) -> + case bit_size(Acc) > BodySizeLimit of + true -> + {error, "Exceeded HTTP request body size limit"}; + false -> + case cowboy_req:read_body(Req0) of + {ok, Data, Req} -> {ok, <>, Req}; + {more, Data, Req} -> read_complete_body(Req, <>) + end end. with_decode(Keys, ReqData, Context, Fun) -> - {ok, Body, ReqData1} = read_complete_body(ReqData), - with_decode(Keys, Body, ReqData1, Context, Fun). + case read_complete_body(ReqData) of + {error, Reason} -> + bad_request(Reason, ReqData, Context); + {ok, Body, ReqData1} -> + with_decode(Keys, Body, ReqData1, Context, Fun) + end. with_decode(Keys, Body, ReqData, Context, Fun) -> case decode(Keys, Body) of diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl index b6905ab7e7..a9325732ed 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl @@ -84,8 +84,15 @@ all_definitions(ReqData, Context) -> Context). accept_json(ReqData0, Context) -> - {ok, Body, ReqData} = rabbit_mgmt_util:read_complete_body(ReqData0), - accept(Body, ReqData, Context). + case rabbit_mgmt_util:read_complete_body(ReqData0) of + {error, Reason} -> + BodySizeLimit = application:get_env(rabbitmq_management, max_http_body_size, ?MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE), + _ = rabbit_log:warning("HTTP API: uploaded definition file exceeded the maximum request body limit of ~p bytes. " + "Use the 'management.http.max_body_size' key in rabbitmq.conf to increase the limit if necessary", [BodySizeLimit]), + rabbit_mgmt_util:bad_request(Reason, ReqData0, Context); + {ok, Body, ReqData} -> + accept(Body, ReqData, Context) + end. vhost_definitions(ReqData, VHost, Context) -> %% rabbit_mgmt_wm_<>:basic/1 filters by VHost if it is available From c6d0382be4d9b6f4d0ab9466b397e353adfa92e0 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Mon, 16 Oct 2023 06:48:23 -0400 Subject: [PATCH 2/3] Reduce default HTTP API request body size limit to 10 MiB per discussion with the team. It should be enough to accomodate a definition file with about 100K queues. --- deps/rabbitmq_management/BUILD.bazel | 2 +- deps/rabbitmq_management/Makefile | 2 +- .../priv/schema/rabbitmq_management.schema | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deps/rabbitmq_management/BUILD.bazel b/deps/rabbitmq_management/BUILD.bazel index 91d8e346cb..8b98847606 100644 --- a/deps/rabbitmq_management/BUILD.bazel +++ b/deps/rabbitmq_management/BUILD.bazel @@ -36,7 +36,7 @@ APP_ENV = """[ {cors_allow_origins, []}, {cors_max_age, 1800}, {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"}, - {max_http_body_size, 20000000} + {max_http_body_size, 10000000} ]""" genrule( diff --git a/deps/rabbitmq_management/Makefile b/deps/rabbitmq_management/Makefile index 0160d48dcc..f6624339b4 100644 --- a/deps/rabbitmq_management/Makefile +++ b/deps/rabbitmq_management/Makefile @@ -13,7 +13,7 @@ define PROJECT_ENV {cors_allow_origins, []}, {cors_max_age, 1800}, {content_security_policy, "script-src 'self' 'unsafe-eval' 'unsafe-inline'; object-src 'self'"}, - {max_http_body_size, 20000000} + {max_http_body_size, 10000000} ] endef diff --git a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema index 8374b43d44..66cfd1559b 100644 --- a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema +++ b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema @@ -28,9 +28,9 @@ {translation, "rabbitmq_management.max_http_body_size", fun(Conf) -> case cuttlefish:conf_get("management.http.max_body_size", Conf, undefined) of - %% 20 MiB allows for about 200K queues across a small (single digit) number of virtual hosts with + %% 10 MiB allows for about 100K queues with short names across a small (single digit) number of virtual hosts with %% an equally small number of users. MK. - undefined -> 20000000; + undefined -> 10000000; Val when is_integer(Val) -> Val; Other -> cuttlefish:invalid("management.http.max_body_size must be set to a positive integer") end From 087794dded3b3fb576ddf4174fee11a0a1e142b2 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Mon, 16 Oct 2023 19:14:16 -0400 Subject: [PATCH 3/3] HTTP API: adapt publishing tests to take the newly introduced 10 MiB default body size limit into account. --- .../test/rabbit_mgmt_http_SUITE.erl | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl index de37d83da0..1a194609ba 100644 --- a/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl +++ b/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl @@ -125,6 +125,7 @@ all_tests() -> [ get_fail_test, publish_test, publish_large_message_test, + publish_large_message_exceeding_http_request_body_size_test, publish_accept_json_test, publish_fail_test, publish_base64_test, @@ -2636,7 +2637,7 @@ get_fail_test(Config) -> passed. --define(LARGE_BODY_BYTES, 25000000). +-define(LARGE_BODY_BYTES, 5000000). publish_test(Config) -> Headers = #{'x-forwarding' => [#{uri => <<"amqp://localhost/%2F/upstream">>}]}, @@ -2679,6 +2680,19 @@ publish_large_message_test(Config) -> http_delete(Config, "/queues/%2F/publish_accept_json_test", {group, '2xx'}), passed. +-define(EXCESSIVELY_LARGE_BODY_BYTES, 35000000). + +publish_large_message_exceeding_http_request_body_size_test(Config) -> + Headers = #{'x-forwarding' => [#{uri => <<"amqp://localhost/%2F/upstream">>}]}, + Body = binary:copy(<<"a">>, ?EXCESSIVELY_LARGE_BODY_BYTES), + Msg = msg(<<"large_message_exceeding_http_request_body_size_test">>, Headers, Body), + http_put(Config, "/queues/%2F/large_message_exceeding_http_request_body_size_test", #{}, {group, '2xx'}), + %% exceeds the default HTTP API request body size limit + http_post_accept_json(Config, "/exchanges/%2F/amq.default/publish", + Msg, ?BAD_REQUEST), + http_delete(Config, "/queues/%2F/large_message_exceeding_http_request_body_size_test", {group, '2xx'}), + passed. + publish_accept_json_test(Config) -> Headers = #{'x-forwarding' => [#{uri => <<"amqp://localhost/%2F/upstream">>}]}, Msg = msg(<<"publish_accept_json_test">>, Headers, <<"Hello world">>),