diff --git a/grpc/lib/grpc/channel.ex b/grpc/lib/grpc/channel.ex index a5e76a7e4..1a6463aef 100644 --- a/grpc/lib/grpc/channel.ex +++ b/grpc/lib/grpc/channel.ex @@ -8,6 +8,9 @@ defmodule GRPC.Channel do ## Fields * `:host` - server's host to connect + * `:hostname` - the name the target resolved from, when `host` is an + address the resolver picked (e.g. a `dns://` target that resolved to an + IP). Used for TLS SNI and certificate hostname verification. * `:port` - server's port to connect * `:scheme` - scheme of connection, like `http` * `:cred` - credentials used for authentication @@ -17,6 +20,7 @@ defmodule GRPC.Channel do """ defstruct host: nil, + hostname: nil, port: nil, scheme: nil, cred: nil, diff --git a/grpc/lib/grpc/client/adapters/mint.ex b/grpc/lib/grpc/client/adapters/mint.ex index bfa8414e6..9b9ded86c 100644 --- a/grpc/lib/grpc/client/adapters/mint.ex +++ b/grpc/lib/grpc/client/adapters/mint.ex @@ -53,6 +53,12 @@ if Code.ensure_loaded?(Mint.HTTP) do |> merge_opts(module_opts) |> Keyword.put(:retry, retry) + # A dns:// target resolves to an address (the host we dial) while the + # hostname it resolved from must still reach TLS for SNI and certificate + # verification. + opts = + if hostname = channel.hostname, do: Keyword.put(opts, :hostname, hostname), else: opts + Process.flag(:trap_exit, true) channel diff --git a/grpc/lib/grpc/client/connection.ex b/grpc/lib/grpc/client/connection.ex index b085b62c9..e25d86401 100644 --- a/grpc/lib/grpc/client/connection.ex +++ b/grpc/lib/grpc/client/connection.ex @@ -882,7 +882,8 @@ defmodule GRPC.Client.Connection do end defp connect_new_channels(new_addresses, added, adapter, opts, state, real_channels) do - Enum.reduce(new_addresses, real_channels, fn %{address: host, port: port}, channels -> + Enum.reduce(new_addresses, real_channels, fn %{address: host, port: port} = address, + channels -> key = build_address_key(host, port) existing = Map.get(channels, key) @@ -897,7 +898,14 @@ defmodule GRPC.Client.Connection do _ -> :ok end - case connect_real_channel(state.virtual_channel, host, port, opts, adapter) do + case connect_real_channel( + state.virtual_channel, + host, + port, + opts, + adapter, + address[:hostname] + ) do {:ok, ch} -> Map.put(channels, key, {:connected, ch}) {:error, reason} -> Map.put(channels, key, {:failed, reason}) end @@ -1056,8 +1064,15 @@ defmodule GRPC.Client.Connection do end defp build_real_channels(addresses, %Channel{} = virtual_channel, norm_opts, adapter) do - Map.new(addresses, fn %{port: port, address: host} -> - case connect_real_channel(virtual_channel, host, port, norm_opts, adapter) do + Map.new(addresses, fn %{port: port, address: host} = address -> + case connect_real_channel( + virtual_channel, + host, + port, + norm_opts, + adapter, + address[:hostname] + ) do {:ok, ch} -> {build_address_key(host, port), {:connected, ch}} @@ -1112,8 +1127,8 @@ defmodule GRPC.Client.Connection do defp choose_lb(:round_robin), do: GRPC.Client.LoadBalancing.RoundRobin defp choose_lb(_), do: GRPC.Client.LoadBalancing.PickFirst - defp connect_real_channel(%Channel{} = vc, host, port, opts, adapter) do - %Channel{vc | host: host, port: port} + defp connect_real_channel(%Channel{} = vc, host, port, opts, adapter, hostname) do + %Channel{vc | host: host, port: port, hostname: hostname} |> adapter.connect(opts[:adapter_opts]) end diff --git a/grpc/lib/grpc/client/resolver/dns.ex b/grpc/lib/grpc/client/resolver/dns.ex index 4c371679c..995d5e680 100644 --- a/grpc/lib/grpc/client/resolver/dns.ex +++ b/grpc/lib/grpc/client/resolver/dns.ex @@ -21,7 +21,7 @@ defmodule GRPC.Client.Resolver.DNS do with {:ok, addresses} <- lookup_addresses(host) do addrs = Enum.map(addresses, fn ip -> - %{address: :inet.ntoa(ip) |> to_string(), port: port} + %{address: :inet.ntoa(ip) |> to_string(), port: port, hostname: host} end) case lookup_service_config(host) do diff --git a/grpc/test/grpc/adapters/mint_test.exs b/grpc/test/grpc/adapters/mint_test.exs index ede5b958b..98ef2cd3f 100644 --- a/grpc/test/grpc/adapters/mint_test.exs +++ b/grpc/test/grpc/adapters/mint_test.exs @@ -3,6 +3,11 @@ defmodule GRPC.Client.Adapters.MintTest do alias GRPC.Client.Adapters.Mint + defmodule TLSEndpoint do + use GRPC.Endpoint + run(FeatureServer) + end + setup do {:ok, _, port} = GRPC.Server.start(FeatureServer, 0) @@ -250,4 +255,34 @@ defmodule GRPC.Client.Adapters.MintTest do assert state.retry == 0 end end + + describe "connect/2 with hostname" do + setup do + server_credential = build(:credential) + + {:ok, _, port} = + GRPC.Server.start_endpoint(TLSEndpoint, 0, adapter_opts: [cred: server_credential]) + + on_exit(fn -> :ok = GRPC.Server.stop_endpoint(TLSEndpoint) end) + + %{port: port} + end + + test "dials the resolved address but hands the hostname to Mint for TLS", %{port: port} do + channel = + build(:channel, + adapter: Mint, + port: port, + host: "127.0.0.1", + hostname: "my-service.local", + scheme: "https" + ) + + {:ok, connected} = Mint.connect(channel, []) + state = :sys.get_state(connected.adapter_payload.conn_pid) + + assert state.host == "127.0.0.1" + assert state.connect_opts[:hostname] == "my-service.local" + end + end end diff --git a/grpc/test/grpc/client/connection_test.exs b/grpc/test/grpc/client/connection_test.exs index 3067e7c4d..be82b8f44 100644 --- a/grpc/test/grpc/client/connection_test.exs +++ b/grpc/test/grpc/client/connection_test.exs @@ -34,6 +34,18 @@ defmodule GRPC.Client.ConnectionTest do end end + # Mirrors what GRPC.Client.Resolver.DNS returns for a dns:// target after + # the hostname fix: each resolved address keeps the target's hostname. + defmodule HostnameResolver do + def resolve(_target) do + {:ok, + %{ + addresses: [%{address: "127.0.0.1", port: 50051, hostname: "my-service.local"}], + service_config: %{} + }} + end + end + setup do %{ ref: make_ref(), @@ -354,6 +366,24 @@ defmodule GRPC.Client.ConnectionTest do end end + describe "connect/2 - hostname from the resolver" do + test "carries the dns:// hostname onto the channel for TLS SNI", %{ref: ref} do + resolver = HostnameResolver + + {:ok, channel} = + Connection.connect("dns://my-service.local:50051", + adapter: GRPC.Test.ClientAdapter, + name: ref, + resolver: resolver + ) + + assert channel.host == "127.0.0.1" + assert channel.hostname == "my-service.local" + + Connection.disconnect(channel) + end + end + describe "connect/2 - distributed named channels" do test "named channels do not conflict across connected nodes" do {:ok, _, port} = GRPC.Server.start(FeatureServer, 0) diff --git a/grpc/test/grpc/resolver/dns_test.exs b/grpc/test/grpc/resolver/dns_test.exs index f1e0a92b3..1c8723f2a 100644 --- a/grpc/test/grpc/resolver/dns_test.exs +++ b/grpc/test/grpc/resolver/dns_test.exs @@ -154,4 +154,20 @@ defmodule GRPC.Client.Resolver.DNSTest do assert [%{address: "127.0.0.1", port: 50051}] = addrs assert {:ok, %{load_balancing_policy: :pick_first}} = config end + + test "keeps the hostname of a dns:// target alongside the resolved IP" do + host = "my-service.local" + config_name = "_grpc_config." <> host + + DNS.MockAdapter + |> expect(:lookup, fn ^host, :a -> + {:ok, [{127, 0, 0, 1}]} + end) + |> expect(:lookup, fn ^config_name, :txt -> + {:ok, []} + end) + + assert {:ok, %{addresses: addrs}} = DNS.resolve("dns://#{host}:50051") + assert [%{address: "127.0.0.1", port: 50051, hostname: ^host}] = addrs + end end