refactor(portal): Don't pin session token to user_agent or remote_ip (#2195)

Removing the check to get Rust PRs to pass.

**Note**: #2182 was dependent on this one, and has since merged into
this one.
This commit is contained in:
Jamil
2023-09-30 07:40:57 -07:00
committed by GitHub
parent e6919c04f9
commit c4c6f3e4ca
16 changed files with 70 additions and 44 deletions

View File

@@ -36,7 +36,8 @@ jobs:
- name: Start docker compose in the background
run: docker compose up -d
- name: Test that client can ping resource
run: docker compose exec -it client ping 172.20.0.100 -c 10
run: docker compose exec -it client timeout 60 bash -c 'until ping -W 1 -c 1 172.20.0.100 &>/dev/null; do true; done'
integration-test_relayed-flow:
runs-on: ubuntu-latest
@@ -74,4 +75,4 @@ jobs:
sudo iptables -I FORWARD 1 -s 172.28.0.100 -d 172.28.0.105 -j DROP
sudo iptables -I FORWARD 1 -s 172.28.0.105 -d 172.28.0.100 -j DROP
- name: Test that client can ping resource
run: docker compose exec -it client ping 172.20.0.100 -c 10
run: docker compose exec -it client timeout 60 bash -c 'until ping -W 1 -c 1 172.20.0.100 &>/dev/null; do true; done'

View File

@@ -10,7 +10,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v2
- uses: actions/setup-python@v4
with:
python-version: "3.9"
- uses: actions/cache@v3

View File

@@ -134,6 +134,8 @@ services:
devices:
- "/dev/net/tun:/dev/net/tun"
depends_on:
gateway:
condition: 'service_healthy'
httpbin:
condition: 'service_healthy'
api:
@@ -143,6 +145,8 @@ services:
ipv4_address: 172.28.0.100
gateway:
healthcheck:
test: [ "CMD-SHELL", "ip link | grep tun-firezone" ]
environment:
FZ_URL: "ws://api:8081/"
FZ_SECRET: "SFMyNTY.g2gDaAJtAAAAJDNjZWYwNTY2LWFkZmQtNDhmZS1hMGYxLTU4MDY3OTYwOGY2Zm0AAABAamp0enhSRkpQWkdCYy1vQ1o5RHkyRndqd2FIWE1BVWRwenVScjJzUnJvcHg3NS16bmhfeHBfNWJUNU9uby1yYm4GAEC0b0KJAWIAAVGA.9Oirn9t8rvQpfOhW7hwGBFVzeMm9di0xYGTlwf9cFFk"
@@ -176,10 +180,6 @@ services:
image: kennethreitz/httpbin
healthcheck:
test: [ "CMD-SHELL", "ps -C gunicorn" ]
start_period: 20s
interval: 30s
retries: 5
timeout: 5s
networks:
resources:
ipv4_address: 172.20.0.100

View File

@@ -611,12 +611,13 @@ defmodule Domain.Auth do
defp verify_token_payload(
token,
{:identity, identity_id, context_payload},
user_agent,
remote_ip
{:identity, identity_id, _context_payload},
_user_agent,
_remote_ip
) do
with {:ok, identity} <- fetch_active_identity_by_id(identity_id),
true <- context_payload == session_context_payload(remote_ip, user_agent),
# XXX: Don't pin tokens to remote_ip and user_agent -- use device external_id instead?
# true <- context_payload == session_context_payload(remote_ip, user_agent),
{:ok, expires_at} <- fetch_session_token_expires_at(token) do
{:ok, identity, expires_at}
else

View File

