Get rid of `allowed_http_hosts` in v2 manifest #1988

rylev · 2023-10-30T17:17:41Z

This removes allowed_http_hosts in favor of allowed_outbound_hosts.

allowed_outbound_hosts has become strict in that it is required to follow the form "$SCHEME://$HOST:$PORT" where $SCHEME can be "*" or a specific scheme, $HOST can be "*" or a specific host, and $PORT can be "*", a specific port, or a port range. In the future we will allow lists of schemes, hosts, and ports.

Some notes:

"insecure:allow-all" is done away with in favor of *://*:*.
Schemes are required but if you don't care you can use globs: *://example.com

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

lann · 2023-10-30T18:29:52Z

crates/loader/src/local.rs

-                .context("`allowed_outbound_hosts` is malformed")?;
-        }
+        spin_outbound_networking::AllowedHostsConfig::parse(&component.allowed_outbound_hosts)
+            .context("`allowed_outbound_hosts` is malformed")?;


Definitely low priority but it might be nice to eventually move this parsing into serde for the nice errors you get out of toml deserialization.

crates/manifest/src/allowed_http_hosts.rs

crates/manifest/tests/ui/maximal.json

itowlson · 2023-10-30T18:41:29Z

examples/rust-outbound-pg/spin.toml

@@ -12,6 +12,6 @@ component = "outbound-pg"
 [component.outbound-pg]
 environment = { DB_URL = "host=localhost user=postgres dbname=spin_dev" }
 source = "target/wasm32-wasi/release/rust_outbound_pg.wasm"
-allowed_outbound_hosts = ["localhost:5432"]
+allowed_outbound_hosts = ["postgres://localhost:5432"]


I'm not sure where this scheme comes from? There's no scheme used in the connection string.

We're not doing literal equality checking on strings: the scheme in this config signifies that you are allowed to make postgres outbound calls to that host and port (and not, for example, http calls).

itowlson · 2023-10-30T18:43:13Z

examples/wasi-http-rust-streaming-outgoing-body/spin.toml

@@ -12,7 +12,7 @@ component = "wasi-http-async"

 [component.wasi-http-async]
 source = "target/wasm32-wasi/release/wasi_http_rust_streaming_outgoing_body.wasm"
-allowed_http_hosts = ["insecure:allow-all"]
+allowed_outbound_hosts = ["https://*:*"]


Would this still allow HTTP (not HTTPS) for localhost scenarios?

I think in the current implementation it would but that seems a bit odd. The most consistent thing would be to require {http, https}://*:* (note that the list syntax is not yet implemented), but that is admittedly a bit annoying.

examples/http-tinygo-outbound-http/spin.toml

tschneidereit · 2023-10-30T21:36:11Z

We just discussed this in the Spin Up meeting, and I think we should hold off on removing the existing way of doing this this shortly before the release.

Landing this change requires a lot of documentation update, solid work on devUX to ensure that developers aren't left stranded because they don't know how to migrate, and potentially also some deeper conversation about the exact design and ergonomics of allowed_outbound_hosts.

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

rylev · 2023-10-31T10:37:07Z

Ok. I've pushed some changes that I think address most of the issues:

`allowed_http_hosts` in v2

We will continue to support allowed_http_hosts in v2 manifests but we will emit a warning when it is used.

For example, say I have the following in my manifest:

allowed_http_hosts = ["foo.com"]

This will work as expected (http traffic to foo.com will be allowed), but the following warning will be emitted:

Warning: Use of the deprecated field `allowed_http_hosts` - to fix, replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = ["https://foo.com:443"]

Default ports

The allowed_outbound_hosts syntax now allows for omitting the port. If the port is omitted, the scheme is used to determine which port is allowed. For example, an https scheme allows for port 443 and a redis scheme allows for port 6379.

A full description of the `allowed_outbound_hosts` syntax

The allowed_outbound_hosts syntax is composed of three parts: a scheme, a host, and an optional port in the following form:

$SCHEME://$HOST:$PORT

