Skip to content

Commit 940574c

Browse files
lannlann
committed
outbound-networking: Refactor allowed_hosts
- Replace HostConfig::List(Vec<String>) with HostConfig::Literal(url::Host) - Reorganize error handling for consistency - Move logic from spin-loader to AllowedHostConfig::is_for_service_chaining Signed-off-by: Lann Martin <[email protected]>
1 parent 6cffbe2 commit 940574c

File tree

2 files changed

+70
-54
lines changed

2 files changed

+70
-54
lines changed

‎crates/loader/src/local.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ use spin_locked_app::{
1212
values::{ValuesMap, ValuesMapBuilder},
1313
};
1414
use spin_manifest::schema::v2::{self, AppManifest, KebabId, WasiFilesMount};
15-
use spin_outbound_networking_config::allowed_hosts::{
16-
AllowedHostsConfig, SERVICE_CHAINING_DOMAIN_SUFFIX,
17-
};
15+
use spin_outbound_networking_config::allowed_hosts::{AllowedHostConfig, AllowedHostsConfig};
1816
use spin_serde::DependencyName;
1917
use std::collections::BTreeMap;
2018
use tokio::{io::AsyncWriteExt, sync::Semaphore};
@@ -841,19 +839,7 @@ fn requires_service_chaining(component: &spin_manifest::schema::v2::Component) -
841839
}
842840

843841
fn is_chaining_host(pattern: &str) -> bool {
844-
use spin_outbound_networking_config::allowed_hosts::{AllowedHostConfig, HostConfig};
845-
846-
let Ok(allowed) = AllowedHostConfig::parse(pattern) else {
847-
return false;
848-
};
849-
850-
match allowed.host() {
851-
HostConfig::List(hosts) => hosts
852-
.iter()
853-
.any(|h| h.ends_with(SERVICE_CHAINING_DOMAIN_SUFFIX)),
854-
HostConfig::AnySubdomain(domain) => domain == SERVICE_CHAINING_DOMAIN_SUFFIX,
855-
_ => false,
856-
}
842+
AllowedHostConfig::parse(pattern).is_ok_and(|config| config.is_for_service_chaining())
857843
}
858844

859845
const SLOTH_WARNING_DELAY_MILLIS: u64 = 1250;

‎crates/outbound-networking-config/src/allowed_hosts.rs

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::sync::Arc;
33

44
use anyhow::{bail, ensure, Context as _};
55
use futures_util::future::{BoxFuture, Shared};
6+
use url::Host;
67

78
/// The domain used for service chaining.
89
pub const SERVICE_CHAINING_DOMAIN: &str = "spin.internal";
@@ -131,10 +132,17 @@ impl AllowedHostConfig {
131132
None => rest,
132133
};
133134

135+
let port = PortConfig::parse(port, scheme)
136+
.with_context(|| format!("Invalid allowed host port {port:?}"))?;
137+
let scheme = SchemeConfig::parse(scheme)
138+
.with_context(|| format!("Invalid allowed host scheme {scheme:?}"))?;
139+
let host =
140+
HostConfig::parse(host).with_context(|| format!("Invalid allowed host {host:?}"))?;
141+
134142
Ok(Self {
135-
scheme: SchemeConfig::parse(scheme)?,
136-
host: HostConfig::parse(host)?,
137-
port: PortConfig::parse(port, scheme)?,
143+
scheme,
144+
host,
145+
port,
138146
original,
139147
})
140148
}
@@ -151,6 +159,11 @@ impl AllowedHostConfig {
151159
&self.port
152160
}
153161

