From f703a477b1fdd7a320dd84eecfd998c67a7a2f57 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Thu, 18 Jun 2026 11:07:57 +0200 Subject: [PATCH 1/4] Fix HTTP/2-on-HTTP/1 mislabel on resumed TLS sessions and the recv_status spin On a resumed TLS 1.3 session ssl:negotiated_protocol/1 reports nothing, so a reused HTTP/2 connection was labeled HTTP/1, fed h2 frames to the h1 parser, and recv_status looped forever on an unconsumable buffer (one CPU pegged). Carry the ALPN protocol across resumption: a per host and advertised-ALPN memo records the protocol learned on a full handshake; session_tickets is gated on that memo so resumption is offered only once the protocol is known, and a resumed session resolves against the gate-time snapshot only when it actually resumed. session_tickets is excluded from the pool key so the gate does not churn buckets. Guard recv_status: read from the socket on {more} and fail fast with {error, {bad_response, not_http}} when the bytes cannot start an HTTP/1 status line, instead of spinning. --- src/hackney_conn.erl | 114 +++++++++++++++++++++-------- src/hackney_ssl.erl | 103 ++++++++++++++++++++++++-- test/hackney_recv_status_tests.erl | 82 +++++++++++++++++++++ test/hackney_ssl_alpn_tests.erl | 96 ++++++++++++++++++++++++ test/hackney_ssl_tests.erl | 18 +++-- 5 files changed, 366 insertions(+), 47 deletions(-) create mode 100644 test/hackney_recv_status_tests.erl create mode 100644 test/hackney_ssl_alpn_tests.erl diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl index 083e7782..a591f7a7 100644 --- a/src/hackney_conn.erl +++ b/src/hackney_conn.erl @@ -898,10 +898,15 @@ connected({call, From}, {upgrade_to_ssl, SslOpts, UpgradeOpts}, #conn_data{socke end, hackney_util:merge_opts(MergedSslOpts, AlpnOpts) end, - case ssl:connect(Socket, FinalSslOpts) of + %% Gate TLS resumption on the ALPN memo and snapshot the cached protocol, so a + %% resumed session (where ssl reports no ALPN) resolves against this snapshot. + AlpnProtos = alpn_advertised(FinalSslOpts), + Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), + GatedSslOpts = gate_resumption(FinalSslOpts, Cached), + case ssl:connect(Socket, GatedSslOpts) of {ok, SslSocket} -> - %% Detect negotiated protocol - Protocol = hackney_ssl:get_negotiated_protocol(SslSocket), + %% Detect negotiated protocol, carrying ALPN across resumption + Protocol = hackney_ssl:negotiated_protocol(SslSocket, Host, AlpnProtos, Cached), %% Update connection to use SSL NewData = Data#conn_data{ transport = hackney_ssl, @@ -2183,21 +2188,22 @@ recv_status_and_headers_loop(Data) -> recv_status(#conn_data{parser = Parser, buffer = Buffer} = Data) -> case hackney_http:execute(Parser, Buffer) of {more, NewParser} -> - %% Check if parser has data in its internal buffer (e.g., after skipping 1XX response) - %% If so, continue parsing; otherwise read from socket - ParserBuffer = hackney_http:get(NewParser, buffer), - case ParserBuffer of - <<>> -> - %% Parser buffer empty - need to read from socket + %% The parser needs more bytes to complete the status line. It already + %% consumed what it could, so re-feeding its own buffer makes no + %% progress (that was an infinite spin); always read from the socket. + %% But a valid HTTP/1 status line starts with "HTTP/": if what we have + %% can never be that (e.g. an HTTP/2 frame on a mislabeled connection), + %% fail fast instead of reading until recv_timeout. + case maybe_http_status_start(hackney_http:get(NewParser, buffer)) of + false -> + {error, {bad_response, not_http}}; + true -> case recv_data(Data) of {ok, RecvData} -> recv_status(Data#conn_data{parser = NewParser, buffer = RecvData}); {error, Reason} -> {error, Reason} - end; - _ -> - %% Parser has data in buffer - continue parsing without reading - recv_status(Data#conn_data{parser = NewParser, buffer = <<>>}) + end end; {response, Version, Status, Reason, NewParser} -> recv_headers(Data#conn_data{ @@ -2211,6 +2217,26 @@ recv_status(#conn_data{parser = Parser, buffer = Buffer} = Data) -> {error, Reason} end. +%% @private Whether the buffered bytes can still be the start of an HTTP/1 status +%% line. Leading blank lines are tolerated (RFC 7230 3.5). True while the buffer is +%% empty or shares a common prefix with "HTTP/" up to the shorter length, so both a +%% still-accumulating version ("HTT") and a longer partial line ("HTTP/1.1 2") pass; +%% false once it diverges (e.g. an HTTP/2 frame), so a protocol mismatch fails fast. +maybe_http_status_start(Buffer) -> + case strip_leading_eol(Buffer) of + <<>> -> + true; + Bin -> + Prefix = <<"HTTP/">>, + N = min(byte_size(Bin), byte_size(Prefix)), + binary:part(Bin, 0, N) =:= binary:part(Prefix, 0, N) + end. + +%% @private Drop leading CR/LF bytes (lenient about blank lines before the status line). +strip_leading_eol(<<"\r", Rest/binary>>) -> strip_leading_eol(Rest); +strip_leading_eol(<<"\n", Rest/binary>>) -> strip_leading_eol(Rest); +strip_leading_eol(Bin) -> Bin. + recv_headers(#conn_data{parser = Parser} = Data, Headers) -> case hackney_http:execute(Parser) of {more, NewParser} -> @@ -2687,7 +2713,7 @@ do_tcp_connect(From, Data) -> } = Data, %% Filter out hackney-specific options that are not valid for transport TransportOpts = proplists:delete(protocols, ConnectOpts), - Opts = case Transport of + case Transport of hackney_ssl -> %% effective_opts is the single builder of ssl:connect options for %% both default and custom ssl_options. It resolves SNI and ALPN @@ -2697,27 +2723,51 @@ do_tcp_connect(From, Data) -> undefined -> SslOpts0; Protocols -> [{protocols, Protocols} | SslOpts0] end, - FinalSslOpts = hackney_ssl:effective_opts(Host, SslOpts1, ConnectOpts), - TransportOpts ++ [{ssl_options, FinalSslOpts}]; - _ -> TransportOpts - end, - case Transport:connect(Host, Port, Opts, Timeout) of - {ok, Socket} -> - Protocol = case Transport of - hackney_ssl -> hackney_ssl:get_negotiated_protocol(Socket); - _ -> http1 - end, - case Protocol of - http2 -> - init_h2_connection(Socket, Data#conn_data{socket = Socket, protocol = http2}, From); - http1 -> - NewData = Data#conn_data{socket = Socket, protocol = http1}, - {next_state, connected, NewData, [{reply, From, ok}]} + FinalSslOpts0 = hackney_ssl:effective_opts(Host, SslOpts1, ConnectOpts), + %% Gate TLS resumption on the ALPN memo and snapshot the cached + %% protocol now, so a resumed session (where ssl reports no ALPN) is + %% resolved against this snapshot rather than re-read from the memo. + AlpnProtos = alpn_advertised(FinalSslOpts0), + Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), + FinalSslOpts = gate_resumption(FinalSslOpts0, Cached), + Opts = TransportOpts ++ [{ssl_options, FinalSslOpts}], + case Transport:connect(Host, Port, Opts, Timeout) of + {ok, Socket} -> + case hackney_ssl:negotiated_protocol(Socket, Host, AlpnProtos, Cached) of + http2 -> + init_h2_connection(Socket, + Data#conn_data{socket = Socket, protocol = http2}, From); + http1 -> + {next_state, connected, + Data#conn_data{socket = Socket, protocol = http1}, + [{reply, From, ok}]} + end; + {error, Reason} -> + {stop_and_reply, normal, [{reply, From, {error, Reason}}]} end; - {error, Reason} -> - {stop_and_reply, normal, [{reply, From, {error, Reason}}]} + _ -> + case Transport:connect(Host, Port, TransportOpts, Timeout) of + {ok, Socket} -> + {next_state, connected, + Data#conn_data{socket = Socket, protocol = http1}, + [{reply, From, ok}]}; + {error, Reason} -> + {stop_and_reply, normal, [{reply, From, {error, Reason}}]} + end end. +%% @private The advertised ALPN protocol list (in offered order) from ssl opts, +%% or [] when none is offered. Used as part of the ALPN memo key. +alpn_advertised(SslOpts) -> + proplists:get_value(alpn_advertised_protocols, SslOpts, []). + +%% @private Gate TLS resumption on the ALPN memo: keep `session_tickets' (offer +%% resumption) only once a full handshake has cached this host+ALPN's protocol. +%% A cold memo (`none') strips it so the handshake is full and reports ALPN, which +%% repopulates the memo. Keeps a resumed session from losing the protocol. +gate_resumption(SslOpts, none) -> proplists:delete(session_tickets, SslOpts); +gate_resumption(SslOpts, _Cached) -> SslOpts. + %% @private Initialize HTTP/2 connection via the h2 library. %% The h2_connection process takes ownership of the socket and delivers %% owner messages ({h2, Conn, Event}) to this gen_statem's mailbox. diff --git a/src/hackney_ssl.erl b/src/hackney_ssl.erl index b66275e4..89280b36 100644 --- a/src/hackney_ssl.erl +++ b/src/hackney_ssl.erl @@ -32,9 +32,16 @@ -define(TLS_KEY_CACHE, hackney_tls_keys). -define(TLS_KEY_CACHE_MAX, 512). +%% Per {Host, advertised-ALPN} memo of the protocol learned on a full handshake, +%% so a resumed TLS session (where ssl:negotiated_protocol/1 reports nothing) is +%% labeled correctly. See resolve_alpn/5 and the resumption gate in hackney_conn. +-define(ALPN_CACHE, hackney_alpn_protocols). +-define(ALPN_CACHE_MAX, 4096). + %% ALPN (Application-Layer Protocol Negotiation) for HTTP/2 -export([alpn_opts/1]). -export([get_negotiated_protocol/1]). +-export([negotiated_protocol/4, resolve_alpn/5, recall_alpn/2]). %% @doc Atoms used to identify messages in {active, once | true} mode. messages(_) -> {ssl, ssl_closed, ssl_error}. @@ -152,15 +159,19 @@ tlsv13_allowed() -> {ok, _} -> false end. -%% @doc Create the TLS key memo table used by `effective_opts_and_key/3'. -%% Idempotent; called from hackney_sup:init/1. +%% @doc Create the TLS key memo table used by `effective_opts_and_key/3' and the +%% per-host ALPN memo used by `resolve_alpn/5'. Idempotent; called from +%% hackney_sup:init/1. -spec init_key_cache() -> ok. init_key_cache() -> - case ets:info(?TLS_KEY_CACHE) of + ok = ensure_table(?TLS_KEY_CACHE), + ok = ensure_table(?ALPN_CACHE), + ok. + +ensure_table(Name) -> + case ets:info(Name) of undefined -> - ?TLS_KEY_CACHE = ets:new(?TLS_KEY_CACHE, - [set, public, named_table, - {read_concurrency, true}]), + Name = ets:new(Name, [set, public, named_table, {read_concurrency, true}]), ok; _ -> ok @@ -211,8 +222,13 @@ env_fingerprint() -> %% lookup semantics) so option order does not change the key, while %% conflicting duplicates such as `[{verify,A},{verify,B}]' and its %% reverse still hash differently. +%% +%% `session_tickets' is excluded (like the HTTP/3 key excludes `session_ticket'): +%% it is identity-neutral (does not affect trust) and the handshake now varies it +%% per the ALPN resumption gate, so it must not change the pool bucket. -spec options_key(list()) -> binary(). -options_key(FinalSslOpts) -> +options_key(FinalSslOpts0) -> + FinalSslOpts = proplists:delete(session_tickets, FinalSslOpts0), {Tuples, Rest} = lists:partition( fun(T) -> is_tuple(T) andalso tuple_size(T) =:= 2 end, FinalSslOpts), @@ -493,6 +509,79 @@ get_negotiated_protocol(SslSocket) -> _ -> http1 end. +%% @doc Resolve the negotiated protocol after a handshake, carrying the ALPN memo +%% across TLS resumption. On a resumed session ssl:negotiated_protocol/1 reports +%% nothing, so we fall back to `Cached' (snapshotted before the handshake), but +%% only when the session actually resumed - a full handshake that reports no ALPN +%% is a genuine HTTP/1 conn. `Cached' is `recall_alpn(Host, AlpnProtos)' read at +%% the resumption gate. +-spec negotiated_protocol(ssl:sslsocket(), string(), list(), http2 | http1 | none) -> + http2 | http1. +negotiated_protocol(SslSocket, Host, AlpnProtos, Cached) -> + resolve_alpn(ssl:negotiated_protocol(SslSocket), resumed(SslSocket), + Cached, Host, AlpnProtos). + +%% @doc Pure ALPN decision (exported for tests). See negotiated_protocol/4. +-spec resolve_alpn({ok, binary()} | {error, term()}, boolean(), + http2 | http1 | none, string(), list()) -> http2 | http1. +resolve_alpn({ok, <<"h2">>}, _Resumed, _Cached, Host, AlpnProtos) -> + remember_alpn(Host, AlpnProtos, http2), + http2; +resolve_alpn({ok, <<"http/1.1">>}, _Resumed, _Cached, Host, AlpnProtos) -> + remember_alpn(Host, AlpnProtos, http1), + http1; +resolve_alpn({error, protocol_not_negotiated}, true, Cached, _Host, _AlpnProtos) + when Cached =/= none -> + %% Genuinely resumed: ALPN is not re-reported, use the gate-time snapshot. + Cached; +resolve_alpn({error, protocol_not_negotiated}, false, _Cached, Host, AlpnProtos) -> + %% Full handshake with no ALPN: a real HTTP/1 conn. Refresh the memo so a stale + %% cached http2 cannot be recalled on a later resumption. + remember_alpn(Host, AlpnProtos, http1), + http1; +resolve_alpn(_Other, _Resumed, _Cached, _Host, _AlpnProtos) -> + %% Defensive: resumed without a snapshot (gate should prevent this) or an + %% unexpected ssl:negotiated_protocol/1 result. + http1. + +%% @doc Whether the TLS handshake resumed a session (PSK / abbreviated handshake). +-spec resumed(ssl:sslsocket()) -> boolean(). +resumed(SslSocket) -> + case ssl:connection_information(SslSocket, [session_resumption]) of + {ok, [{session_resumption, R}]} -> R =:= true; + _ -> false + end. + +%% @doc Recall the protocol learned for `{Host, AlpnProtos}' on a full handshake, +%% or `none' if not cached. `AlpnProtos' is the advertised ALPN list in offered +%% order (order is the client's preference and can change negotiation). +-spec recall_alpn(string(), list()) -> http2 | http1 | none. +recall_alpn(Host, AlpnProtos) -> + try ets:lookup(?ALPN_CACHE, {Host, AlpnProtos}) of + [{_, Proto}] -> Proto; + [] -> none + catch + %% Table absent: hackney used as a library without the app started. + error:badarg -> none + end. + +%% @private Cache the protocol for `{Host, AlpnProtos}'. Soft-capped: a generous +%% bound cleared wholesale on overflow. Eviction is not correctness-critical - a +%% cold memo just means the next handshake is full (the gate offers no resumption) +%% and re-learns the protocol. +-spec remember_alpn(string(), list(), http2 | http1) -> ok. +remember_alpn(Host, AlpnProtos, Proto) -> + try + case ets:info(?ALPN_CACHE, size) >= ?ALPN_CACHE_MAX of + true -> _ = ets:delete_all_objects(?ALPN_CACHE); + false -> ok + end, + _ = ets:insert(?ALPN_CACHE, {{Host, AlpnProtos}, Proto}), + ok + catch + error:badarg -> ok + end. + %% @private Convert protocol atom to ALPN protocol identifier -spec proto_to_alpn(http2 | http1 | http11) -> binary(). proto_to_alpn(http2) -> <<"h2">>; diff --git a/test/hackney_recv_status_tests.erl b/test/hackney_recv_status_tests.erl new file mode 100644 index 00000000..2c64760b --- /dev/null +++ b/test/hackney_recv_status_tests.erl @@ -0,0 +1,82 @@ +%%% -*- erlang -*- +%%% +%%% This file is part of hackney released under the Apache 2 license. +%%% See the NOTICE for more information. + +%% Regression for the recv_status no-progress spin: an HTTP/2 frame fed to the +%% HTTP/1 parser (a connection mislabeled as http1) must fail fast instead of +%% looping forever on an unconsumable buffer. +-module(hackney_recv_status_tests). + +-include_lib("eunit/include/eunit.hrl"). + +%% A real HTTP/2 server connection preface SETTINGS frame (length 18, type 4, +%% stream 0) - the bytes observed in the hang trace. +-define(H2_SETTINGS, + <<0,0,18,4,0,0,0,0,0,0,3,0,0,0,128,0,4,0,1,0,0,0,5,0, + 255,255,255,0,0,4,8,0,0,0,0,0,127,255,0,0>>). + +setup() -> + {ok, _} = application:ensure_all_started(hackney), + ok. + +teardown(_) -> + application:stop(hackney), + ok. + +recv_status_test_() -> + {setup, fun setup/0, fun teardown/1, + [ + {"h2 frame on an http1 conn fails fast, no spin", + {timeout, 10, fun h2_frame_fails_fast/0}}, + {"a normal HTTP/1.1 response still parses", + {timeout, 10, fun normal_response_ok/0}} + ]}. + +%% The server replies with an HTTP/2 SETTINGS frame and keeps the socket open, so +%% without the guard recv_status spins (and the eunit timeout fails the test); +%% with it, the request returns a fast {error, {bad_response, _}}. +h2_frame_fails_fast() -> + Port = start_raw_server(?H2_SETTINGS), + Pid = connect_tcp(Port), + ?assertMatch({error, {bad_response, _}}, + hackney_conn:request(Pid, <<"GET">>, <<"/">>, [], <<>>)), + catch hackney_conn:stop(Pid). + +normal_response_ok() -> + Resp = <<"HTTP/1.1 200 OK\r\ncontent-type: text/plain\r\n" + "content-length: 2\r\n\r\nhi">>, + Port = start_raw_server(Resp), + Pid = connect_tcp(Port), + ?assertMatch({ok, 200, _}, hackney_conn:request(Pid, <<"GET">>, <<"/">>, [], <<>>)), + ?assertEqual({ok, <<"hi">>}, hackney_conn:body(Pid)), + catch hackney_conn:stop(Pid). + +%%==================================================================== +%% Helpers +%%==================================================================== + +connect_tcp(Port) -> + ConnOpts = #{host => "127.0.0.1", port => Port, transport => hackney_tcp, + connect_timeout => 5000, recv_timeout => 5000, + connect_options => [], ssl_options => []}, + {ok, Pid} = hackney_conn_sup:start_conn(ConnOpts), + ok = hackney_conn:connect(Pid), + Pid. + +%% Accept one connection, read the request, send Reply, and hold the socket open +%% until told to stop (so the client sees Reply but not a close). +start_raw_server(Reply) -> + Self = self(), + {ok, LSock} = gen_tcp:listen(0, [binary, {active, false}, {reuseaddr, true}]), + {ok, Port} = inet:port(LSock), + spawn_link(fun() -> + {ok, Sock} = gen_tcp:accept(LSock, 5000), + _ = gen_tcp:recv(Sock, 0, 5000), %% consume the request line/headers + ok = gen_tcp:send(Sock, Reply), + Self ! {server_ready, self()}, + receive stop -> ok after 9000 -> ok end, + gen_tcp:close(Sock), + gen_tcp:close(LSock) + end), + Port. diff --git a/test/hackney_ssl_alpn_tests.erl b/test/hackney_ssl_alpn_tests.erl new file mode 100644 index 00000000..c293aab0 --- /dev/null +++ b/test/hackney_ssl_alpn_tests.erl @@ -0,0 +1,96 @@ +%%% -*- erlang -*- +%%% +%%% This file is part of hackney released under the Apache 2 license. +%%% See the NOTICE for more information. + +%% Unit tests for the ALPN-across-resumption memo (hackney_ssl). The memo is a +%% shared named ETS table, so each case uses a distinct host to avoid interference. +-module(hackney_ssl_alpn_tests). + +-include_lib("eunit/include/eunit.hrl"). + +h2() -> [<<"h2">>, <<"http/1.1">>]. +h2_reordered() -> [<<"http/1.1">>, <<"h2">>]. +h1() -> [<<"http/1.1">>]. + +setup() -> + ok = hackney_ssl:init_key_cache(). + +alpn_test_() -> + {setup, fun setup/0, + [ + fun cold_gate/0, + fun learn_h2_then_recall/0, + fun resumed_recall/0, + fun http1_learn_and_recall/0, + fun full_handshake_refreshes_stale_http2/0, + fun carried_snapshot_is_race_free/0, + fun resumed_without_snapshot_defaults_http1/0, + fun key_preserves_order_and_protocol_set/0, + fun options_key_ignores_session_tickets/0 + ]}. + +%% A never-seen host+ALPN is not cached, so the gate would not offer resumption. +cold_gate() -> + ?assertEqual(none, hackney_ssl:recall_alpn("cold.example", h2())). + +%% A full handshake reporting h2 caches it. +learn_h2_then_recall() -> + ?assertEqual(http2, + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "learn.example", h2())), + ?assertEqual(http2, hackney_ssl:recall_alpn("learn.example", h2())). + +%% Core: a genuinely resumed session (no ALPN reported) recalls the carried snapshot. +resumed_recall() -> + ?assertEqual(http2, + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "res.example", h2())), + ?assertEqual(http2, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http2, + "res.example", h2())). + +http1_learn_and_recall() -> + ?assertEqual(http1, + hackney_ssl:resolve_alpn({ok, <<"http/1.1">>}, false, none, "h1.example", h1())), + ?assertEqual(http1, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http1, + "h1.example", h1())). + +%% Resumption-status guard + memo refresh: a full handshake that reports no ALPN is +%% a real http1 even with http2 carried, and it overwrites the stale http2 so a +%% later resumption cannot recall it. +full_handshake_refreshes_stale_http2() -> + ?assertEqual(http2, + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ref.example", h2())), + ?assertEqual(http1, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, false, http2, + "ref.example", h2())), + ?assertEqual(http1, hackney_ssl:recall_alpn("ref.example", h2())). + +%% The carried snapshot is authoritative regardless of current memo contents +%% (closes the gate-vs-resolution eviction race). +carried_snapshot_is_race_free() -> + ?assertEqual(none, hackney_ssl:recall_alpn("race.example", h2())), + ?assertEqual(http2, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http2, + "race.example", h2())). + +%% Resumed but no snapshot (gate should prevent this) defaults to http1, not a guess. +resumed_without_snapshot_defaults_http1() -> + ?assertEqual(http1, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, none, + "ns.example", h2())). + +%% The memo key includes the advertised list in order and the exact protocol set. +key_preserves_order_and_protocol_set() -> + ?assertEqual(http2, + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ord.example", h2())), + ?assertEqual(none, hackney_ssl:recall_alpn("ord.example", h2_reordered())), + ?assertEqual(none, hackney_ssl:recall_alpn("ord.example", h1())). + +%% The pool tls_key must not depend on session_tickets (the handshake varies it). +options_key_ignores_session_tickets() -> + Base = [{verify, verify_none}, + {alpn_advertised_protocols, [<<"h2">>, <<"http/1.1">>]}], + WithTickets = [{session_tickets, auto} | Base], + ?assertEqual(hackney_ssl:options_key(Base), + hackney_ssl:options_key(WithTickets)). diff --git a/test/hackney_ssl_tests.erl b/test/hackney_ssl_tests.erl index 6481129e..57037f5d 100644 --- a/test/hackney_ssl_tests.erl +++ b/test/hackney_ssl_tests.erl @@ -337,9 +337,10 @@ effective_opts_resumption_requires_tlsv13_test() -> application:unset_env(ssl, protocol_version) end. -options_key_differs_on_resumption_test() -> - %% Resumption-enabled and resumption-disabled connections are - %% handshaken differently and must not share pool buckets. +options_key_ignores_resumption_test() -> + %% session_tickets is identity-neutral and excluded from the pool key: the + %% ALPN resumption gate varies it per connection, so resumption-on and + %% resumption-off connections must share the same pool bucket (not churn it). On = hackney_ssl:options_key(hackney_ssl:effective_opts("example.com", [], [])), application:set_env(hackney, tls_session_resumption, false), Off = try @@ -347,7 +348,7 @@ options_key_differs_on_resumption_test() -> after application:unset_env(hackney, tls_session_resumption) end, - ?assertNotEqual(On, Off). + ?assertEqual(On, Off). %%==================================================================== %% effective_opts_and_key memoization Tests (Unit tests) @@ -391,16 +392,17 @@ tls_key_cache_distinct_inputs() -> ?assertEqual(3, ets:info(hackney_tls_keys, size)). tls_key_cache_env_fingerprint() -> - %% A runtime env flip changes the effective options, so the memo must - %% not serve the key cached under the previous env. + %% A runtime env flip that changes the key must not be served stale from the + %% memo. default_protocols drives the advertised ALPN, which is part of the + %% key (unlike session_tickets, which is now excluded), so flip it here. true = ets:delete_all_objects(hackney_tls_keys), {_, On} = hackney_ssl:effective_opts_and_key("example.com", [], []), - application:set_env(hackney, tls_session_resumption, false), + application:set_env(hackney, default_protocols, [http1]), Off = try {_, K} = hackney_ssl:effective_opts_and_key("example.com", [], []), K after - application:unset_env(hackney, tls_session_resumption) + application:unset_env(hackney, default_protocols) end, ?assertNotEqual(On, Off). From ffb5a3600d8e5cf8b07149e631c1c9c804e07088 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Thu, 18 Jun 2026 11:09:31 +0200 Subject: [PATCH 2/4] Add 4.4.5 changelog entry --- NEWS.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/NEWS.md b/NEWS.md index 56ec8ce1..994df928 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,27 @@ # NEWS +4.4.5 - 2026-06-18 +------------------ + +### Fixed + +- HTTPS: a connection reused over a resumed TLS 1.3 session is no longer + mislabeled as HTTP/1 when it negotiated HTTP/2. `ssl:negotiated_protocol/1` + reports nothing on a resumed session, so hackney now remembers the protocol + learned on the full handshake (per host and advertised ALPN) and offers + resumption only once that protocol is known, resolving a resumed session + against that snapshot. Reused h2 connections take the h2 path instead of + feeding h2 frames to the HTTP/1 parser. +- HTTP/1.1: a response that cannot begin an HTTP/1 status line (for example an + HTTP/2 frame on a mislabeled connection) now fails fast with + `{error, {bad_response, not_http}}` instead of spinning the CPU in the + status-line parser. +- Connection pooling: `Connection: close` responses are no longer returned to + the pool on the sync body path; checkin only pools connections proven + keep-alive and socket-ready (unknown defaults to close); and a closed pooled + entry is discarded at checkout instead of being redialed inside the pool + process (#888). + 4.4.4 - 2026-06-17 ------------------ From a1fbeeabb778ffd458c19df637adb56876088caa Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Thu, 18 Jun 2026 11:30:35 +0200 Subject: [PATCH 3/4] Scope ALPN memo to the resumable ticket source A custom-ssl_options handshake (not resumption-eligible, does not seed the ticket store) could overwrite the shared {Host, AlpnProtos} ALPN entry, so a later default resumed session read a poisoned snapshot and spoke the wrong protocol. Only handshakes hackney enabled resumption for (session_tickets in the effective opts, i.e. the default config, the single ticket source per host) now update the memo; non-resumable handshakes use their own reported ALPN and leave the entry untouched. --- src/hackney_conn.erl | 16 +++++++++-- src/hackney_ssl.erl | 46 +++++++++++++++++++------------- test/hackney_ssl_alpn_tests.erl | 47 ++++++++++++++++++++++++--------- 3 files changed, 76 insertions(+), 33 deletions(-) diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl index a591f7a7..1688ce5b 100644 --- a/src/hackney_conn.erl +++ b/src/hackney_conn.erl @@ -900,13 +900,16 @@ connected({call, From}, {upgrade_to_ssl, SslOpts, UpgradeOpts}, #conn_data{socke end, %% Gate TLS resumption on the ALPN memo and snapshot the cached protocol, so a %% resumed session (where ssl reports no ALPN) resolves against this snapshot. + %% Resumable: only the resumption-eligible config (session_tickets enabled) + %% updates the memo, so a custom-ssl_options handshake cannot poison it. AlpnProtos = alpn_advertised(FinalSslOpts), + Resumable = resumption_enabled(FinalSslOpts), Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), GatedSslOpts = gate_resumption(FinalSslOpts, Cached), case ssl:connect(Socket, GatedSslOpts) of {ok, SslSocket} -> %% Detect negotiated protocol, carrying ALPN across resumption - Protocol = hackney_ssl:negotiated_protocol(SslSocket, Host, AlpnProtos, Cached), + Protocol = hackney_ssl:negotiated_protocol(SslSocket, Host, AlpnProtos, Cached, Resumable), %% Update connection to use SSL NewData = Data#conn_data{ transport = hackney_ssl, @@ -2727,13 +2730,15 @@ do_tcp_connect(From, Data) -> %% Gate TLS resumption on the ALPN memo and snapshot the cached %% protocol now, so a resumed session (where ssl reports no ALPN) is %% resolved against this snapshot rather than re-read from the memo. + %% Resumable: only the resumption-eligible config updates the memo. AlpnProtos = alpn_advertised(FinalSslOpts0), + Resumable = resumption_enabled(FinalSslOpts0), Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), FinalSslOpts = gate_resumption(FinalSslOpts0, Cached), Opts = TransportOpts ++ [{ssl_options, FinalSslOpts}], case Transport:connect(Host, Port, Opts, Timeout) of {ok, Socket} -> - case hackney_ssl:negotiated_protocol(Socket, Host, AlpnProtos, Cached) of + case hackney_ssl:negotiated_protocol(Socket, Host, AlpnProtos, Cached, Resumable) of http2 -> init_h2_connection(Socket, Data#conn_data{socket = Socket, protocol = http2}, From); @@ -2768,6 +2773,13 @@ alpn_advertised(SslOpts) -> gate_resumption(SslOpts, none) -> proplists:delete(session_tickets, SslOpts); gate_resumption(SslOpts, _Cached) -> SslOpts. +%% @private Whether hackney enabled TLS resumption for this conn. effective_opts/3 +%% adds `session_tickets' only for the resumable (default) config, which is the +%% single ticket source per host; only such conns may update the ALPN memo, so a +%% custom-ssl_options handshake cannot poison the entry a resumed session reads. +resumption_enabled(SslOpts) -> + lists:keymember(session_tickets, 1, SslOpts). + %% @private Initialize HTTP/2 connection via the h2 library. %% The h2_connection process takes ownership of the socket and delivers %% owner messages ({h2, Conn, Event}) to this gen_statem's mailbox. diff --git a/src/hackney_ssl.erl b/src/hackney_ssl.erl index 89280b36..76a4916b 100644 --- a/src/hackney_ssl.erl +++ b/src/hackney_ssl.erl @@ -41,7 +41,7 @@ %% ALPN (Application-Layer Protocol Negotiation) for HTTP/2 -export([alpn_opts/1]). -export([get_negotiated_protocol/1]). --export([negotiated_protocol/4, resolve_alpn/5, recall_alpn/2]). +-export([negotiated_protocol/5, resolve_alpn/6, recall_alpn/2]). %% @doc Atoms used to identify messages in {active, once | true} mode. messages(_) -> {ssl, ssl_closed, ssl_error}. @@ -514,36 +514,46 @@ get_negotiated_protocol(SslSocket) -> %% nothing, so we fall back to `Cached' (snapshotted before the handshake), but %% only when the session actually resumed - a full handshake that reports no ALPN %% is a genuine HTTP/1 conn. `Cached' is `recall_alpn(Host, AlpnProtos)' read at -%% the resumption gate. --spec negotiated_protocol(ssl:sslsocket(), string(), list(), http2 | http1 | none) -> - http2 | http1. -negotiated_protocol(SslSocket, Host, AlpnProtos, Cached) -> +%% the resumption gate. `Resumable' is whether this conn is tied to the resumable +%% ticket source (hackney enabled session_tickets); only then is the memo written, +%% so a custom-ssl_options handshake that does not seed the ticket store cannot +%% poison the shared `{Host, AlpnProtos}' entry a later resumed session reads. +-spec negotiated_protocol(ssl:sslsocket(), string(), list(), + http2 | http1 | none, boolean()) -> http2 | http1. +negotiated_protocol(SslSocket, Host, AlpnProtos, Cached, Resumable) -> resolve_alpn(ssl:negotiated_protocol(SslSocket), resumed(SslSocket), - Cached, Host, AlpnProtos). + Cached, Host, AlpnProtos, Resumable). -%% @doc Pure ALPN decision (exported for tests). See negotiated_protocol/4. +%% @doc Pure ALPN decision (exported for tests). See negotiated_protocol/5. -spec resolve_alpn({ok, binary()} | {error, term()}, boolean(), - http2 | http1 | none, string(), list()) -> http2 | http1. -resolve_alpn({ok, <<"h2">>}, _Resumed, _Cached, Host, AlpnProtos) -> - remember_alpn(Host, AlpnProtos, http2), + http2 | http1 | none, string(), list(), boolean()) -> + http2 | http1. +resolve_alpn({ok, <<"h2">>}, _Resumed, _Cached, Host, AlpnProtos, Resumable) -> + maybe_remember(Resumable, Host, AlpnProtos, http2), http2; -resolve_alpn({ok, <<"http/1.1">>}, _Resumed, _Cached, Host, AlpnProtos) -> - remember_alpn(Host, AlpnProtos, http1), +resolve_alpn({ok, <<"http/1.1">>}, _Resumed, _Cached, Host, AlpnProtos, Resumable) -> + maybe_remember(Resumable, Host, AlpnProtos, http1), http1; -resolve_alpn({error, protocol_not_negotiated}, true, Cached, _Host, _AlpnProtos) +resolve_alpn({error, protocol_not_negotiated}, true, Cached, _Host, _AlpnProtos, _Resumable) when Cached =/= none -> %% Genuinely resumed: ALPN is not re-reported, use the gate-time snapshot. Cached; -resolve_alpn({error, protocol_not_negotiated}, false, _Cached, Host, AlpnProtos) -> - %% Full handshake with no ALPN: a real HTTP/1 conn. Refresh the memo so a stale - %% cached http2 cannot be recalled on a later resumption. - remember_alpn(Host, AlpnProtos, http1), +resolve_alpn({error, protocol_not_negotiated}, false, _Cached, Host, AlpnProtos, Resumable) -> + %% Full handshake with no ALPN: a real HTTP/1 conn. Refresh the memo (only for + %% the resumable source) so a stale cached http2 cannot be recalled later. + maybe_remember(Resumable, Host, AlpnProtos, http1), http1; -resolve_alpn(_Other, _Resumed, _Cached, _Host, _AlpnProtos) -> +resolve_alpn(_Other, _Resumed, _Cached, _Host, _AlpnProtos, _Resumable) -> %% Defensive: resumed without a snapshot (gate should prevent this) or an %% unexpected ssl:negotiated_protocol/1 result. http1. +%% @private Write the ALPN memo only for handshakes tied to the resumable ticket +%% source (hackney's default-config resumption). A non-resumable handshake +%% (custom ssl_options) leaves the shared entry untouched. +maybe_remember(true, Host, AlpnProtos, Proto) -> remember_alpn(Host, AlpnProtos, Proto); +maybe_remember(false, _Host, _AlpnProtos, _Proto) -> ok. + %% @doc Whether the TLS handshake resumed a session (PSK / abbreviated handshake). -spec resumed(ssl:sslsocket()) -> boolean(). resumed(SslSocket) -> diff --git a/test/hackney_ssl_alpn_tests.erl b/test/hackney_ssl_alpn_tests.erl index c293aab0..546246df 100644 --- a/test/hackney_ssl_alpn_tests.erl +++ b/test/hackney_ssl_alpn_tests.erl @@ -5,6 +5,7 @@ %% Unit tests for the ALPN-across-resumption memo (hackney_ssl). The memo is a %% shared named ETS table, so each case uses a distinct host to avoid interference. +%% resolve_alpn/6 args: (NegResult, Resumed, Cached, Host, AlpnProtos, Resumable). -module(hackney_ssl_alpn_tests). -include_lib("eunit/include/eunit.hrl"). @@ -27,6 +28,7 @@ alpn_test_() -> fun carried_snapshot_is_race_free/0, fun resumed_without_snapshot_defaults_http1/0, fun key_preserves_order_and_protocol_set/0, + fun non_resumable_handshake_does_not_write_memo/0, fun options_key_ignores_session_tickets/0 ]}. @@ -34,36 +36,36 @@ alpn_test_() -> cold_gate() -> ?assertEqual(none, hackney_ssl:recall_alpn("cold.example", h2())). -%% A full handshake reporting h2 caches it. +%% A resumable full handshake reporting h2 caches it. learn_h2_then_recall() -> ?assertEqual(http2, - hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "learn.example", h2())), + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "learn.example", h2(), true)), ?assertEqual(http2, hackney_ssl:recall_alpn("learn.example", h2())). %% Core: a genuinely resumed session (no ALPN reported) recalls the carried snapshot. resumed_recall() -> ?assertEqual(http2, - hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "res.example", h2())), + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "res.example", h2(), true)), ?assertEqual(http2, hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http2, - "res.example", h2())). + "res.example", h2(), true)). http1_learn_and_recall() -> ?assertEqual(http1, - hackney_ssl:resolve_alpn({ok, <<"http/1.1">>}, false, none, "h1.example", h1())), + hackney_ssl:resolve_alpn({ok, <<"http/1.1">>}, false, none, "h1.example", h1(), true)), ?assertEqual(http1, hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http1, - "h1.example", h1())). + "h1.example", h1(), true)). %% Resumption-status guard + memo refresh: a full handshake that reports no ALPN is -%% a real http1 even with http2 carried, and it overwrites the stale http2 so a -%% later resumption cannot recall it. +%% a real http1 even with http2 carried, and it overwrites the stale http2 (for the +%% resumable source) so a later resumption cannot recall it. full_handshake_refreshes_stale_http2() -> ?assertEqual(http2, - hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ref.example", h2())), + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ref.example", h2(), true)), ?assertEqual(http1, hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, false, http2, - "ref.example", h2())), + "ref.example", h2(), true)), ?assertEqual(http1, hackney_ssl:recall_alpn("ref.example", h2())). %% The carried snapshot is authoritative regardless of current memo contents @@ -72,21 +74,40 @@ carried_snapshot_is_race_free() -> ?assertEqual(none, hackney_ssl:recall_alpn("race.example", h2())), ?assertEqual(http2, hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, http2, - "race.example", h2())). + "race.example", h2(), true)). %% Resumed but no snapshot (gate should prevent this) defaults to http1, not a guess. resumed_without_snapshot_defaults_http1() -> ?assertEqual(http1, hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, true, none, - "ns.example", h2())). + "ns.example", h2(), true)). %% The memo key includes the advertised list in order and the exact protocol set. key_preserves_order_and_protocol_set() -> ?assertEqual(http2, - hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ord.example", h2())), + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, "ord.example", h2(), true)), ?assertEqual(none, hackney_ssl:recall_alpn("ord.example", h2_reordered())), ?assertEqual(none, hackney_ssl:recall_alpn("ord.example", h1())). +%% A non-resumable (custom ssl_options) handshake must not write the shared memo, +%% so it cannot poison the entry a resumed session reads. +non_resumable_handshake_does_not_write_memo() -> + %% Non-resumable full handshake: returns the reported protocol but writes nothing. + ?assertEqual(http1, + hackney_ssl:resolve_alpn({ok, <<"http/1.1">>}, false, none, + "poison.example", h2(), false)), + ?assertEqual(none, hackney_ssl:recall_alpn("poison.example", h2())), + %% The resumable source owns the entry. + ?assertEqual(http2, + hackney_ssl:resolve_alpn({ok, <<"h2">>}, false, none, + "poison.example", h2(), true)), + ?assertEqual(http2, hackney_ssl:recall_alpn("poison.example", h2())), + %% A later non-resumable http1 handshake does not overwrite it. + ?assertEqual(http1, + hackney_ssl:resolve_alpn({error, protocol_not_negotiated}, false, none, + "poison.example", h2(), false)), + ?assertEqual(http2, hackney_ssl:recall_alpn("poison.example", h2())). + %% The pool tls_key must not depend on session_tickets (the handshake varies it). options_key_ignores_session_tickets() -> Base = [{verify, verify_none}, From 072c5da641ae4afef1395fe957f74a69f07ee966 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Thu, 18 Jun 2026 11:33:53 +0200 Subject: [PATCH 4/4] Only automatic tickets make a conn ALPN-memo-eligible The previous check treated any session_tickets entry as resumable, so a custom ssl_options handshake carrying {session_tickets, disabled} or manual could still overwrite the shared {Host, AlpnProtos} memo. Gate on {session_tickets, auto} (what effective_opts adds for the resumable default config) via the new hackney_ssl:auto_tickets/1, so only that ticket source updates the memo. --- src/hackney_conn.erl | 11 ++--------- src/hackney_ssl.erl | 11 ++++++++++- test/hackney_ssl_alpn_tests.erl | 10 ++++++++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl index 1688ce5b..8084519a 100644 --- a/src/hackney_conn.erl +++ b/src/hackney_conn.erl @@ -903,7 +903,7 @@ connected({call, From}, {upgrade_to_ssl, SslOpts, UpgradeOpts}, #conn_data{socke %% Resumable: only the resumption-eligible config (session_tickets enabled) %% updates the memo, so a custom-ssl_options handshake cannot poison it. AlpnProtos = alpn_advertised(FinalSslOpts), - Resumable = resumption_enabled(FinalSslOpts), + Resumable = hackney_ssl:auto_tickets(FinalSslOpts), Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), GatedSslOpts = gate_resumption(FinalSslOpts, Cached), case ssl:connect(Socket, GatedSslOpts) of @@ -2732,7 +2732,7 @@ do_tcp_connect(From, Data) -> %% resolved against this snapshot rather than re-read from the memo. %% Resumable: only the resumption-eligible config updates the memo. AlpnProtos = alpn_advertised(FinalSslOpts0), - Resumable = resumption_enabled(FinalSslOpts0), + Resumable = hackney_ssl:auto_tickets(FinalSslOpts0), Cached = hackney_ssl:recall_alpn(Host, AlpnProtos), FinalSslOpts = gate_resumption(FinalSslOpts0, Cached), Opts = TransportOpts ++ [{ssl_options, FinalSslOpts}], @@ -2773,13 +2773,6 @@ alpn_advertised(SslOpts) -> gate_resumption(SslOpts, none) -> proplists:delete(session_tickets, SslOpts); gate_resumption(SslOpts, _Cached) -> SslOpts. -%% @private Whether hackney enabled TLS resumption for this conn. effective_opts/3 -%% adds `session_tickets' only for the resumable (default) config, which is the -%% single ticket source per host; only such conns may update the ALPN memo, so a -%% custom-ssl_options handshake cannot poison the entry a resumed session reads. -resumption_enabled(SslOpts) -> - lists:keymember(session_tickets, 1, SslOpts). - %% @private Initialize HTTP/2 connection via the h2 library. %% The h2_connection process takes ownership of the socket and delivers %% owner messages ({h2, Conn, Event}) to this gen_statem's mailbox. diff --git a/src/hackney_ssl.erl b/src/hackney_ssl.erl index 76a4916b..544222c3 100644 --- a/src/hackney_ssl.erl +++ b/src/hackney_ssl.erl @@ -41,7 +41,7 @@ %% ALPN (Application-Layer Protocol Negotiation) for HTTP/2 -export([alpn_opts/1]). -export([get_negotiated_protocol/1]). --export([negotiated_protocol/5, resolve_alpn/6, recall_alpn/2]). +-export([negotiated_protocol/5, resolve_alpn/6, recall_alpn/2, auto_tickets/1]). %% @doc Atoms used to identify messages in {active, once | true} mode. messages(_) -> {ssl, ssl_closed, ssl_error}. @@ -554,6 +554,15 @@ resolve_alpn(_Other, _Resumed, _Cached, _Host, _AlpnProtos, _Resumable) -> maybe_remember(true, Host, AlpnProtos, Proto) -> remember_alpn(Host, AlpnProtos, Proto); maybe_remember(false, _Host, _AlpnProtos, _Proto) -> ok. +%% @doc Whether the effective opts carry hackney's automatic TLS resumption +%% (`{session_tickets, auto}'), which `effective_opts/3' adds only for the +%% resumable default config. Only such connections are tied to that ticket source +%% and may update the ALPN memo; a caller-supplied `disabled' or `manual' tickets +%% entry is not memo-eligible and must not overwrite the shared entry. +-spec auto_tickets(list()) -> boolean(). +auto_tickets(SslOpts) -> + lists:member({session_tickets, auto}, SslOpts). + %% @doc Whether the TLS handshake resumed a session (PSK / abbreviated handshake). -spec resumed(ssl:sslsocket()) -> boolean(). resumed(SslSocket) -> diff --git a/test/hackney_ssl_alpn_tests.erl b/test/hackney_ssl_alpn_tests.erl index 546246df..e0d6d7f8 100644 --- a/test/hackney_ssl_alpn_tests.erl +++ b/test/hackney_ssl_alpn_tests.erl @@ -29,6 +29,7 @@ alpn_test_() -> fun resumed_without_snapshot_defaults_http1/0, fun key_preserves_order_and_protocol_set/0, fun non_resumable_handshake_does_not_write_memo/0, + fun only_auto_tickets_are_memo_eligible/0, fun options_key_ignores_session_tickets/0 ]}. @@ -108,6 +109,15 @@ non_resumable_handshake_does_not_write_memo() -> "poison.example", h2(), false)), ?assertEqual(http2, hackney_ssl:recall_alpn("poison.example", h2())). +%% Only hackney's automatic tickets make a conn memo-eligible; a caller-supplied +%% disabled/manual session_tickets entry must not (it is not the resumable source). +only_auto_tickets_are_memo_eligible() -> + ?assert(hackney_ssl:auto_tickets([{verify, verify_none}, {session_tickets, auto}])), + ?assertNot(hackney_ssl:auto_tickets([{session_tickets, disabled}])), + ?assertNot(hackney_ssl:auto_tickets([{session_tickets, manual}])), + ?assertNot(hackney_ssl:auto_tickets([{verify, verify_none}])), + ?assertNot(hackney_ssl:auto_tickets([])). + %% The pool tls_key must not depend on session_tickets (the handshake varies it). options_key_ignores_session_tickets() -> Base = [{verify, verify_none},