From 60a6e1af86da1c69f0bb7be95cc590e2b7676dc7 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 25 Oct 2026 20:45:27 +0100 Subject: [PATCH] Share the redirect origin check, NEWS for #978 and #979 hackney.erl and hackney_h3.erl each had their own same_origin/2; both redirect paths now use hackney_url:same_origin/2. The test-only export of maybe_strip_auth_on_redirect/3 is gone: the origin cases are unit tests on hackney_url, and location_trusted is covered end to end. --- NEWS.md | 17 ++++++- src/hackney.erl | 11 +---- src/hackney_h3.erl | 10 +--- src/hackney_url.erl | 17 +++++++ test/hackney_redirect_tests.erl | 81 ++++++--------------------------- test/hackney_url_tests.erl | 19 ++++++++ 6 files changed, 68 insertions(+), 87 deletions(-) diff --git a/NEWS.md b/NEWS.md index 344f2045..c89d32e9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,7 +12,22 @@ Unreleased as in hackney 1.x (#973). - A failed TLS handshake on a pooled connection no longer frees a `max_per_host` slot held by another request. The slot was released twice, - so the host could get one more concurrent connection than allowed. + so the host could get one more concurrent connection than allowed (#984). +- Pinning your own CA with `cacerts` or `cacertfile` now actually limits + which servers are trusted. hackney still accepted any certificate chain + that included a publicly trusted root, so a pinned connection could reach + a server its CA never vouched for (#978). +- Following a redirect no longer resends `basic_auth` and `cookie` to + another port on the same host, or over plain HTTP after HTTPS. Only a + redirect to the same scheme, host and port keeps them, unless you set + `location_trusted` (#979). + +### Changed + +- If you pin `cacerts` that include the bundled public roots, hackney no + longer falls back to trusting a cross-signed root it finds in the + server's chain. Add the root you need to your list, or pass your own + `partial_chain` (#978). 4.8.7 - 2026-10-25 ------------------ diff --git a/src/hackney.erl b/src/hackney.erl index 39aaf0eb..6e90c4fc 100644 --- a/src/hackney.erl +++ b/src/hackney.erl @@ -69,7 +69,6 @@ -export([get_proxy_config/3]). -export([check_no_proxy/2]). -export([start_conn_with_socket/5]). --export([maybe_strip_auth_on_redirect/3]). -endif. -define(METHOD_TPL(Method), @@ -1633,7 +1632,7 @@ find_last_slash(Bin, Pos) -> %% @private Strip sensitive auth options when redirecting to a different origin. %% This prevents credential leakage per CVE-2018-1000007. The origin is the -%% {scheme, host, port} triple (as in hackney_h3:same_origin/2, GHSA-h73q): +%% {scheme, host, port} triple (hackney_url:same_origin/2, GHSA-h73q): %% comparing the host alone let an https -> http redirect resend basic_auth %% and cookie in cleartext, and a redirect to another port hand them to a %% different service on the same host. @@ -1645,7 +1644,7 @@ maybe_strip_auth_on_redirect(CurrentURL, NewURL, Options) -> %% User explicitly allows forwarding auth to different origins Options; false -> - case same_origin(CurrentURL, NewURL) of + case hackney_url:same_origin(CurrentURL, NewURL) of true -> %% Same origin - keep auth options Options; @@ -1659,12 +1658,6 @@ maybe_strip_auth_on_redirect(CurrentURL, NewURL, Options) -> end end. -%% @private Same scheme, host (case-insensitive) and port. -same_origin(#hackney_url{scheme = SchemeA, host = HostA, port = PortA}, - #hackney_url{scheme = SchemeB, host = HostB, port = PortB}) -> - {SchemeA, string:to_lower(HostA), PortA} =:= - {SchemeB, string:to_lower(HostB), PortB}. - async_request(ConnPid, Method, Path, Headers, Body, AsyncMode, StreamTo, FollowRedirect, Options) -> %% Handle body encoding {FinalHeaders, FinalBody} = encode_body(Headers, Body, []), diff --git a/src/hackney_h3.erl b/src/hackney_h3.erl index 26e6907a..394d2d6b 100644 --- a/src/hackney_h3.erl +++ b/src/hackney_h3.erl @@ -227,20 +227,12 @@ maybe_strip_redirect_headers(OldUrl, NewUrl, Headers, Opts) -> case maps:get(location_trusted, Opts, false) of true -> Headers; false -> - case same_origin(OldUrl, NewUrl) of + case hackney_url:same_origin(OldUrl, NewUrl) of true -> Headers; false -> drop_credential_headers(Headers) end end. -same_origin(UrlA, UrlB) -> - #hackney_url{scheme = SchemeA, host = HostA, port = PortA} = - hackney_url:parse_url(UrlA), - #hackney_url{scheme = SchemeB, host = HostB, port = PortB} = - hackney_url:parse_url(UrlB), - {SchemeA, string:to_lower(HostA), PortA} =:= - {SchemeB, string:to_lower(HostB), PortB}. - drop_credential_headers(Headers) -> Sensitive = [<<"authorization">>, <<"cookie">>, <<"proxy-authorization">>], lists:filter( diff --git a/src/hackney_url.erl b/src/hackney_url.erl index 8e8f3774..bb3bc7c0 100644 --- a/src/hackney_url.erl +++ b/src/hackney_url.erl @@ -26,6 +26,7 @@ -export([idnconvert_hostname/1]). -export([is_ip_literal/1]). +-export([same_origin/2]). -include("hackney_lib.hrl"). @@ -242,6 +243,22 @@ fold_dot(16#FF0E) -> $.; fold_dot(16#FF61) -> $.; fold_dot(C) -> C. +%% @doc True when both URLs share an origin: the same scheme, host (compared +%% case-insensitively) and port. Redirects to another origin drop credentials +%% (CVE-2018-1000007, GHSA-h73q). Takes parsed URLs or URLs to parse; the +%% default port is filled in by parse_url/1, so an explicit default port is +%% the same origin as none. +-spec same_origin(hackney_url() | binary() | string(), + hackney_url() | binary() | string()) -> boolean(). +same_origin(#hackney_url{scheme = SchemeA, host = HostA, port = PortA}, + #hackney_url{scheme = SchemeB, host = HostB, port = PortB}) -> + {SchemeA, string:lowercase(HostA), PortA} =:= + {SchemeB, string:lowercase(HostB), PortB}; +same_origin(#hackney_url{} = UrlA, UrlB) -> + same_origin(UrlA, parse_url(UrlB)); +same_origin(UrlA, UrlB) -> + same_origin(parse_url(UrlA), UrlB). + %% @doc True when Host is an IPv4/IPv6 literal. RFC 6066 forbids sending SNI %% for IP literals. Hosts reach the TLS layer bracket-stripped, but strip a %% stray bracket pair defensively. diff --git a/test/hackney_redirect_tests.erl b/test/hackney_redirect_tests.erl index acb4452a..f0d769ef 100644 --- a/test/hackney_redirect_tests.erl +++ b/test/hackney_redirect_tests.erl @@ -209,7 +209,9 @@ redirect_integration_test_() -> {"same-origin redirect keeps basic_auth and cookie", fun test_same_origin_redirect_keeps_auth/0}, {"same-host other-port redirect strips basic_auth and cookie", - fun test_other_port_redirect_strips_auth/0} + fun test_other_port_redirect_strips_auth/0}, + {"location_trusted keeps basic_auth and cookie on another origin", + fun test_location_trusted_redirect_keeps_auth/0} ]}. test_absolute_redirect_follow() -> @@ -269,8 +271,11 @@ auth_opts() -> %% /get echoes the request headers as JSON. redirected_request_headers(Target) -> + redirected_request_headers(Target, []). + +redirected_request_headers(Target, Extra) -> URL = url(<<"/redirect-to?url=", (hackney_url:urlencode(Target))/binary>>), - {ok, 200, _Headers, Body} = hackney:request(get, URL, [], <<>>, auth_opts()), + {ok, 200, _Headers, Body} = hackney:request(get, URL, [], <<>>, Extra ++ auth_opts()), #{<<"headers">> := Seen} = jsx:decode(Body, [return_maps]), Seen. @@ -286,72 +291,12 @@ test_other_port_redirect_strips_auth() -> ?assertNot(maps:is_key(<<"authorization">>, Seen)), ?assertNot(maps:is_key(<<"cookie">>, Seen)). -%% ============================================================================= -%% Tests for CVE-2018-1000007 - auth stripping on cross-origin redirects -%% ============================================================================= - -%% Test that auth options are stripped on cross-origin redirects by default -auth_strip_on_cross_host_redirect_test_() -> - BaseURL = hackney_url:parse_url(<<"https://api.example.com/resource">>), - SameHostURL = hackney_url:parse_url(<<"https://api.example.com/other">>), - SameOriginCaseURL = hackney_url:parse_url(<<"https://API.Example.com:443/other">>), - DiffHostURL = hackney_url:parse_url(<<"https://s3.amazonaws.com/bucket/file">>), - DowngradeURL = hackney_url:parse_url(<<"http://api.example.com/resource">>), - DiffPortURL = hackney_url:parse_url(<<"https://api.example.com:8443/resource">>), - OptionsWithAuth = [{basic_auth, {"user", "pass"}}, {cookie, "session=abc"}, {timeout, 5000}], - Strip = fun hackney:maybe_strip_auth_on_redirect/3, - - [ - {"same host keeps auth options", - fun() -> - Result = Strip(BaseURL, SameHostURL, OptionsWithAuth), - ?assert(proplists:is_defined(basic_auth, Result)), - ?assert(proplists:is_defined(cookie, Result)), - ?assert(proplists:is_defined(timeout, Result)) - end}, - {"host case and explicit default port are the same origin", - fun() -> - Result = Strip(BaseURL, SameOriginCaseURL, OptionsWithAuth), - ?assert(proplists:is_defined(basic_auth, Result)), - ?assert(proplists:is_defined(cookie, Result)) - end}, - {"different host strips auth options", - fun() -> - Result = Strip(BaseURL, DiffHostURL, OptionsWithAuth), - ?assertNot(proplists:is_defined(basic_auth, Result)), - ?assertNot(proplists:is_defined(cookie, Result)), - %% Non-auth options should be kept - ?assert(proplists:is_defined(timeout, Result)) - end}, - {"https -> http on the same host strips auth options", - fun() -> - Result = Strip(BaseURL, DowngradeURL, OptionsWithAuth), - ?assertNot(proplists:is_defined(basic_auth, Result)), - ?assertNot(proplists:is_defined(cookie, Result)), - ?assert(proplists:is_defined(timeout, Result)) - end}, - {"different port on the same host strips auth options", - fun() -> - Result = Strip(BaseURL, DiffPortURL, OptionsWithAuth), - ?assertNot(proplists:is_defined(basic_auth, Result)), - ?assertNot(proplists:is_defined(cookie, Result)), - ?assert(proplists:is_defined(timeout, Result)) - end}, - {"location_trusted keeps auth on different host", - fun() -> - OptionsWithTrusted = [{location_trusted, true} | OptionsWithAuth], - Result = Strip(BaseURL, DiffHostURL, OptionsWithTrusted), - ?assert(proplists:is_defined(basic_auth, Result)), - ?assert(proplists:is_defined(cookie, Result)) - end}, - {"location_trusted keeps auth on https -> http", - fun() -> - OptionsWithTrusted = [{location_trusted, true} | OptionsWithAuth], - Result = Strip(BaseURL, DowngradeURL, OptionsWithTrusted), - ?assert(proplists:is_defined(basic_auth, Result)), - ?assert(proplists:is_defined(cookie, Result)) - end} - ]. +%% location_trusted opts back into forwarding credentials to another origin, +%% like curl's --location-trusted. +test_location_trusted_redirect_keeps_auth() -> + Seen = redirected_request_headers(other_url(<<"/get">>), [{location_trusted, true}]), + ?assertMatch(#{<<"authorization">> := <<"Basic ", _/binary>>}, Seen), + ?assertEqual(<<"sid=abc">>, maps:get(<<"cookie">>, Seen)). %% ============================================================================= %% Tests for HTTP 1xx Informational Responses (issue #631) diff --git a/test/hackney_url_tests.erl b/test/hackney_url_tests.erl index 35c7c7ad..a9b4365d 100644 --- a/test/hackney_url_tests.erl +++ b/test/hackney_url_tests.erl @@ -646,3 +646,22 @@ parse_url_does_not_intern_attacker_schemes_test() -> %% Allow a small slack for any unrelated atoms minted during the loop; %% a regression would mint ~200 (one per unique scheme). ?assert(Delta < 20). + +%% Redirects keep credentials only within one origin (CVE-2018-1000007, +%% GHSA-h73q): same scheme, host and port. +same_origin_test_() -> + Base = <<"https://api.example.com/resource">>, + [{"same scheme, host and port", + ?_assert(hackney_url:same_origin(Base, <<"https://api.example.com/other">>))}, + {"host case and explicit default port are the same origin", + ?_assert(hackney_url:same_origin(Base, <<"https://API.Example.com:443/other">>))}, + {"another host is another origin", + ?_assertNot(hackney_url:same_origin(Base, <<"https://s3.amazonaws.com/bucket">>))}, + {"https -> http on the same host is another origin", + ?_assertNot(hackney_url:same_origin(Base, <<"http://api.example.com/resource">>))}, + {"another port on the same host is another origin", + ?_assertNot(hackney_url:same_origin(Base, <<"https://api.example.com:8443/resource">>))}, + {"parsed and unparsed URLs compare alike", + ?_assert(hackney_url:same_origin(hackney_url:parse_url(Base), Base))}, + {"string URLs are accepted", + ?_assert(hackney_url:same_origin("https://api.example.com/a", Base))}].