This is an automated email from the ASF dual-hosted git repository. vatamane pushed a commit to branch scrub-security-header in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 04736ec397126ec98d7c69d318a4d7f56ac950fc Author: Nick Vatamaniuc <[email protected]> AuthorDate: Wed Oct 11 02:28:22 2023 -0400 Add an option to scrub some sensitive headers from external json requests We're already doing it for the cached request object in the process dictionary so it makes sense to do it for the external json requests as well. For compatibility, allow reverting to previous behavior using the `[chttpd] scrub_json_request` config setting. --- rel/overlay/etc/default.ini | 4 ++ src/chttpd/src/chttpd_external.erl | 18 +++++-- src/chttpd/src/chttpd_util.erl | 13 +++-- src/chttpd/test/eunit/chttpd_external_test.erl | 70 +++++++++++++++++++++----- 4 files changed, 85 insertions(+), 20 deletions(-) diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini index 68ae900a3..c7f3744ce 100644 --- a/rel/overlay/etc/default.ini +++ b/rel/overlay/etc/default.ini @@ -214,6 +214,10 @@ bind_address = 127.0.0.1 ; stampede in case when there are lot of concurrent clients connecting. ;disconnect_check_jitter_msec = 15000 +; Scrub auth and cookie headers from external json request objects. +; Set to false to avoid scrubbing and revert to the previous behavior. +;scrub_json_request = true + ;[jwt_auth] ; List of claims to validate ; can be the name of a claim like "exp" or a tuple if the claim requires diff --git a/src/chttpd/src/chttpd_external.erl b/src/chttpd/src/chttpd_external.erl index 352087d58..4cd1d996f 100644 --- a/src/chttpd/src/chttpd_external.erl +++ b/src/chttpd/src/chttpd_external.erl @@ -29,8 +29,14 @@ json_req_obj(Req, Db, DocId) -> json_req_obj(Req, Db, DocId, all) -> Fields = json_req_obj_fields(), json_req_obj(Req, Db, DocId, Fields); -json_req_obj(Req, Db, DocId, Fields) when is_list(Fields) -> - {[{Field, json_req_obj_field(Field, Req, Db, DocId)} || Field <- Fields]}. +json_req_obj(#httpd{} = Req, Db, DocId, Fields) when is_list(Fields) -> + MochiReq = + case scrub_json_request() of + true -> chttpd_util:scrub_mochiweb_client_req(Req#httpd.mochi_req); + false -> Req#httpd.mochi_req + end, + Req1 = Req#httpd{mochi_req = MochiReq}, + {[{Field, json_req_obj_field(Field, Req1, Db, DocId)} || Field <- Fields]}. json_req_obj_fields() -> [ @@ -103,7 +109,10 @@ json_req_obj_field(<<"form">>, #httpd{mochi_req = Req, method = Method} = HttpRe end, to_json_terms(ParsedForm); json_req_obj_field(<<"cookie">>, #httpd{mochi_req = Req}, _Db, _DocId) -> - to_json_terms(Req:parse_cookie()); + case scrub_json_request() of + true -> {[]}; + false -> to_json_terms(Req:parse_cookie()) + end; json_req_obj_field(<<"userCtx">>, #httpd{}, Db, _DocId) -> couch_util:json_user_ctx(Db); json_req_obj_field(<<"secObj">>, #httpd{user_ctx = UserCtx}, Db, _DocId) -> @@ -216,3 +225,6 @@ default_or_content_type(DefaultContentType, Headers) -> true -> Headers end. + +scrub_json_request() -> + config:get_boolean("chttpd", "scrub_json_request", true). diff --git a/src/chttpd/src/chttpd_util.erl b/src/chttpd/src/chttpd_util.erl index b03bba0b1..b0dacf651 100644 --- a/src/chttpd/src/chttpd_util.erl +++ b/src/chttpd/src/chttpd_util.erl @@ -23,6 +23,7 @@ get_chttpd_auth_config_boolean/2, maybe_add_csp_header/3, get_db_info/1, + scrub_mochiweb_client_req/1, mochiweb_client_req_set/1, mochiweb_client_req_clean/0, mochiweb_client_req_get/0, @@ -121,20 +122,22 @@ get_db_info(DbName) -> _Tag:Error -> {error, Error} end. -mochiweb_client_req_set(ClientReq) -> +scrub_mochiweb_client_req(ClientReq) -> Method = mochiweb_request:get(method, ClientReq), Socket = mochiweb_request:get(socket, ClientReq), Path = mochiweb_request:get(raw_path, ClientReq), Version = mochiweb_request:get(version, ClientReq), Opts = mochiweb_request:get(opts, ClientReq), Headers = mochiweb_request:get(headers, ClientReq), - % Remove any sensitive info in case process dict gets dumped - % to the logs at some point Headers1 = mochiweb_headers:delete_any("Authorization", Headers), Headers2 = mochiweb_headers:delete_any("Cookie", Headers1), Headers3 = mochiweb_headers:delete_any("X-Auth-CouchDB-Token", Headers2), - ClientReq1 = mochiweb_request:new(Socket, Opts, Method, Path, Version, Headers3), - put(?MOCHIWEB_CLIENT_REQ, ClientReq1). + mochiweb_request:new(Socket, Opts, Method, Path, Version, Headers3). + +mochiweb_client_req_set(ClientReq) -> + % Remove any sensitive info in case process dict gets dumped + % to the logs at some point + put(?MOCHIWEB_CLIENT_REQ, scrub_mochiweb_client_req(ClientReq)). mochiweb_client_req_clean() -> erase(?MOCHIWEB_CLIENT_REQ). diff --git a/src/chttpd/test/eunit/chttpd_external_test.erl b/src/chttpd/test/eunit/chttpd_external_test.erl index cd691fbaa..d8b691486 100644 --- a/src/chttpd/test/eunit/chttpd_external_test.erl +++ b/src/chttpd/test/eunit/chttpd_external_test.erl @@ -28,14 +28,21 @@ setup_mock() -> teardown_mock(_) -> meck:unload(). +headers() -> + [ + {"host", "example.com"}, + {"AutHoriZatioN", "Basic s3cr3t"}, + {"COOkiE", "cookie1=val1; cookie2=val2"}, + {"x-AUth-CouchDB-TokeN", "S3cr3tT0k3n"} + ]. + setup_local_httpd_req() -> ok = meck:new(mochiweb, [passthrough]), ok = meck:expect(mochiweb_socket, peername, fun(_) -> {ok, {{127, 0, 0, 1}, 5984}} end), ok = meck:expect(mochiweb_request, recv_body, 2, {[{<<"a">>, 42}]}), - Headers = mochiweb_headers:make([{"host", "example.com"}]), - MochiReq = mochiweb_request:new(nil, 'GET', "/", {1, 1}, Headers), + MochiReq = mochiweb:new_request({nil, {'GET', "/", {1, 1}}, headers()}), #httpd{ mochi_req = MochiReq, method = 'GET', @@ -45,8 +52,7 @@ setup_local_httpd_req() -> }. setup_remote_httpd_req() -> - Headers = mochiweb_headers:make([{"host", "example.com"}]), - MochiReq = mochiweb_request:new(nil, 'GET', "/", {1, 1}, Headers), + MochiReq = mochiweb:new_request({nil, {'GET', "/", {1, 1}}, headers()}), #httpd{ mochi_req = MochiReq, method = 'GET', @@ -67,7 +73,10 @@ json_req_obj_local_httpd_req_test_() -> { setup, fun setup_local_httpd_req/0, - fun should_convert_req_to_json_obj/1 + with([ + ?TDEF(should_convert_req_to_json_obj_not_scrubbed), + ?TDEF(should_convert_req_to_json_obj_scrubbed) + ]) } } }. @@ -82,22 +91,59 @@ json_req_obj_remote_httpd_req_test_() -> { setup, fun setup_remote_httpd_req/0, - fun should_convert_req_to_json_obj/1 + with([ + ?TDEF(should_convert_req_to_json_obj_not_scrubbed), + ?TDEF(should_convert_req_to_json_obj_scrubbed) + ]) } } }. -should_convert_req_to_json_obj(HttpdReq) -> - Expect = expect(), +should_convert_req_to_json_obj_not_scrubbed(HttpdReq) -> + meck:expect(config, get_boolean, fun("chttpd", "scrub_json_request", _) -> false end), + Expect = expect(expect_headers_not_scrubbed(), expect_cookie_not_scrubbed()), + {Result} = chttpd_external:json_req_obj(HttpdReq, <<"fake">>), + lists:map( + fun({K, V}) -> + {K, ?assertEqual(couch_util:get_value(K, Expect), V)} + end, + Result + ). + +should_convert_req_to_json_obj_scrubbed(HttpdReq) -> + meck:expect(config, get_boolean, fun("chttpd", "scrub_json_request", _) -> true end), + Expect = expect(expect_headers_scrubbed(), expect_cookie_scrubbed()), {Result} = chttpd_external:json_req_obj(HttpdReq, <<"fake">>), lists:map( fun({K, V}) -> - {K, ?_assertEqual(couch_util:get_value(K, Expect), V)} + {K, ?assertEqual(couch_util:get_value(K, Expect), V)} end, Result ). -expect() -> +expect_headers_not_scrubbed() -> + {[ + {<<"AutHoriZatioN">>, <<"Basic s3cr3t">>}, + {<<"COOkiE">>, <<"cookie1=val1; cookie2=val2">>}, + {<<"host">>, <<"example.com">>}, + {<<"x-AUth-CouchDB-TokeN">>, <<"S3cr3tT0k3n">>} + ]}. + +expect_headers_scrubbed() -> + {[ + {<<"host">>, <<"example.com">>} + ]}. + +expect_cookie_not_scrubbed() -> + {[ + {<<"cookie1">>, <<"val1">>}, + {<<"cookie2">>, <<"val2">>} + ]}. + +expect_cookie_scrubbed() -> + {[]}. + +expect({[_ | _]} = Headers, Cookie) -> [ {<<"info">>, {[{name, <<"fake">>}]}}, {<<"uuid">>, <<"4">>}, @@ -107,11 +153,11 @@ expect() -> {<<"path">>, [<<"/">>]}, {<<"raw_path">>, <<"/">>}, {<<"query">>, {[]}}, - {<<"headers">>, {[{<<"host">>, <<"example.com">>}]}}, + {<<"headers">>, Headers}, {<<"body">>, {[{<<"a">>, 42}]}}, {<<"peer">>, <<"127.0.0.1">>}, {<<"form">>, {[]}}, - {<<"cookie">>, {[]}}, + {<<"cookie">>, Cookie}, {<<"userCtx">>, {[ {<<"db">>, <<"fake">>},
