Skip to content

Commit b63eaaa

Browse files
authored
Don't leak target options into the proxy connection in forward-proxy mode (#494)
Mint.HTTP.connect/4 merged the target connect options with the proxy options and handed the whole set to the connection to the proxy. A target :hostname was therefore used as the proxy connection's hostname, so with an :https proxy the proxy's certificate was verified against the target's identity, and :transport_opts meant for the target applied to the proxy socket. Forward-proxy mode now keeps the two option sets separate, like tunnel mode already does: the proxy connection is configured by the options in the :proxy tuple, while the options given to Mint.HTTP.connect/4 describe the target (its :hostname identity and the :proxy_headers).
1 parent 4e38b23 commit b63eaaa

4 files changed

Lines changed: 182 additions & 17 deletions

File tree

lib/mint/http.ex

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,9 @@ defmodule Mint.HTTP do
432432
{:ok, {proxy_scheme, proxy_address, proxy_port, proxy_opts}} ->
433433
case Util.scheme_to_transport(scheme) do
434434
Transport.TCP ->
435-
proxy = {proxy_scheme, proxy_address, proxy_port}
436-
host = {scheme, address, port}
437-
opts = Keyword.merge(opts, proxy_opts)
438-
UnsafeProxy.connect(proxy, host, opts)
435+
proxy = {proxy_scheme, proxy_address, proxy_port, proxy_opts}
436+
host = {scheme, address, port, opts}
437+
UnsafeProxy.connect(proxy, host)
439438

440439
Transport.SSL ->
441440
proxy = {proxy_scheme, proxy_address, proxy_port, proxy_opts}

lib/mint/unsafe_proxy.ex

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ defmodule Mint.UnsafeProxy do
1616

1717
@opaque t() :: %UnsafeProxy{}
1818

19-
@type host_triple() :: {Types.scheme(), address :: Types.address(), :inet.port_number()}
19+
@type host_tuple() ::
20+
{Types.scheme(), address :: Types.address(), :inet.port_number(), opts :: keyword()}
2021

21-
@spec connect(host_triple(), host_triple(), opts :: keyword()) ::
22-
{:ok, t()} | {:error, Types.error()}
23-
def connect(proxy, host, opts \\ []) do
24-
{proxy_scheme, proxy_address, proxy_port} = proxy
25-
{scheme, address, port} = host
22+
@spec connect(host_tuple(), host_tuple()) :: {:ok, t()} | {:error, Types.error()}
23+
def connect(proxy, host) do
24+
{proxy_scheme, proxy_address, proxy_port, proxy_opts} = proxy
25+
{scheme, address, port, opts} = host
2626
hostname = Mint.Core.Util.hostname(opts, address)
2727

