Skip to content

Commit 2c42eff

Browse files
feat(iroh-relay)!: implement authentication (#3086)
## Description Design RFC: https://www.notion.so/number-zero/Relay-Authentication-16f5df1306fb80ac9e31c1ccb04e026b?pvs=4 ## Breaking Changes - added: field `access` to `iroh_relay::server::RelayConfig` ## Notes & open questions - This reuses the `health` frame in the relay protocol to indicate the authentcation issue. The frame was previously never sent in our code, but is now more explicitly interpreted to disconnect from the server. - ~~Should the callback be `async`? If this does more complex things like access a DB we probably want this to be `async`.~~ it is now async ## Change checklist - [ ] Self-review. - [ ] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [ ] Tests if relevant. - [ ] All breaking changes documented.
1 parent 024ab7f commit 2c42eff

File tree

7 files changed

+263
-12
lines changed

7 files changed

+263
-12
lines changed

iroh-relay/src/main.rs

+85
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ use std::{
1111

1212
use anyhow::{bail, Context as _, Result};
1313
use clap::Parser;
14+
use futures_lite::FutureExt;
15+
use iroh_base::NodeId;
1416
use iroh_relay::{
1517
defaults::{
1618
DEFAULT_HTTPS_PORT, DEFAULT_HTTP_PORT, DEFAULT_METRICS_PORT, DEFAULT_RELAY_QUIC_PORT,
@@ -170,6 +172,59 @@ struct Config {
170172
metrics_bind_addr: Option<SocketAddr>,
171173
/// The capacity of the key cache.
172174
key_cache_capacity: Option<usize>,
175+
/// Access control for relaying connections.
176+
///
177+
/// This controls which nodes are allowed to relay connections, other endpoints, like STUN are not controlled by this.
178+
#[serde(default)]
179+
access: AccessConfig,
180+
}
181+
182+
#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)]
183+
#[serde(rename_all = "lowercase")]
184+
enum AccessConfig {
185+
/// Allows everyone
186+
#[default]
187+
Everyone,
188+
/// Allows only these nodes.
189+
Allowlist(Vec<NodeId>),
190+
/// Allows everyone, except these nodes.
191+
Denylist(Vec<NodeId>),
192+
}
193+
194+
impl From<AccessConfig> for iroh_relay::server::AccessConfig {
195+
fn from(cfg: AccessConfig) -> Self {
196+
match cfg {
197+
AccessConfig::Everyone => iroh_relay::server::AccessConfig::Everyone,
198+
AccessConfig::Allowlist(allow_list) => {
199+
let allow_list = Arc::new(allow_list);
200+
iroh_relay::server::AccessConfig::Restricted(Box::new(move |node_id| {
201+
let allow_list = allow_list.clone();
202+
async move {
203+
if allow_list.contains(&node_id) {
204+
iroh_relay::server::Access::Allow
205+
} else {
206+
iroh_relay::server::Access::Deny
207+
}
208+
}
209+
.boxed()
210+
}))
211+
}
212+
AccessConfig::Denylist(deny_list) => {
213+
let deny_list = Arc::new(deny_list);
214+
iroh_relay::server::AccessConfig::Restricted(Box::new(move |node_id| {
215+
let deny_list = deny_list.clone();
216+
async move {
217+
if deny_list.contains(&node_id) {
218+
iroh_relay::server::Access::Deny
219+
} else {
220+
iroh_relay::server::Access::Allow
221+
}
222+
}
223+
.boxed()
224+
}))
225+
}
226+
}
227+
}
173228
}
174229

175230
impl Config {
@@ -202,6 +257,7 @@ impl Default for Config {
202257
enable_metrics: cfg_defaults::enable_metrics(),
203258
metrics_bind_addr: None,
204259
key_cache_capacity: Default::default(),
260+
access: AccessConfig::Everyone,
205261
}
206262
}
207263
}
@@ -574,7 +630,9 @@ async fn build_relay_config(cfg: Config) -> Result<relay::ServerConfig<std::io::
574630
tls: relay_tls.and_then(|tls| if dangerous_http_only { None } else { Some(tls) }),
575631
limits,
576632
key_cache_capacity: cfg.key_cache_capacity,
633+
access: cfg.access.clone().into(),
577634
};
635+
578636
let stun_config = relay::StunConfig {
579637
bind_addr: cfg.stun_bind_addr(),
580638
};
@@ -639,6 +697,9 @@ mod metrics {
639697
mod tests {
640698
use std::num::NonZeroU32;
641699

700+
use iroh_base::SecretKey;
701+
use rand::SeedableRng;
702+
use rand_chacha::ChaCha8Rng;
642703
use testresult::TestResult;
643704

644705
use super::*;
@@ -676,4 +737,28 @@ mod tests {
676737

677738
Ok(())
678739
}
740+
741+
#[tokio::test]
742+
async fn test_access_config() -> TestResult {
743+
let config = "
744+
access = \"everyone\"
745+
";
746+
let config = Config::from_str(config)?;
747+
assert_eq!(config.access, AccessConfig::Everyone);
748+
749+
let mut rng = ChaCha8Rng::seed_from_u64(0);
750+
let node_id = SecretKey::generate(&mut rng).public();
751+
752+
let config = format!(
753+
"
754+
access.allowlist = [
755+
\"{node_id}\",
756+
]
757+
"
758+
);
759+
let config = Config::from_str(dbg!(&config))?;
760+
assert_eq!(config.access, AccessConfig::Allowlist(vec![node_id]));
761+
762+
Ok(())
763+
}
679764
}

iroh-relay/src/protos/relay.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,9 @@ pub(crate) enum FrameType {
9696
/// 8 byte payload, the contents of ping being replied to
9797
Pong = 13,
9898
/// Sent from server to client to tell the client if their connection is
99-
/// unhealthy somehow. Currently the only unhealthy state is whether the
100-
/// connection is detected as a duplicate.
101-
/// The entire frame body is the text of the error message. An empty message
102-
/// clears the error state.
99+
/// unhealthy somehow.
100+
///
101+
/// Currently this is used to indicate that the connection was closed because of authentication issues.
103102
Health = 14,
104103

105104
/// Sent from server to client for the server to declare that it's restarting.

iroh-relay/src/server.rs

+132-1
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ use std::{fmt, future::Future, net::SocketAddr, num::NonZeroU32, pin::Pin, sync:
2020

2121
use anyhow::{anyhow, bail, Context, Result};
2222
use derive_more::Debug;
23-
use futures_lite::StreamExt;
23+
use futures_lite::{future::Boxed, StreamExt};
2424
use http::{
2525
response::Builder as ResponseBuilder, HeaderMap, Method, Request, Response, StatusCode,
2626
};
2727
use hyper::body::Incoming;
28+
use iroh_base::NodeId;
2829
#[cfg(feature = "test-utils")]
2930
use iroh_base::RelayUrl;
3031
use iroh_metrics::inc;
@@ -120,6 +121,40 @@ pub struct RelayConfig<EC: fmt::Debug, EA: fmt::Debug = EC> {
120121
pub limits: Limits,
121122
/// Key cache capacity.
122123
pub key_cache_capacity: Option<usize>,
124+
/// Access configuration.
125+
pub access: AccessConfig,
126+
}
127+
128+
/// Controls which nodes are allowed to use the relay.
129+
#[derive(derive_more::Debug)]
130+
pub enum AccessConfig {
131+
/// Everyone
132+
Everyone,
133+
/// Only nodes for which the function returns `Access::Allow`.
134+
#[debug("restricted")]
135+
Restricted(Box<dyn Fn(NodeId) -> Boxed<Access> + Send + Sync + 'static>),
136+
}
137+
138+
impl AccessConfig {
139+
/// Is this node allowed?
140+
pub async fn is_allowed(&self, node: NodeId) -> bool {
141+
match self {
142+
Self::Everyone => true,
143+
Self::Restricted(check) => {
144+
let res = check(node).await;
145+
matches!(res, Access::Allow)
146+
}
147+
}
148+
}
149+
}
150+
151+
/// Access restriction for a node.
152+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
153+
pub enum Access {
154+
/// Access is allowed.
155+
Allow,
156+
/// Access is denied.
157+
Deny,
123158
}
124159

125160
/// Configuration for the STUN server.
@@ -318,6 +353,7 @@ impl Server {
318353
let mut builder = http_server::ServerBuilder::new(relay_bind_addr)
319354
.headers(headers)
320355
.key_cache_capacity(key_cache_capacity)
356+
.access(relay_config.access)
321357
.request_handler(Method::GET, "/", Box::new(root_handler))
322358
.request_handler(Method::GET, "/index.html", Box::new(root_handler))
323359
.request_handler(Method::GET, RELAY_PROBE_PATH, Box::new(probe_handler))
@@ -772,6 +808,7 @@ mod tests {
772808
use std::{net::Ipv4Addr, time::Duration};
773809

774810
use bytes::Bytes;
811+
use futures_lite::FutureExt;
775812
use futures_util::SinkExt;
776813
use http::header::UPGRADE;
777814
use iroh_base::{NodeId, SecretKey};
@@ -790,6 +827,7 @@ mod tests {
790827
tls: None,
791828
limits: Default::default(),
792829
key_cache_capacity: Some(1024),
830+
access: AccessConfig::Everyone,
793831
}),
794832
quic: None,
795833
stun: None,
@@ -840,6 +878,7 @@ mod tests {
840878
tls: None,
841879
limits: Default::default(),
842880
key_cache_capacity: Some(1024),
881+
access: AccessConfig::Everyone,
843882
}),
844883
stun: None,
845884
quic: None,
@@ -1106,4 +1145,96 @@ mod tests {
11061145
assert_eq!(txid, txid_back);
11071146
assert_eq!(response_addr, socket.local_addr().unwrap());
11081147
}
1148+
1149+
#[tokio::test]
1150+
async fn test_relay_access_control() -> Result<()> {
1151+
let _guard = iroh_test::logging::setup();
1152+
1153+
let a_secret_key = SecretKey::generate(rand::thread_rng());
1154+
let a_key = a_secret_key.public();
1155+
1156+
let server = Server::spawn(ServerConfig::<(), ()> {
1157+
relay: Some(RelayConfig::<(), ()> {
1158+
http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
1159+
tls: None,
1160+
limits: Default::default(),
1161+
key_cache_capacity: Some(1024),
1162+
access: AccessConfig::Restricted(Box::new(move |node_id| {
1163+
async move {
1164+
info!("checking {}", node_id);
1165+
// reject node a
1166+
if node_id == a_key {
1167+
Access::Deny
1168+
} else {
1169+
Access::Allow
1170+
}
1171+
}
1172+
.boxed()
1173+
})),
1174+
}),
1175+
quic: None,
1176+
stun: None,
1177+
metrics_addr: None,
1178+
})
1179+
.await
1180+
.unwrap();
1181+
let relay_url = format!("http://{}", server.http_addr().unwrap());
1182+
let relay_url: RelayUrl = relay_url.parse()?;
1183+
1184+
// set up client a
1185+
let resolver = crate::dns::default_resolver().clone();
1186+
let mut client_a = ClientBuilder::new(relay_url.clone(), a_secret_key, resolver)
1187+
.connect()
1188+
.await?;
1189+
1190+
// the next message should be the rejection of the connection
1191+
tokio::time::timeout(Duration::from_millis(500), async move {
1192+
match client_a.next().await.unwrap().unwrap() {
1193+
ReceivedMessage::Health { problem } => {
1194+
assert_eq!(problem, Some("not authenticated".to_string()));
1195+
}
1196+
msg => {
1197+
panic!("other msg: {:?}", msg);
1198+
}
1199+
}
1200+
})
1201+
.await?;
1202+
1203+
// test that another client has access
1204+
1205+
// set up client b
1206+
let b_secret_key = SecretKey::generate(rand::thread_rng());
1207+
let b_key = b_secret_key.public();
1208+
1209+
let resolver = crate::dns::default_resolver().clone();
1210+
let mut client_b = ClientBuilder::new(relay_url.clone(), b_secret_key, resolver)
1211+
.connect()
1212+
.await?;
1213+
1214+
// set up client c
1215+
let c_secret_key = SecretKey::generate(rand::thread_rng());
1216+
let c_key = c_secret_key.public();
1217+
1218+
let resolver = crate::dns::default_resolver().clone();
1219+
let mut client_c = ClientBuilder::new(relay_url.clone(), c_secret_key, resolver)
1220+
.connect()
1221+
.await?;
1222+
1223+
// send message from b to c
1224+
let msg = Bytes::from("hello, c");
1225+
let res = try_send_recv(&mut client_b, &mut client_c, c_key, msg.clone()).await?;
1226+
1227+
if let ReceivedMessage::ReceivedPacket {
1228+
remote_node_id,
1229+
data,
1230+
} = res
1231+
{
1232+
assert_eq!(b_key, remote_node_id);
1233+
assert_eq!(msg, data);
1234+
} else {
1235+
panic!("client_c received unexpected message {res:?}");
1236+
}
1237+
1238+
Ok(())
1239+
}
11091240
}

iroh-relay/src/server/client.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use std::{future::Future, num::NonZeroU32, pin::Pin, sync::Arc, task::Poll, time::Duration};
44

5-
use anyhow::{Context, Result};
5+
use anyhow::{bail, Context, Result};
66
use bytes::Bytes;
77
use futures_lite::FutureExt;
88
use futures_sink::Sink;
@@ -333,8 +333,8 @@ impl Actor {
333333
self.write_frame(Frame::Pong { data }).await?;
334334
inc!(Metrics, sent_pong);
335335
}
336-
Frame::Health { .. } => {
337-
inc!(Metrics, other_packets_recv);
336+
Frame::Health { problem } => {
337+
bail!("server issue: {:?}", problem);
338338
}
339339
_ => {
340340
inc!(Metrics, unknown_frames);

0 commit comments

Comments
 (0)