Skip to content

Commit cefb6e5

Browse files
authored
Handle ArgumentErrors from Plug.Crypto.non_executable_binary_to_term (#2269)
This was found in the NervesCloud Sentry. If an invalid device identifier, this this case an Erlang term, is encoded in the shared secret auth, `Plug.Crypto.non_executable_binary_to_term/2` correctly catches it, but an error is raised which we don't handle nicely. This PR rescues `ArgumentError`s, logs a message, and rejects the auth request.
1 parent 77f7211 commit cefb6e5

File tree

2 files changed

+31
-0
lines changed

2 files changed

+31
-0
lines changed

lib/nerves_hub_web/channels/device_socket.ex

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,17 @@ defmodule NervesHubWeb.DeviceSocket do
114114

115115
{:error, :invalid_auth}
116116
end
117+
rescue
118+
e in ArgumentError ->
119+
headers = Map.new(x_headers)
120+
121+
:telemetry.execute([:nerves_hub, :devices, :invalid_auth], %{count: 1}, %{
122+
auth: :shared_secrets,
123+
reason: e,
124+
product_key: Map.get(headers, "x-nh-key", "*empty*")
125+
})
126+
127+
{:error, :invalid_auth}
117128
end
118129

119130
def connect(_params, _socket, _connect_info) do

test/nerves_hub_web/channels/websocket_test.exs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,26 @@ defmodule NervesHubWeb.WebsocketTest do
481481
refute_online(device)
482482
end
483483
end
484+
485+
test "safely rejects if an ETF is passed in as a device identifier", %{user: user} do
486+
org = Fixtures.org_fixture(user)
487+
product = Fixtures.product_fixture(user, org)
488+
assert {:ok, auth} = Products.create_shared_secret_auth(product)
489+
490+
identifier = &Ecto.UUID.generate/0
491+
492+
opts = [
493+
mint_opts: [protocols: [:http1]],
494+
uri: "ws://127.0.0.1:#{@web_port}/device-socket/websocket",
495+
headers: Utils.nh1_key_secret_headers(auth, identifier)
496+
]
497+
498+
{:ok, socket} = SocketClient.start_link(opts)
499+
500+
SocketClient.wait_connect(socket)
501+
502+
refute SocketClient.connected?(socket)
503+
end
484504
end
485505

486506
describe "duplicate connections using the same device id" do

0 commit comments

Comments
 (0)