Each of these elements can be either a specific string (e.g., scheme can be 'http' or 'redis') or a glob which allows anything. Additionally ports allow for port ranges using the syntax ($START..$END). In the future, we can easily extend this to more flexible syntax such as lists (e.g., {redis, mysql}://example.com:{3000, 4000, 5000..5999}).

Scheme: The scheme decides which type of request is allowed. For example, if you specify 'redis' as the scheme than redis requests are allowed but postgres, mysql, http, etc. are not. Note that this is independent of whether the url the user is using in their code has the scheme or not. Making a request from the user's code to 'example.com' or 'redis://example.com' are both permitted when the allowed_outbound_hosts list includes 'redis://example.com'.
Host: the host must match exactly between the url the user provides to an outbound networking interface being used and the configuration in allowed_outbound_hosts. For example Redis requests to 'example.com' are permitted by allowed_outbound_hosts = ["redis://example.com"] but requests to 'subdomain.example.com' are not (even if 'subdomain.example.com' redirects to 'example.com').
- There is one special host "self" that allows making requests back to the current application.
Port: The port used to make the request must be in the range specified by the config. For example, a request to 'example.com:4001' is allowed by allowed_outbound_hosts = ["*://example.com:4001"] and allowed_outbound_hosts = ["*://example.com:4000..4999"] but is not allowed by allowed_outbound_hosts = ["*://example.com:4000"].

crates/loader/tests/ui/insecure-allow-all-with-invalid-url.err

crates/manifest/src/schema/v2.rs

crates/outbound-networking/src/lib.rs

crates/outbound-http/src/host_impl.rs

crates/outbound-networking/src/lib.rs

examples/http-tinygo-outbound-http/spin.toml

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

vdice · 2023-10-31T16:17:15Z

crates/manifest/src/schema/v2.rs

+        if !normalized.is_empty() {
+            terminal::warn!(
+                "Use of the deprecated field `allowed_http_hosts` - to fix, \
+            replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",


suggestion: thoughts on reducing to just the following?

Suggested change

replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",

replace with `allowed_outbound_hosts = {normalized:?}`",

@vdice maybe

Suggested change

replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",

replace `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",

so as to be absolutely specific?

If there are no other changes, I might sneak this in to a follow-up PR to avoid having to run CI again, but otherwise 👍

itowlson

Thanks for working through what must at times have been frustrating feedback. I've not fully grasped the internals of the PR, but it looks good, and the behaviour gives us the long-term plan going forward while making users following older content successful. The ideal outcome.

itowlson · 2023-10-31T18:31:57Z

I will update the docs to align with this. (Thanks for picking up the templates!)

itowlson · 2023-10-31T19:52:04Z

@vdice heads up for the JS and Python templates

rylev added 2 commits October 30, 2023 12:58

Remove allow_http_hosts in favor of allowed_outbound_hosts

5e889c1

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

Fully represent v1 manifests in v2 manifest

4b81811

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

rylev requested review from lann and itowlson October 30, 2023 17:17

lann reviewed Oct 30, 2023

View reviewed changes

itowlson reviewed Oct 30, 2023

View reviewed changes

Still allow deprecated 'allowed_http_hosts'

641555a

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

rylev force-pushed the no-more-http-hosts branch from 4a52b09 to a875503 Compare October 31, 2023 10:48

lann reviewed Oct 31, 2023

View reviewed changes

crates/loader/tests/ui/insecure-allow-all-with-invalid-url.err Outdated Show resolved Hide resolved

rylev force-pushed the no-more-http-hosts branch from a875503 to b340a3a Compare October 31, 2023 13:19

lann reviewed Oct 31, 2023

View reviewed changes

lann mentioned this pull request Oct 31, 2023

chore(examples/tinygo-outbound-*): add allowed_outbound_hosts config #1978

Merged

rylev force-pushed the no-more-http-hosts branch 3 times, most recently from c946bd8 to f9f9947 Compare October 31, 2023 15:08

Don't require a port

6785e96

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>

rylev force-pushed the no-more-http-hosts branch from f9f9947 to 6785e96 Compare October 31, 2023 15:44

vdice reviewed Oct 31, 2023

View reviewed changes

lann approved these changes Oct 31, 2023

View reviewed changes

itowlson approved these changes Oct 31, 2023

View reviewed changes

rylev merged commit 25de63b into main Oct 31, 2023
9 checks passed

rylev deleted the no-more-http-hosts branch October 31, 2023 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get rid of `allowed_http_hosts` in v2 manifest #1988

Get rid of `allowed_http_hosts` in v2 manifest #1988

rylev commented Oct 30, 2023 •

edited by lann

Loading

lann Oct 30, 2023

itowlson Oct 30, 2023

rylev Oct 31, 2023

itowlson Oct 30, 2023

rylev Oct 31, 2023

tschneidereit commented Oct 30, 2023

rylev commented Oct 31, 2023 •

edited

Loading

vdice Oct 31, 2023

itowlson Oct 31, 2023

rylev Oct 31, 2023

itowlson left a comment

itowlson commented Oct 31, 2023

itowlson commented Oct 31, 2023

	replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",
	replace with `allowed_outbound_hosts = {normalized:?}`",

	replace the use of `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",
	replace `allowed_http_hosts` with `allowed_outbound_hosts = {normalized:?}`",

Get rid of allowed_http_hosts in v2 manifest #1988

Get rid of allowed_http_hosts in v2 manifest #1988

Conversation

rylev commented Oct 30, 2023 • edited by lann Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tschneidereit commented Oct 30, 2023

rylev commented Oct 31, 2023 • edited Loading

allowed_http_hosts in v2

Default ports

A full description of the allowed_outbound_hosts syntax

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itowlson left a comment

Choose a reason for hiding this comment

itowlson commented Oct 31, 2023

itowlson commented Oct 31, 2023

Get rid of `allowed_http_hosts` in v2 manifest #1988

Get rid of `allowed_http_hosts` in v2 manifest #1988

rylev commented Oct 30, 2023 •

edited by lann

Loading

rylev commented Oct 31, 2023 •

edited

Loading

`allowed_http_hosts` in v2

A full description of the `allowed_outbound_hosts` syntax