Share the redirect origin check, NEWS for #978 and #979

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
 ------------------

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, []),

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(

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.

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)

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))}].