162+
/// Returns true if this config is for service chaining requests.
163+
pub fn is_for_service_chaining(&self) -> bool {
164+
self.host.is_for_service_chaining()
165+
}
166+
154167
/// Returns true if the given URL is allowed.
155168
fn allows(&self, url: &OutboundUrl) -> bool {
156169
self.scheme.allows(&url.scheme)
@@ -198,7 +211,7 @@ impl SchemeConfig {
198211
}
199212

200213
if scheme.chars().any(|c| !c.is_alphabetic()) {
201-
anyhow::bail!("scheme {scheme:?} contains non alphabetic character");
214+
anyhow::bail!("only alphabetic characters are allowed");
202215
}
203216

204217
Ok(Self::List(vec![scheme.into()]))
@@ -224,7 +237,7 @@ pub enum HostConfig {
224237
Any,
225238
AnySubdomain(String),
226239
ToSelf,
227-
List(Vec<String>),
240+
Literal(Host),
228241
Cidr(ip_network::IpNetwork),
229242
}
230243

@@ -249,47 +262,67 @@ impl HostConfig {
249262
return Ok(Self::Cidr(net));
250263
}
251264

252-
if matches!(host.split('/').nth(1), Some(path) if !path.is_empty()) {
253-
bail!("hosts must not contain paths");
265+
host = host.trim_end_matches('/');
266+
if host.contains('/') {
267+
bail!("must not include a path");
254268
}
255269

256270
if let Some(domain) = host.strip_prefix("*.") {
257271
if domain.contains('*') {
258-
bail!("Invalid allowed host {host}: wildcards are allowed only as prefixes");
272+
bail!("wildcards are allowed only as prefixes");
259273
}
260274
return Ok(Self::AnySubdomain(format!(".{domain}")));
261275
}
262276

263277
if host.contains('*') {
264-
bail!("Invalid allowed host {host}: wildcards are allowed only as subdomains");
278+
bail!("wildcards are allowed only as subdomains");
265279
}
266280

267-
// Remove trailing slashes
268-
host = host.trim_end_matches('/');
281+
Self::literal(host)
282+
}
269283

270-
Ok(Self::List(vec![host.into()]))
284+
/// Returns a HostConfig from the given literal host name.
285+
fn literal(host: &str) -> anyhow::Result<Self> {
286+
Ok(Self::Literal(Host::parse(host)?))
271287
}
272288

273289
/// Returns true if the given host is allowed.
274290
fn allows(&self, host: &str) -> bool {
275-
match self {
276-
HostConfig::Any => true,
277-
HostConfig::AnySubdomain(suffix) => host.ends_with(suffix),
278-
HostConfig::List(l) => l.iter().any(|h| h.as_str() == host),
279-
HostConfig::ToSelf => false,
280-
HostConfig::Cidr(c) => {
281-
let Ok(ip) = host.parse::<std::net::IpAddr>() else {
282-
return false;
283-
};
284-
c.contains(ip)
291+
let host: Host = match Host::parse(host) {
292+
Ok(host) => host,
293+
Err(err) => {
294+
tracing::warn!(?err, "invalid host in HostConfig::allows");
295+
return false;
285296
}
297+
};
298+
match (self, host) {
299+
(HostConfig::Any, _) => true,
300+
(HostConfig::AnySubdomain(suffix), Host::Domain(domain)) => domain.ends_with(suffix),
301+
(HostConfig::Literal(literal), host) => host == *literal,
302+
(HostConfig::Cidr(c), Host::Ipv4(ip)) => c.contains(ip),
303+
(HostConfig::Cidr(c), Host::Ipv6(ip)) => c.contains(ip),
304+
// AnySubdomain only matches domains
305+
(HostConfig::AnySubdomain(_), _) => false,
306+
// Cidr doesn't match domains
307+
(HostConfig::Cidr(_), Host::Domain(_)) => false,
308+
// ToSelf is checked separately with allow_relative
309+
(HostConfig::ToSelf, _) => false,
286310
}
287311
}
288312

289313
/// Returns true if relative ("self") requests are allowed.
290314
fn allows_relative(&self) -> bool {
291315
matches!(self, Self::Any | Self::ToSelf)
292316
}
317+
318+
/// Returns true if this config is for service chaining requests.
319+
fn is_for_service_chaining(&self) -> bool {
320+
match self {
321+
Self::Literal(Host::Domain(domain)) => domain.ends_with(SERVICE_CHAINING_DOMAIN_SUFFIX),
322+
Self::AnySubdomain(suffix) => suffix == SERVICE_CHAINING_DOMAIN_SUFFIX,
323+
_ => false,
324+
}
325+
}
293326
}
294327

295328
/// Represents the port part of an allowed_outbound_hosts item.
@@ -601,9 +634,6 @@ mod test {
601634
}
602635

603636
impl HostConfig {
604-
fn new(host: &str) -> Self {
605-
Self::List(vec![host.into()])
606-
}
607637
fn subdomain(domain: &str) -> Self {
608638
Self::AnySubdomain(format!(".{domain}"))
609639
}
@@ -652,7 +682,7 @@ mod test {
652682
assert_eq!(
653683
AllowedHostConfig::new(
654684
SchemeConfig::new("http"),
655-
HostConfig::new("spin.fermyon.dev"),
685+
HostConfig::literal("spin.fermyon.dev").unwrap(),
656686
PortConfig::new(80)
657687
),
658688
AllowedHostConfig::parse("http://spin.fermyon.dev").unwrap()
@@ -662,7 +692,7 @@ mod test {
662692
AllowedHostConfig::new(
663693
SchemeConfig::new("http"),
664694
// Trailing slash is removed
665-
HostConfig::new("spin.fermyon.dev"),
695+
HostConfig::literal("spin.fermyon.dev").unwrap(),
666696
PortConfig::new(80)
667697
),
668698
AllowedHostConfig::parse("http://spin.fermyon.dev/").unwrap()
@@ -671,7 +701,7 @@ mod test {
671701
assert_eq!(
672702
AllowedHostConfig::new(
673703
SchemeConfig::new("https"),
674-
HostConfig::new("spin.fermyon.dev"),
704+
HostConfig::literal("spin.fermyon.dev").unwrap(),
675705
PortConfig::new(443)
676706
),
677707
AllowedHostConfig::parse("https://spin.fermyon.dev").unwrap()
@@ -683,23 +713,23 @@ mod test {
683713
assert_eq!(
684714
AllowedHostConfig::new(
685715
SchemeConfig::new("http"),
686-
HostConfig::new("spin.fermyon.dev"),
716+
HostConfig::literal("spin.fermyon.dev").unwrap(),
687717
PortConfig::new(4444)
688718
),
689719
AllowedHostConfig::parse("http://spin.fermyon.dev:4444").unwrap()
690720
);
691721
assert_eq!(
692722
AllowedHostConfig::new(
693723
SchemeConfig::new("http"),
694-
HostConfig::new("spin.fermyon.dev"),
724+
HostConfig::literal("spin.fermyon.dev").unwrap(),
695725
PortConfig::new(4444)
696726
),
697727
AllowedHostConfig::parse("http://spin.fermyon.dev:4444/").unwrap()
698728
);
699729
assert_eq!(
700730
AllowedHostConfig::new(
701731
SchemeConfig::new("https"),
702-
HostConfig::new("spin.fermyon.dev"),
732+
HostConfig::literal("spin.fermyon.dev").unwrap(),
703733
PortConfig::new(5555)
704734
),
705735
AllowedHostConfig::parse("https://spin.fermyon.dev:5555").unwrap()
@@ -711,7 +741,7 @@ mod test {
711741
assert_eq!(
712742
AllowedHostConfig::new(
713743
SchemeConfig::new("http"),
714-
HostConfig::new("spin.fermyon.dev"),
744+
HostConfig::literal("spin.fermyon.dev").unwrap(),
715745
PortConfig::range(4444..5555)
716746
),
717747
AllowedHostConfig::parse("http://spin.fermyon.dev:4444..5555").unwrap()
@@ -733,7 +763,7 @@ mod test {
733763
assert_eq!(
734764
AllowedHostConfig::new(
735765
SchemeConfig::Any,
736-
HostConfig::new("spin.fermyon.dev"),
766+
HostConfig::literal("spin.fermyon.dev").unwrap(),
737767
PortConfig::new(7777)
738768
),
739769
AllowedHostConfig::parse("*://spin.fermyon.dev:7777").unwrap()
@@ -758,7 +788,7 @@ mod test {
758788
assert_eq!(
759789
AllowedHostConfig::new(
760790
SchemeConfig::new("http"),
761-
HostConfig::new("localhost"),
791+
HostConfig::literal("localhost").unwrap(),
762792
PortConfig::new(80)
763793
),
764794
AllowedHostConfig::parse("http://localhost").unwrap()
@@ -767,7 +797,7 @@ mod test {
767797
assert_eq!(
768798
AllowedHostConfig::new(
769799
SchemeConfig::new("http"),
770-
HostConfig::new("localhost"),
800+
HostConfig::literal("localhost").unwrap(),
771801
PortConfig::new(3001)
772802
),
773803
AllowedHostConfig::parse("http://localhost:3001").unwrap()
@@ -791,23 +821,23 @@ mod test {
791821
assert_eq!(
792822
AllowedHostConfig::new(
793823
SchemeConfig::new("http"),
794-
HostConfig::new("192.168.1.1"),
824+
HostConfig::literal("192.168.1.1").unwrap(),
795825
PortConfig::new(80)
796826
),
797827
AllowedHostConfig::parse("http://192.168.1.1").unwrap()
798828
);
799829
assert_eq!(
800830
AllowedHostConfig::new(
801831
SchemeConfig::new("http"),
802-
HostConfig::new("192.168.1.1"),
832+
HostConfig::literal("192.168.1.1").unwrap(),
803833
PortConfig::new(3002)
804834
),
805835
AllowedHostConfig::parse("http://192.168.1.1:3002").unwrap()
806836
);
807837
assert_eq!(
808838
AllowedHostConfig::new(
809839
SchemeConfig::new("http"),
810-
HostConfig::new("[::1]"),
840+
HostConfig::literal("[::1]").unwrap(),
811841
PortConfig::new(8001)
812842
),
813843
AllowedHostConfig::parse("http://[::1]:8001").unwrap()

0 commit comments

Comments
 (0)