Skip to content

Commit 6e39bb7

Browse files
lannlann
committed
outbound-networking: minor refactoring in allowed_hosts
Signed-off-by: Lann Martin <lann.martin@fermyon.com>
1 parent 3dbb9ce commit 6e39bb7

File tree

2 files changed

+86
-54
lines changed

2 files changed

+86
-54
lines changed

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

Lines changed: 84 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::ops::Range;
33
use anyhow::{bail, ensure, Context};
44
use spin_factors::{App, AppComponent};
55
use spin_locked_app::MetadataKey;
6+
use url::Host;
67

78
const ALLOWED_HOSTS_KEY: MetadataKey<Vec<String>> = MetadataKey::new("allowed_outbound_hosts");
89
const ALLOWED_HTTP_KEY: MetadataKey<Vec<String>> = MetadataKey::new("allowed_http_hosts");
@@ -105,10 +106,17 @@ impl AllowedHostConfig {
105106
None => rest,
106107
};
107108

109+
let port = PortConfig::parse(port, scheme)
110+
.with_context(|| format!("Invalid allowed host port {port:?}"))?;
111+
let scheme = SchemeConfig::parse(scheme)
112+
.with_context(|| format!("Invalid allowed host scheme {scheme:?}"))?;
113+
let host =
114+
HostConfig::parse(host).with_context(|| format!("Invalid allowed host {host:?}"))?;
115+
108116
Ok(Self {
109-
scheme: SchemeConfig::parse(scheme)?,
110-
host: HostConfig::parse(host)?,
111-
port: PortConfig::parse(port, scheme)?,
117+
scheme,
118+
host,
119+
port,
112120
original,
113121
})
114122
}
@@ -128,6 +136,11 @@ impl AllowedHostConfig {
128136
&self.port
129137
}
130138