28-
with {:ok, state} <- Mint.HTTP1.connect(proxy_scheme, proxy_address, proxy_port, opts) do
28+
with {:ok, state} <- Mint.HTTP1.connect(proxy_scheme, proxy_address, proxy_port, proxy_opts) do
2929
conn = %UnsafeProxy{
3030
scheme: scheme,
3131
hostname: hostname,
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
defmodule Mint.UnsafeProxyOptsTest do
2+
use ExUnit.Case, async: true
3+
4+
import Mint.HTTP1.TestHelpers
5+
6+
alias Mint.{HTTP, TransportError}
7+
8+
# Offline tests for how connect options are split in forward-proxy mode: the
9+
# proxy connection is configured by the options in the :proxy tuple, while
10+
# the options given to Mint.HTTP.connect/4 describe the target. They use
11+
# local TCP/TLS servers as the proxy, so they need no live proxy or internet
12+
# connection.
13+
14+
@cert_opts [
15+
certfile: Path.absname("../support/mint/certificate.pem", __DIR__),
16+
keyfile: Path.absname("../support/mint/key.pem", __DIR__)
17+
]
18+
19+
@tag :capture_log
20+
test "target transport options are not used for the proxy connection" do
21+
{proxy_port, _proxy_ref} = start_tls_proxy(@cert_opts)
22+
23+
assert {:error, %TransportError{}} =
24+
HTTP.connect(:http, "example.com", 80,
25+
proxy: {:https, "localhost", proxy_port, []},
26+
transport_opts: [verify: :verify_none]
27+
)
28+
end
29+
30+
test "proxy transport options are used for the proxy connection" do
31+
{proxy_port, proxy_ref} = start_tls_proxy(@cert_opts)
32+
33+
assert {:ok, conn} =
34+
HTTP.connect(:http, "example.com", 80,
35+
proxy: {:https, "localhost", proxy_port, [transport_opts: [verify: :verify_none]]}
36+
)
37+
38+
assert {:ok, conn, request} = HTTP.request(conn, "GET", "/", [], nil)
39+
assert_receive {^proxy_ref, :request, head}, 2000
40+
assert head =~ "GET http://example.com/ HTTP/1.1\r\n"
41+
42+
assert {:ok, _conn, responses} = receive_stream(conn)
43+
assert [{:status, ^request, 200}, {:headers, ^request, _headers} | rest] = responses
44+
assert merge_body(rest, request) == "ok"
45+
end
46+
47+
test "the target :hostname option is not used to verify the proxy certificate" do
48+
%{server_config: server_config, client_config: client_config} = pkix_test_chain()
49+
{proxy_port, proxy_ref} = start_tls_proxy(server_config)
50+
51+
assert {:ok, conn} =
52+
HTTP.connect(:http, "example.com", 80,
53+
proxy:
54+
{:https, "localhost", proxy_port,
55+
[transport_opts: [cacerts: client_config[:cacerts]]]},
56+
hostname: "wrong.example"
57+
)
58+
59+
# The :hostname option still overrides the target identity in the request.
60+
assert {:ok, _conn, _request} = HTTP.request(conn, "GET", "/", [], nil)
61+
assert_receive {^proxy_ref, :request, head}, 2000
62+
assert head =~ "GET http://wrong.example/ HTTP/1.1\r\n"
63+
end
64+
65+
test ":proxy_headers are taken from the connect options" do
66+
{proxy_port, proxy_ref} = start_tcp_proxy()
67+
68+
assert {:ok, conn} =
69+
HTTP.connect(:http, "example.com", 80,
70+
proxy: {:http, "localhost", proxy_port, []},
71+
proxy_headers: [{"proxy-authorization", "Basic dGVzdDpwYXNzd29yZA=="}]
72+
)
73+
74+
assert {:ok, _conn, _request} = HTTP.request(conn, "GET", "/", [], nil)
75+
assert_receive {^proxy_ref, :request, head}, 2000
76+
assert head =~ "proxy-authorization: Basic dGVzdDpwYXNzd29yZA==\r\n"
77+
end
78+
79+
# Starts a one-shot TLS server that accepts a single connection, reports the
80+
# raw request head to the test process, and replies with a canned response.
81+
defp start_tls_proxy(ssl_opts) do
82+
test_pid = self()
83+
ref = make_ref()
84+
socket_opts = [mode: :binary, packet: :raw, active: false, reuseaddr: true]
85+
{:ok, listen_socket} = :ssl.listen(0, socket_opts ++ ssl_opts)
86+
{:ok, {_address, port}} = :ssl.sockname(listen_socket)
87+
88+
spawn_link(fn ->
89+
{:ok, socket} = :ssl.transport_accept(listen_socket)
90+
91+
case :ssl.handshake(socket, 10_000) do
92+
{:ok, socket} ->
93+
send(test_pid, {ref, :request, recv_ssl_request_head(socket)})
94+
:ok = :ssl.send(socket, "HTTP/1.1 200 OK\r\ncontent-length: 2\r\n\r\nok")
95+
96+
receive do
97+
:stop -> :ok
98+
end
99+
100+
{:error, _reason} ->
101+
:ok
102+
end
103+
end)
104+
105+
{port, ref}
106+
end
107+
108+
# Same as start_tls_proxy/1, but plain TCP.
109+
defp start_tcp_proxy do
110+
test_pid = self()
111+
ref = make_ref()
112+
113+
{:ok, listen_socket} =
114+
:gen_tcp.listen(0, mode: :binary, packet: :raw, active: false, reuseaddr: true)
115+
116+
{:ok, port} = :inet.port(listen_socket)
117+
118+
spawn_link(fn ->
119+
{:ok, socket} = :gen_tcp.accept(listen_socket)
120+
send(test_pid, {ref, :request, recv_tcp_request_head(socket)})
121+
:ok = :gen_tcp.send(socket, "HTTP/1.1 200 OK\r\ncontent-length: 2\r\n\r\nok")
122+
123+
receive do
124+
:stop -> :ok
125+
end
126+
end)
127+
128+
{port, ref}
129+
end
130+
131+
defp recv_ssl_request_head(socket, buffer \\ "") do
132+
if String.contains?(buffer, "\r\n\r\n") do
133+
buffer
134+
else
135+
{:ok, data} = :ssl.recv(socket, 0, 2000)
136+
recv_ssl_request_head(socket, buffer <> data)
137+
end
138+
end
139+
140+
defp recv_tcp_request_head(socket, buffer \\ "") do
141+
if String.contains?(buffer, "\r\n\r\n") do
142+
buffer
143+
else
144+
{:ok, data} = :gen_tcp.recv(socket, 0, 2000)
145+
recv_tcp_request_head(socket, buffer <> data)
146+
end
147+
end
148+
149+
defp pkix_test_chain do
150+
san_extension = {:Extension, {2, 5, 29, 17}, false, [dNSName: ~c"localhost"]}
151+
cert_opts = [digest: :sha256, key: {:rsa, 2048, 17}]
152+
153+
:public_key.pkix_test_data(%{
154+
server_chain: %{
155+
root: cert_opts,
156+
intermediates: [],
157+
peer: cert_opts ++ [extensions: [san_extension]]
158+
},
159+
client_chain: %{
160+
root: cert_opts,
161+
intermediates: [],
162+
peer: cert_opts
163+
}
164+
})
165+
end
166+
end

test/mint/unsafe_proxy_test.exs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ defmodule Mint.UnsafeProxyTest do
1111
test "200 response - http://httpbin.org" do
1212
assert {:ok, conn} =
1313
UnsafeProxy.connect(
14-
{:http, "localhost", HttpBin.proxy_port()},
15-
{:http, HttpBin.proxy_host(), HttpBin.http_port()}
14+
{:http, "localhost", HttpBin.proxy_port(), []},
15+
{:http, HttpBin.proxy_host(), HttpBin.http_port(), []}
1616
)
1717

1818
assert {:ok, conn, request} = UnsafeProxy.request(conn, "GET", "/", [], nil)
@@ -73,8 +73,8 @@ defmodule Mint.UnsafeProxyTest do
7373
test "Mint.HTTP.protocol/1 on an unsafe proxy connection" do
7474
assert {:ok, %UnsafeProxy{} = conn} =
7575
UnsafeProxy.connect(
76-
{:http, "localhost", HttpBin.proxy_port()},
77-
{:http, HttpBin.proxy_host(), HttpBin.http_port()}
76+
{:http, "localhost", HttpBin.proxy_port(), []},
77+
{:http, HttpBin.proxy_host(), HttpBin.http_port(), []}
7878
)
7979

8080
assert Mint.HTTP.protocol(conn) == :http1
@@ -86,8 +86,8 @@ defmodule Mint.UnsafeProxyTest do
8686

8787
assert {:ok, %UnsafeProxy{state: %{socket: socket}} = conn} =
8888
UnsafeProxy.connect(
89-
{:http, "localhost", HttpBin.proxy_port()},
90-
{:http, HttpBin.proxy_host(), HttpBin.http_port()}
89+
{:http, "localhost", HttpBin.proxy_port(), []},
90+
{:http, HttpBin.proxy_host(), HttpBin.http_port(), []}
9191
)
9292

9393
assert is_connection_message(conn, {:tcp, socket, "foo"}) == true

0 commit comments

Comments
 (0)