Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion config/runtime.exs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ if config_env() == :prod do
config :nerves_hub, NervesHubWeb.DeviceSocket,
shared_secrets: [
enabled: System.get_env("DEVICE_SHARED_SECRETS_ENABLED", "false") == "true"
]
],
web_endpoint_supported:
System.get_env("DEVICE_SOCKET_WEB_ENDPOINT_SUPPORTED", "true") == "true"
end

##
Expand Down
31 changes: 29 additions & 2 deletions lib/nerves_hub_web/channels/device_socket.ex
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,28 @@ defmodule NervesHubWeb.DeviceSocket do

# Used by Devices connecting with HMAC Shared Secrets
@decorate with_span("Channels.DeviceSocket.connect")
def connect(_params, socket, %{x_headers: x_headers})
def connect(_params, socket, %{x_headers: x_headers} = connect_info)
when is_list(x_headers) and length(x_headers) > 0 do
headers = Map.new(x_headers)

with :ok <- check_shared_secret_enabled(),
with :ok <- check_source_enabled(connect_info[:source]),
:ok <- check_shared_secret_enabled(),
{:ok, key, salt, verification_opts} <- decode_from_headers(headers),
{:ok, auth} <- get_shared_secret_auth(key),
{:ok, signature} <- Map.fetch(headers, "x-nh-signature"),
{:ok, identifier} <- Crypto.verify(auth.secret, salt, signature, verification_opts),
{:ok, device} <- get_or_maybe_create_device(auth, identifier) do
socket_and_assigns(socket, device)
else
{:error, :check_uri} = error ->
:telemetry.execute([:nerves_hub, :devices, :invalid_auth], %{count: 1}, %{
auth: :shared_secrets,
reason: error,
product_key: Map.get(headers, "x-nh-key", "*empty*")
})

error

error ->
:telemetry.execute([:nerves_hub, :devices, :invalid_auth], %{count: 1}, %{
auth: :shared_secrets,
Expand Down Expand Up @@ -188,6 +198,14 @@ defmodule NervesHubWeb.DeviceSocket do
end
end

defp check_source_enabled(source) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should have an in-between mode for the transition that just throws a "wrong-endpoint" key in the device connection metadata or similar. And then we could show it to people.

Though we have few enough people that will be hit by it that we can essentially tell them and give them a deadline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wouldn't want to add too much extra complexity to this. Instead we can just send out a warning to current users, check our logs to make sure people are off, and then turn it off. I have no intention to turn this off yet.

if source_enabled?(source) do
:ok
else
{:error, :check_uri}
end
end

defp socket_and_assigns(socket, device) do
# disconnect devices using the same identifier
_ = socket.endpoint.broadcast_from(self(), "device_socket:#{device.id}", "disconnect", %{})
Expand Down Expand Up @@ -279,4 +297,13 @@ defmodule NervesHubWeb.DeviceSocket do
|> Keyword.get(:shared_secrets, [])
|> Keyword.get(:enabled, false)
end

def source_enabled?(nil) do
true
end

def source_enabled?(NervesHubWeb.Endpoint) do
Application.get_env(:nerves_hub, __MODULE__, [])
|> Keyword.get(:web_endpoint_supported, true)
end
end
2 changes: 1 addition & 1 deletion lib/nerves_hub_web/endpoint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule NervesHubWeb.Endpoint do
"/device-socket",
NervesHubWeb.DeviceSocket,
websocket: [
connect_info: [:peer_data, :x_headers],
connect_info: [:peer_data, :x_headers, source: __MODULE__],
compress: true,
timeout: 180_000,
fullsweep_after: 0,
Expand Down
13 changes: 10 additions & 3 deletions lib/nerves_hub_web/helpers/websocket_connection_error.ex
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
defmodule NervesHub.Helpers.WebsocketConnectionError do
import Plug.Conn

@message "no certificate pair or shared secrets connection settings were provided"
@no_auth_message "no certificate pair or shared secrets connection settings were provided"
@check_uri_message "incorrect uri used, please contact support"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we get some configured URL info into this message so we can actually tell them the fix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Although, just so you know, slipstream doesn't show this message by default, which is a pity


def handle_error(conn, :no_auth) do
conn
|> put_resp_header("nh-connection-error-reason", @message)
|> send_resp(401, @message)
|> put_resp_header("nh-connection-error-reason", @no_auth_message)
|> send_resp(401, @no_auth_message)
end

def handle_error(conn, :check_uri) do
conn
|> put_resp_header("nh-connection-error-reason", @check_uri_message)
|> send_resp(404, @check_uri_message)
end

def handle_error(conn, _reason), do: send_resp(conn, 401, "")
Expand Down
43 changes: 43 additions & 0 deletions test/nerves_hub_web/channels/websocket_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,49 @@ defmodule NervesHubWeb.WebsocketTest do
end
end

describe "web endpoint device connections" do
@describetag :tmp_dir

setup do
Application.put_env(:nerves_hub, NervesHubWeb.DeviceSocket, shared_secrets: [enabled: true])
Application.put_env(:nerves_hub, NervesHubWeb.DeviceSocket, web_endpoint_supported: false)

on_exit(fn ->
Application.put_env(:nerves_hub, NervesHubWeb.DeviceSocket,
shared_secrets: [enabled: false]
)

Application.put_env(:nerves_hub, NervesHubWeb.DeviceSocket, web_endpoint_supported: true)
end)
end

test "can disable devices connecting via the web endpoint", %{user: user} do
org = Fixtures.org_fixture(user)
product = Fixtures.product_fixture(user, org)
assert {:ok, auth} = Products.create_shared_secret_auth(product)

identifier = Ecto.UUID.generate()
refute Repo.get_by(Device, identifier: identifier)

opts = [
mint_opts: [protocols: [:http1]],
uri: "ws://127.0.0.1:#{@web_port}/device-socket/websocket",
headers: Utils.nh1_key_secret_headers(auth, identifier)
]

{:ok, socket} = SocketClient.start_link(opts)

SocketClient.wait_connect(socket)

refute SocketClient.connected?(socket)

assigns = SocketClient.state(socket).assigns

assert assigns.error_code == 404
assert assigns.error_reason == "incorrect uri used, please contact support"
end
end

describe "duplicate connections using the same device id" do
@describetag :tmp_dir

Expand Down
2 changes: 1 addition & 1 deletion test/support/socket_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ defmodule SocketClient do

@impl Slipstream
def handle_disconnect(
{:error, {:upgrade_failure, %{reason: %{status_code: 401} = reason}}},
{:error, {:upgrade_failure, %{reason: reason}}},
socket
) do
socket =
Expand Down
Loading