139+
/// Returns true if this config is for service chaining requests.
140+
pub fn is_for_service_chaining(&self) -> bool {
141+
self.host.is_for_service_chaining()
142+
}
143+
131144
/// Returns true if the given url is allowed by this config.
132145
fn allows(&self, url: &OutboundUrl) -> bool {
133146
self.scheme.allows(&url.scheme)
@@ -172,11 +185,11 @@ impl SchemeConfig {
172185

173186
if scheme.starts_with('{') {
174187
// TODO:
175-
bail!("scheme lists are not yet supported")
188+
anyhow::bail!("scheme lists are not yet supported")
176189
}
177190

178191
if scheme.chars().any(|c| !c.is_alphabetic()) {
179-
anyhow::bail!(" scheme {scheme:?} contains non alphabetic character");
192+
anyhow::bail!("only alphabetic character(s) are allowed");
180193
}
181194

182195
Ok(Self::List(vec![scheme.into()]))
@@ -202,7 +215,7 @@ pub enum HostConfig {
202215
Any,
203216
AnySubdomain(String),
204217
ToSelf,
205-
List(Vec<String>),
218+
Literal(Host),
206219
Cidr(ip_network::IpNetwork),
207220
}
208221

@@ -227,49 +240,66 @@ impl HostConfig {
227240
return Ok(Self::Cidr(net));
228241
}
229242

230-
if matches!(host.split('/').nth(1), Some(path) if !path.is_empty()) {
231-
bail!("hosts must not contain paths");
243+
host = host.trim_end_matches('/');
244+
if host.contains('/') {
245+
bail!("must not include a path");
232246
}
233247

234248
if let Some(domain) = host.strip_prefix("*.") {
235249
if domain.contains('*') {
236-
bail!("Invalid allowed host {host}: wildcards are allowed only as prefixes");
250+
bail!("wildcards are allowed only as prefixes");
237251
}
238252
return Ok(Self::AnySubdomain(format!(".{domain}")));
239253
}
240254

241255
if host.contains('*') {
242-
bail!("Invalid allowed host {host}: wildcards are allowed only as subdomains");
256+
bail!("wildcards are allowed only as subdomains");
243257
}
244258

245-
// Remove trailing slashes
246-
host = host.trim_end_matches('/');
259+
Self::literal(host)
260+
}
247261

248-
Ok(Self::List(vec![host.into()]))
262+
/// Returns a HostConfig from the given literal host name.
263+
fn literal(host: &str) -> anyhow::Result<Self> {
264+
Ok(Self::Literal(Host::parse(host)?))
249265
}
250266

251267
/// Returns true if the given host is allowed by this config.
252268
fn allows(&self, host: &str) -> bool {
253-
match self {
254-
HostConfig::Any => true,
255-
HostConfig::AnySubdomain(suffix) => host.ends_with(suffix),
256-
HostConfig::List(l) => l.iter().any(|h| h.as_str() == host),
257-
HostConfig::ToSelf => false,
258-
HostConfig::Cidr(c) => {
259-
let Ok(ip) = host.parse::<std::net::IpAddr>() else {
260-
return false;
261-
};
262-
c.contains(ip)
269+
let host: Host = match Host::parse(host) {
270+
Ok(host) => host,
271+
Err(err) => {
272+
tracing::warn!(?err, "invalid host in HostConfig::allows");
273+
return false;
263274
}
275+
};
276+
match (self, host) {
277+
(HostConfig::Any, _) => true,
278+
(HostConfig::AnySubdomain(suffix), Host::Domain(domain)) => domain.ends_with(suffix),
279+
(HostConfig::Literal(literal), host) => host == *literal,
280+
(HostConfig::Cidr(c), Host::Ipv4(ip)) => c.contains(ip),
281+
(HostConfig::Cidr(c), Host::Ipv6(ip)) => c.contains(ip),
282+
// Note: HostConfig::ToSelf is checked with ::allow_relative
283+
_ => false,
264284
}
265285
}
266286

267287
/// Returns true if this config allows relative ("self") requests.
268288
fn allows_relative(&self) -> bool {
269289
matches!(self, Self::Any | Self::ToSelf)
270290
}
291+
292+
/// Returns true if this config is for service chaining requests.
293+
fn is_for_service_chaining(&self) -> bool {
294+
match self {
295+
Self::Literal(Host::Domain(domain)) => domain.ends_with(SERVICE_CHAINING_DOMAIN_SUFFIX),
296+
Self::AnySubdomain(suffix) => suffix == SERVICE_CHAINING_DOMAIN_SUFFIX,
297+
_ => false,
298+
}
299+
}
271300
}
272301

302+
/// Represents the port part of an allowed_outbound_hosts item.
273303
#[derive(Debug, PartialEq, Eq, Clone)]
274304
pub enum PortConfig {
275305
Any,
@@ -561,9 +591,10 @@ mod test {
561591
}
562592

563593
impl HostConfig {
564-
fn new(host: &str) -> Self {
565-
Self::List(vec![host.into()])
594+
fn unwrap_literal(host: &str) -> Self {
595+
Self::literal(host).unwrap_or_else(|_| panic!("invalid host {host:?}"))
566596
}
597+
567598
fn subdomain(domain: &str) -> Self {
568599
Self::AnySubdomain(format!(".{domain}"))
569600
}
@@ -579,8 +610,8 @@ mod test {
579610
}
580611
}
581612

582-
fn dummy_resolver() -> spin_expressions::PreparedResolver {
583-
spin_expressions::PreparedResolver::default()
613+
fn dummy_resolver() -> PreparedResolver {
614+
PreparedResolver::default()
584615
}
585616

586617
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
@@ -612,7 +643,7 @@ mod test {
612643
assert_eq!(
613644
AllowedHostConfig::new(
614645
SchemeConfig::new("http"),
615-
HostConfig::new("spin.fermyon.dev"),
646+
HostConfig::unwrap_literal("spin.fermyon.dev"),
616647
PortConfig::new(80)
617648
),
618649
AllowedHostConfig::parse("http://spin.fermyon.dev").unwrap()
@@ -622,7 +653,7 @@ mod test {
622653
AllowedHostConfig::new(
623654
SchemeConfig::new("http"),
624655
// Trailing slash is removed
625-
HostConfig::new("spin.fermyon.dev"),
656+
HostConfig::unwrap_literal("spin.fermyon.dev"),
626657
PortConfig::new(80)
627658
),
628659
AllowedHostConfig::parse("http://spin.fermyon.dev/").unwrap()
@@ -631,7 +662,7 @@ mod test {
631662
assert_eq!(
632663
AllowedHostConfig::new(
633664
SchemeConfig::new("https"),
634-
HostConfig::new("spin.fermyon.dev"),
665+
HostConfig::unwrap_literal("spin.fermyon.dev"),
635666
PortConfig::new(443)
636667
),
637668
AllowedHostConfig::parse("https://spin.fermyon.dev").unwrap()
@@ -643,23 +674,23 @@ mod test {
643674
assert_eq!(
644675
AllowedHostConfig::new(
645676
SchemeConfig::new("http"),
646-
HostConfig::new("spin.fermyon.dev"),
677+
HostConfig::unwrap_literal("spin.fermyon.dev"),
647678
PortConfig::new(4444)
648679
),
649680
AllowedHostConfig::parse("http://spin.fermyon.dev:4444").unwrap()
650681
);
651682
assert_eq!(
652683
AllowedHostConfig::new(
653684
SchemeConfig::new("http"),
654-
HostConfig::new("spin.fermyon.dev"),
685+
HostConfig::unwrap_literal("spin.fermyon.dev"),
655686
PortConfig::new(4444)
656687
),
657688
AllowedHostConfig::parse("http://spin.fermyon.dev:4444/").unwrap()
658689
);
659690
assert_eq!(
660691
AllowedHostConfig::new(
661692
SchemeConfig::new("https"),
662-
HostConfig::new("spin.fermyon.dev"),
693+
HostConfig::unwrap_literal("spin.fermyon.dev"),
663694
PortConfig::new(5555)
664695
),
665696
AllowedHostConfig::parse("https://spin.fermyon.dev:5555").unwrap()
@@ -671,7 +702,7 @@ mod test {
671702
assert_eq!(
672703
AllowedHostConfig::new(
673704
SchemeConfig::new("http"),
674-
HostConfig::new("spin.fermyon.dev"),
705+
HostConfig::unwrap_literal("spin.fermyon.dev"),
675706
PortConfig::range(4444..5555)
676707
),
677708
AllowedHostConfig::parse("http://spin.fermyon.dev:4444..5555").unwrap()
@@ -693,7 +724,7 @@ mod test {
693724
assert_eq!(
694725
AllowedHostConfig::new(
695726
SchemeConfig::Any,
696-
HostConfig::new("spin.fermyon.dev"),
727+
HostConfig::unwrap_literal("spin.fermyon.dev"),
697728
PortConfig::new(7777)
698729
),
699730
AllowedHostConfig::parse("*://spin.fermyon.dev:7777").unwrap()
@@ -718,7 +749,7 @@ mod test {
718749
assert_eq!(
719750
AllowedHostConfig::new(
720751
SchemeConfig::new("http"),
721-
HostConfig::new("localhost"),
752+
HostConfig::unwrap_literal("localhost"),
722753
PortConfig::new(80)
723754
),
724755
AllowedHostConfig::parse("http://localhost").unwrap()
@@ -727,7 +758,7 @@ mod test {
727758
assert_eq!(
728759
AllowedHostConfig::new(
729760
SchemeConfig::new("http"),
730-
HostConfig::new("localhost"),
761+
HostConfig::unwrap_literal("localhost"),
731762
PortConfig::new(3001)
732763
),
733764
AllowedHostConfig::parse("http://localhost:3001").unwrap()
@@ -751,23 +782,23 @@ mod test {
751782
assert_eq!(
752783
AllowedHostConfig::new(
753784
SchemeConfig::new("http"),
754-
HostConfig::new("192.168.1.1"),
785+
HostConfig::unwrap_literal("192.168.1.1"),
755786
PortConfig::new(80)
756787
),
757788
AllowedHostConfig::parse("http://192.168.1.1").unwrap()
758789
);
759790
assert_eq!(
760791
AllowedHostConfig::new(
761792
SchemeConfig::new("http"),
762-
HostConfig::new("192.168.1.1"),
793+
HostConfig::unwrap_literal("192.168.1.1"),
763794
PortConfig::new(3002)
764795
),
765796
AllowedHostConfig::parse("http://192.168.1.1:3002").unwrap()
766797
);
767798
assert_eq!(
768799
AllowedHostConfig::new(
769800
SchemeConfig::new("http"),
770-
HostConfig::new("[::1]"),
801+
HostConfig::unwrap_literal("[::1]"),
771802
PortConfig::new(8001)
772803
),
773804
AllowedHostConfig::parse("http://[::1]:8001").unwrap()
@@ -811,6 +842,19 @@ mod test {
811842
assert!(AllowedHostConfig::parse("http://*.fermyon.dev/a").is_err());
812843
}
813844

845+
#[test]
846+
fn test_rejects_invalid_parts() {
847+
for invalid in [
848+
"h@x://localhost",
849+
"http://invalid host",
850+
"http://inv@lid-host",
851+
"http://",
852+
"http://:80",
853+
] {
854+
AllowedHostConfig::parse(invalid).expect_err(invalid);
855+
}
856+
}
857+
814858
#[test]
815859
fn test_allowed_hosts_respects_allow_all() {
816860
assert!(AllowedHostsConfig::parse(&["insecure:allow-all"], &dummy_resolver()).is_err());

‎crates/loader/src/local.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use anyhow::{anyhow, bail, ensure, Context, Result};
44
use futures::{future::try_join_all, StreamExt};
55
use reqwest::Url;
66
use spin_common::{paths::parent_dir, sloth, ui::quoted_path};
7-
use spin_factor_outbound_networking::SERVICE_CHAINING_DOMAIN_SUFFIX;
87
use spin_locked_app::{
98
locked::{
109
self, ContentPath, ContentRef, LockedApp, LockedComponent, LockedComponentDependency,
@@ -838,19 +837,8 @@ fn requires_service_chaining(component: &spin_manifest::schema::v2::Component) -
838837
}
839838

840839
fn is_chaining_host(pattern: &str) -> bool {
841-
use spin_factor_outbound_networking::{AllowedHostConfig, HostConfig};
842-
843-
let Ok(allowed) = AllowedHostConfig::parse(pattern) else {
844-
return false;
845-
};
846-
847-
match allowed.host() {
848-
HostConfig::List(hosts) => hosts
849-
.iter()
850-
.any(|h| h.ends_with(SERVICE_CHAINING_DOMAIN_SUFFIX)),
851-
HostConfig::AnySubdomain(domain) => domain == SERVICE_CHAINING_DOMAIN_SUFFIX,
852-
_ => false,
853-
}
840+
spin_factor_outbound_networking::AllowedHostConfig::parse(pattern)
841+
.is_ok_and(|config| config.is_for_service_chaining())
854842
}
855843

856844
const SLOTH_WARNING_DELAY_MILLIS: u64 = 1250;

0 commit comments

Comments
 (0)