@@ -2447,22 +2447,24 @@ defmodule Domain.AuthTest do
assert updated_identity.last_seen_user_agent != identity.last_seen_user_agent
end
test "returns error when session token is created with a different remote ip", %{
subject: subject,
user_agent: user_agent
} do
{:ok, token} = create_session_token_from_subject(subject)
assert sign_in(token, user_agent, {127, 0, 0, 1}) == {:error, :unauthorized}
end
test "returns error when session token is created with a different user agent", %{
subject: subject,
remote_ip: remote_ip
} do
user_agent = "iOS/12.6 (iPhone) connlib/0.7.412"
{:ok, token} = create_session_token_from_subject(subject)
assert sign_in(token, user_agent, remote_ip) == {:error, :unauthorized}
end
# XXX: Use different params to pin the session token on as these are likely to change
# over the lifetime of the session token.
# test "returns error when session token is created with a different remote ip", %{
# subject: subject,
# user_agent: user_agent
# } do
# {:ok, token} = create_session_token_from_subject(subject)
# assert sign_in(token, user_agent, {127, 0, 0, 1}) == {:error, :unauthorized}
# end
#
# test "returns error when session token is created with a different user agent", %{
# subject: subject,
# remote_ip: remote_ip
# } do
# user_agent = "iOS/12.6 (iPhone) connlib/0.7.412"
# {:ok, token} = create_session_token_from_subject(subject)
# assert sign_in(token, user_agent, remote_ip) == {:error, :unauthorized}
# end
test "returns error when token is created for a deleted identity", %{
identity: identity,

View File

@@ -1,3 +1,3 @@
target/
clients/android/connlib/build/
clients/android/connlib/jniLibs/
connlib/clients/android/connlib/build/
connlib/clients/android/connlib/jniLibs/

View File

@@ -13,6 +13,8 @@ members = [
"connlib/gateway",
]
resolver = "2"
[workspace.dependencies]
boringtun = { version = "0.6", default-features = false }
chrono = { version = "0.4", default-features = false, features = ["std", "clock", "oldtime", "serde"] }

View File

@@ -1,4 +1,4 @@
FROM rust:1.70-slim as BUILDER
FROM rust:1.72-slim as BUILDER
ARG PACKAGE
WORKDIR /build/
COPY . ./
@@ -10,7 +10,7 @@ RUN --mount=type=cache,target=./target \
RUN --mount=type=cache,target=./target \
mv ./target/release/$PACKAGE /usr/local/bin/$PACKAGE
FROM debian:11.7-slim
FROM debian:12-slim
ARG PACKAGE
WORKDIR /app/
COPY --from=BUILDER /usr/local/bin/$PACKAGE .

View File

@@ -1,5 +1,5 @@
// Swift bridge generated code triggers this below
#![allow(improper_ctypes, non_camel_case_types)]
#![allow(clippy::unnecessary_cast, improper_ctypes, non_camel_case_types)]
use firezone_client_connlib::{file_logger, Callbacks, Error, ResourceDescription, Session};
use ip_network::IpNetwork;

View File

@@ -244,7 +244,7 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
Messages::IceCandidates(ice_candidate) => self.add_ice_candidate(ice_candidate).await,
Messages::SignedLogUrl(url) => {
let Some(path) = self.tunnel.callbacks().roll_log_file() else {
return Ok(())
return Ok(());
};
tokio::spawn(async move {

View File

@@ -75,7 +75,7 @@ impl Handle {
let mut inner = try_unlock(&self.inner);
let new_writer = inner.create_new_writer()?;
let Some((_, name)) = inner.current.replace(new_writer) else {
return Ok(None)
return Ok(None);
};
Ok(Some(inner.directory.join(name)))

View File

@@ -259,7 +259,7 @@ fn upload_interval_from_env_or_default() -> Duration {
let Some(interval) = option_env!("CONNLIB_LOG_UPLOAD_INTERVAL_SECS") else {
tracing::warn!(interval = ?DEFAULT, "Env variable `CONNLIB_LOG_UPLOAD_INTERVAL_SECS` was not set during compile-time, falling back to default");
return DEFAULT
return DEFAULT;
};
let interval = match interval.parse() {

View File

@@ -106,7 +106,8 @@ where
.map_err(|_| Error::InvalidReference)?;
{
let mut awaiting_connections = self.awaiting_connection.lock();
let Some(awaiting_connection) = awaiting_connections.get_mut(&resource_id.into()) else {
let Some(awaiting_connection) = awaiting_connections.get_mut(&resource_id.into())
else {
return Err(Error::UnexpectedConnectionDetails);
};
awaiting_connection.response_received = true;
@@ -196,7 +197,10 @@ where
let Some(gateway_public_key) =
tunnel.gateway_public_keys.lock().remove(&gateway_id)
else {
tunnel.awaiting_connection.lock().remove(&resource_id.into());
tunnel
.awaiting_connection
.lock()
.remove(&resource_id.into());
tunnel.peer_connections.lock().remove(&gateway_id.into());
tunnel
.gateway_awaiting_connection

View File

@@ -31,7 +31,15 @@ impl Allocation {
let (client_to_peer_sender, client_to_peer_receiver) = mpsc::channel(MAX_BUFFERED_ITEMS);
let task = tokio::spawn(async move {
let Err(e) = forward_incoming_relay_data(relay_data_sender, client_to_peer_receiver, id, family, port).await else {
let Err(e) = forward_incoming_relay_data(
relay_data_sender,
client_to_peer_receiver,
id,
family,
port,
)
.await
else {
unreachable!()
};

View File

@@ -459,12 +459,20 @@ impl TestServer {
for expected_output in output {
let Some(actual_output) = self.server.next_command() else {
let msg = match expected_output {
Output::SendMessage((recipient, msg)) => format!("to send message {:?} to {recipient}", msg),
Output::SendMessage((recipient, msg)) => {
format!("to send message {:?} to {recipient}", msg)
}
Wake(time) => format!("to be woken at {time:?}"),
CreateAllocation(port, family) => format!("to create allocation on port {port} for address family {family}"),
FreeAllocation(port, family) => format!("to free allocation on port {port} for address family {family}"),
Output::SendChannelData((peer, _)) => format!("to send channel data from {peer} to client"),
Output::Forward((peer, _, _)) => format!("to forward data to peer {peer}")
CreateAllocation(port, family) => {
format!("to create allocation on port {port} for address family {family}")
}
FreeAllocation(port, family) => {
format!("to free allocation on port {port} for address family {family}")
}
Output::SendChannelData((peer, _)) => {
format!("to send channel data from {peer} to client")
}
Output::Forward((peer, _, _)) => format!("to forward data to peer {peer}"),
};
panic!("No commands produced but expected {msg}");

View File

@@ -1,5 +1,5 @@
[toolchain]
channel = "1.71.0"
channel = "1.72.1"
components = ["rustfmt", "clippy"]
targets = [
"x86_64-unknown-linux-musl",