From 21e80252f048cd711f14a5c227b2112f82673f11 Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Mon, 6 Nov 2023 20:05:34 +0100 Subject: [PATCH] wasi-sockets: Simplify ip name lookup interface (#7483) * Refactor ip_name_lookup test * Update ip-name-lookup::resolve-addresses - Remove the non-essential parameters - Lift the restriction against parsing IP addresses. Implementations would still have to parse IP addresses to decide whether or not to return an error * Deduplicate to_canonical --- .../src/bin/preview2_ip_name_lookup.rs | 90 ++++++++++++------ crates/test-programs/src/sockets.rs | 26 ++++++ .../wit/deps/sockets/ip-name-lookup.wit | 30 ++---- crates/wasi/src/preview2/host/mod.rs | 2 +- crates/wasi/src/preview2/host/network.rs | 34 +++---- crates/wasi/src/preview2/ip_name_lookup.rs | 93 +++++++++---------- crates/wasi/src/preview2/network.rs | 21 +++++ .../wasi/wit/deps/sockets/ip-name-lookup.wit | 30 ++---- 8 files changed, 187 insertions(+), 139 deletions(-) diff --git a/crates/test-programs/src/bin/preview2_ip_name_lookup.rs b/crates/test-programs/src/bin/preview2_ip_name_lookup.rs index 29c851a44b6f..dbad2e807d65 100644 --- a/crates/test-programs/src/bin/preview2_ip_name_lookup.rs +++ b/crates/test-programs/src/bin/preview2_ip_name_lookup.rs @@ -1,36 +1,70 @@ -use test_programs::wasi::clocks::*; -use test_programs::wasi::io::*; +use test_programs::wasi::sockets::network::{ErrorCode, IpAddress}; use test_programs::wasi::sockets::*; fn main() { - let network = instance_network::instance_network(); + // Valid domains + resolve("localhost").unwrap(); + resolve("example.com").unwrap(); + resolve("münchen.de").unwrap(); + + // Valid IP addresses + assert_eq!(resolve_one("0.0.0.0").unwrap(), IpAddress::IPV4_UNSPECIFIED); + assert_eq!(resolve_one("127.0.0.1").unwrap(), IpAddress::IPV4_LOOPBACK); + assert_eq!( + resolve_one("192.0.2.0").unwrap(), + IpAddress::Ipv4((192, 0, 2, 0)) + ); + assert_eq!(resolve_one("::").unwrap(), IpAddress::IPV6_UNSPECIFIED); + assert_eq!(resolve_one("::1").unwrap(), IpAddress::IPV6_LOOPBACK); + assert_eq!(resolve_one("[::]").unwrap(), IpAddress::IPV6_UNSPECIFIED); + assert_eq!( + resolve_one("2001:0db8:0:0:0:0:0:0").unwrap(), + IpAddress::Ipv6((0x2001, 0x0db8, 0, 0, 0, 0, 0, 0)) + ); + assert_eq!( + resolve_one("dead:beef::").unwrap(), + IpAddress::Ipv6((0xdead, 0xbeef, 0, 0, 0, 0, 0, 0)) + ); + assert_eq!( + resolve_one("dead:beef::0").unwrap(), + IpAddress::Ipv6((0xdead, 0xbeef, 0, 0, 0, 0, 0, 0)) + ); + assert_eq!( + resolve_one("DEAD:BEEF::0").unwrap(), + IpAddress::Ipv6((0xdead, 0xbeef, 0, 0, 0, 0, 0, 0)) + ); - let addresses = - ip_name_lookup::resolve_addresses(&network, "example.com", None, false).unwrap(); - let pollable = addresses.subscribe(); - pollable.block(); - assert!(addresses.resolve_next_address().is_ok()); + // Invalid inputs + assert_eq!(resolve("").unwrap_err(), ErrorCode::InvalidArgument); + assert_eq!(resolve(" ").unwrap_err(), ErrorCode::InvalidArgument); + assert_eq!(resolve("a.b<&>").unwrap_err(), ErrorCode::InvalidArgument); + assert_eq!( + resolve("127.0.0.1:80").unwrap_err(), + ErrorCode::InvalidArgument + ); + assert_eq!(resolve("[::]:80").unwrap_err(), ErrorCode::InvalidArgument); + assert_eq!( + resolve("http://example.com/").unwrap_err(), + ErrorCode::InvalidArgument + ); +} - let result = ip_name_lookup::resolve_addresses(&network, "a.b<&>", None, false); - assert!(matches!(result, Err(network::ErrorCode::InvalidArgument))); +fn resolve(name: &str) -> Result, ErrorCode> { + let network = instance_network::instance_network(); - // Try resolving a valid address and ensure that it eventually terminates. - // To help prevent this test from being flaky this additionally times out - // the resolution and allows errors. - let addresses = ip_name_lookup::resolve_addresses(&network, "github.com", None, false).unwrap(); - let lookup = addresses.subscribe(); - let timeout = monotonic_clock::subscribe_duration(1_000_000_000); - let ready = poll::poll(&[&lookup, &timeout]); - assert!(ready.len() > 0); - match ready[0] { - 0 => loop { - match addresses.resolve_next_address() { - Ok(Some(_)) => {} - Ok(None) => break, - Err(_) => break, - } - }, - 1 => {} - _ => unreachable!(), + match network.blocking_resolve_addresses(name) { + // The following error codes signal that the input passed validation + // and a lookup was actually attempted, but failed. Ignore these to + // make the CI tests less flaky: + Err( + ErrorCode::NameUnresolvable + | ErrorCode::TemporaryResolverFailure + | ErrorCode::PermanentResolverFailure, + ) => Ok(vec![]), + r => r, } } + +fn resolve_one(name: &str) -> Result { + Ok(resolve(name)?.first().unwrap().to_owned()) +} diff --git a/crates/test-programs/src/sockets.rs b/crates/test-programs/src/sockets.rs index 55426cdf763b..1d0e7f0895f3 100644 --- a/crates/test-programs/src/sockets.rs +++ b/crates/test-programs/src/sockets.rs @@ -2,6 +2,7 @@ use crate::wasi::clocks::monotonic_clock; use crate::wasi::io::poll::{self, Pollable}; use crate::wasi::io::streams::{InputStream, OutputStream, StreamError}; use crate::wasi::sockets::instance_network; +use crate::wasi::sockets::ip_name_lookup; use crate::wasi::sockets::network::{ ErrorCode, IpAddress, IpAddressFamily, IpSocketAddress, Ipv4SocketAddress, Ipv6SocketAddress, Network, @@ -53,6 +54,31 @@ impl Network { pub fn default() -> Network { instance_network::instance_network() } + + pub fn blocking_resolve_addresses(&self, name: &str) -> Result, ErrorCode> { + let stream = ip_name_lookup::resolve_addresses(&self, name)?; + + let timeout = monotonic_clock::subscribe_duration(TIMEOUT_NS); + let pollable = stream.subscribe(); + + let mut addresses = vec![]; + + loop { + match stream.resolve_next_address() { + Ok(Some(addr)) => { + addresses.push(addr); + } + Ok(None) => match addresses[..] { + [] => return Err(ErrorCode::NameUnresolvable), + _ => return Ok(addresses), + }, + Err(ErrorCode::WouldBlock) => { + pollable.wait_until(&timeout)?; + } + Err(err) => return Err(err), + } + } + } } impl TcpSocket { diff --git a/crates/wasi-http/wit/deps/sockets/ip-name-lookup.wit b/crates/wasi-http/wit/deps/sockets/ip-name-lookup.wit index 5ffe2a217c61..270e70258d89 100644 --- a/crates/wasi-http/wit/deps/sockets/ip-name-lookup.wit +++ b/crates/wasi-http/wit/deps/sockets/ip-name-lookup.wit @@ -1,40 +1,30 @@ interface ip-name-lookup { use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; - use network.{network, error-code, ip-address, ip-address-family}; + use network.{network, error-code, ip-address}; /// Resolve an internet host name to a list of IP addresses. /// - /// See the wasi-socket proposal README.md for a comparison with getaddrinfo. - /// - /// # Parameters - /// - `name`: The name to look up. IP addresses are not allowed. Unicode domain names are automatically converted - /// to ASCII using IDNA encoding. - /// - `address-family`: If provided, limit the results to addresses of this specific address family. - /// - `include-unavailable`: When set to true, this function will also return addresses of which the runtime - /// thinks (or knows) can't be connected to at the moment. For example, this will return IPv6 addresses on - /// systems without an active IPv6 interface. Notes: - /// - Even when no public IPv6 interfaces are present or active, names like "localhost" can still resolve to an IPv6 address. - /// - Whatever is "available" or "unavailable" is volatile and can change everytime a network cable is unplugged. + /// Unicode domain names are automatically converted to ASCII using IDNA encoding. + /// If the input is an IP address string, the address is parsed and returned + /// as-is without making any external requests. /// - /// This function never blocks. It either immediately fails or immediately returns successfully with a `resolve-address-stream` - /// that can be used to (asynchronously) fetch the results. + /// See the wasi-socket proposal README.md for a comparison with getaddrinfo. /// - /// At the moment, the stream never completes successfully with 0 items. Ie. the first call - /// to `resolve-next-address` never returns `ok(none)`. This may change in the future. + /// This function never blocks. It either immediately fails or immediately + /// returns successfully with a `resolve-address-stream` that can be used + /// to (asynchronously) fetch the results. /// /// # Typical errors - /// - `invalid-argument`: `name` is a syntactically invalid domain name. - /// - `invalid-argument`: `name` is an IP address. - /// - `not-supported`: The specified `address-family` is not supported. (EAI_FAMILY) + /// - `invalid-argument`: `name` is a syntactically invalid domain name or IP address. /// /// # References: /// - /// - /// - /// - - resolve-addresses: func(network: borrow, name: string, address-family: option, include-unavailable: bool) -> result; + resolve-addresses: func(network: borrow, name: string) -> result; resource resolve-address-stream { /// Returns the next address from the resolver. diff --git a/crates/wasi/src/preview2/host/mod.rs b/crates/wasi/src/preview2/host/mod.rs index 651d2cd38e0c..7aa4a8741a9b 100644 --- a/crates/wasi/src/preview2/host/mod.rs +++ b/crates/wasi/src/preview2/host/mod.rs @@ -4,7 +4,7 @@ mod exit; pub(crate) mod filesystem; mod instance_network; mod io; -mod network; +pub(crate) mod network; mod random; mod tcp; mod tcp_create_socket; diff --git a/crates/wasi/src/preview2/host/network.rs b/crates/wasi/src/preview2/host/network.rs index 5233149f4491..ce6f2f1556ce 100644 --- a/crates/wasi/src/preview2/host/network.rs +++ b/crates/wasi/src/preview2/host/network.rs @@ -1,7 +1,8 @@ use crate::preview2::bindings::sockets::network::{ - self, ErrorCode, IpAddressFamily, IpSocketAddress, Ipv4Address, Ipv4SocketAddress, Ipv6Address, + self, ErrorCode, IpAddress, IpAddressFamily, IpSocketAddress, Ipv4SocketAddress, Ipv6SocketAddress, }; +use crate::preview2::network::{from_ipv4_addr, from_ipv6_addr, to_ipv4_addr, to_ipv6_addr}; use crate::preview2::{SocketError, WasiView}; use rustix::io::Errno; use std::io; @@ -104,6 +105,15 @@ impl From for ErrorCode { } } +impl From for IpAddress { + fn from(addr: std::net::IpAddr) -> Self { + match addr { + std::net::IpAddr::V4(v4) => Self::Ipv4(from_ipv4_addr(v4)), + std::net::IpAddr::V6(v6) => Self::Ipv6(from_ipv6_addr(v6)), + } + } +} + impl From for std::net::SocketAddr { fn from(addr: IpSocketAddress) -> Self { match addr { @@ -159,26 +169,6 @@ impl From for Ipv6SocketAddress { } } -fn to_ipv4_addr(addr: Ipv4Address) -> std::net::Ipv4Addr { - let (x0, x1, x2, x3) = addr; - std::net::Ipv4Addr::new(x0, x1, x2, x3) -} - -fn from_ipv4_addr(addr: std::net::Ipv4Addr) -> Ipv4Address { - let [x0, x1, x2, x3] = addr.octets(); - (x0, x1, x2, x3) -} - -fn to_ipv6_addr(addr: Ipv6Address) -> std::net::Ipv6Addr { - let (x0, x1, x2, x3, x4, x5, x6, x7) = addr; - std::net::Ipv6Addr::new(x0, x1, x2, x3, x4, x5, x6, x7) -} - -fn from_ipv6_addr(addr: std::net::Ipv6Addr) -> Ipv6Address { - let [x0, x1, x2, x3, x4, x5, x6, x7] = addr.segments(); - (x0, x1, x2, x3, x4, x5, x6, x7) -} - impl std::net::ToSocketAddrs for IpSocketAddress { type Iter = ::Iter; @@ -288,7 +278,7 @@ pub(crate) mod util { } // Can be removed once `IpAddr::to_canonical` becomes stable. - fn to_canonical(addr: &IpAddr) -> IpAddr { + pub fn to_canonical(addr: &IpAddr) -> IpAddr { match addr { IpAddr::V4(ipv4) => IpAddr::V4(*ipv4), IpAddr::V6(ipv6) => { diff --git a/crates/wasi/src/preview2/ip_name_lookup.rs b/crates/wasi/src/preview2/ip_name_lookup.rs index b832bd5017c7..ab69f8f1bf9c 100644 --- a/crates/wasi/src/preview2/ip_name_lookup.rs +++ b/crates/wasi/src/preview2/ip_name_lookup.rs @@ -1,14 +1,18 @@ use crate::preview2::bindings::sockets::ip_name_lookup::{Host, HostResolveAddressStream}; -use crate::preview2::bindings::sockets::network::{ErrorCode, IpAddress, IpAddressFamily, Network}; +use crate::preview2::bindings::sockets::network::{ErrorCode, IpAddress, Network}; +use crate::preview2::host::network::util; use crate::preview2::poll::{subscribe, Pollable, Subscribe}; use crate::preview2::{spawn_blocking, AbortOnDropJoinHandle, SocketError, WasiView}; use anyhow::Result; use std::mem; -use std::net::{SocketAddr, ToSocketAddrs}; +use std::net::{Ipv6Addr, ToSocketAddrs}; use std::pin::Pin; +use std::str::FromStr; use std::vec; use wasmtime::component::Resource; +use super::network::{from_ipv4_addr, from_ipv6_addr}; + pub enum ResolveAddressStream { Waiting(AbortOnDropJoinHandle, SocketError>>), Done(Result, SocketError>), @@ -20,60 +24,16 @@ impl Host for T { &mut self, network: Resource, name: String, - family: Option, - include_unavailable: bool, ) -> Result, SocketError> { let network = self.table().get(&network)?; - // `Host::parse` serves us two functions: - // 1. validate the input is not an IP address, - // 2. convert unicode domains to punycode. - let name = match url::Host::parse(&name).map_err(|_| ErrorCode::InvalidArgument)? { - url::Host::Domain(name) => name, - url::Host::Ipv4(_) => return Err(ErrorCode::InvalidArgument.into()), - url::Host::Ipv6(_) => return Err(ErrorCode::InvalidArgument.into()), - }; + let host = parse(&name)?; if !network.allow_ip_name_lookup { return Err(ErrorCode::PermanentResolverFailure.into()); } - // ignored for now, should probably have a future PR to actually take - // this into account. This would require invoking `getaddrinfo` directly - // rather than using the standard library to do it for us. - let _ = include_unavailable; - - // For now use the standard library to perform actual resolution through - // the usage of the `ToSocketAddrs` trait. This blocks the current - // thread, so use `spawn_blocking`. Finally note that this is only - // resolving names, not ports, so force the port to be 0. - let task = spawn_blocking(move || -> Result, SocketError> { - let result = (name.as_str(), 0) - .to_socket_addrs() - .map_err(|_| ErrorCode::NameUnresolvable)?; // If/when we use `getaddrinfo` directly, map the error properly. - Ok(result - .filter_map(|addr| { - // In lieu of preventing these addresses from being resolved - // in the first place, filter them out here. - match addr { - SocketAddr::V4(addr) => match family { - None | Some(IpAddressFamily::Ipv4) => { - let [a, b, c, d] = addr.ip().octets(); - Some(IpAddress::Ipv4((a, b, c, d))) - } - Some(IpAddressFamily::Ipv6) => None, - }, - SocketAddr::V6(addr) => match family { - None | Some(IpAddressFamily::Ipv6) => { - let [a, b, c, d, e, f, g, h] = addr.ip().segments(); - Some(IpAddress::Ipv6((a, b, c, d, e, f, g, h))) - } - Some(IpAddressFamily::Ipv4) => None, - }, - } - }) - .collect()) - }); + let task = spawn_blocking(move || blocking_resolve(&host)); let resource = self.table_mut().push(ResolveAddressStream::Waiting(task))?; Ok(resource) } @@ -126,3 +86,40 @@ impl Subscribe for ResolveAddressStream { } } } + +fn parse(name: &str) -> Result { + // `url::Host::parse` serves us two functions: + // 1. validate the input is a valid domain name or IP, + // 2. convert unicode domains to punycode. + match url::Host::parse(&name) { + Ok(host) => Ok(host), + + // `url::Host::parse` doesn't understand bare IPv6 addresses without [brackets] + Err(_) => { + if let Ok(addr) = Ipv6Addr::from_str(name) { + Ok(url::Host::Ipv6(addr)) + } else { + Err(ErrorCode::InvalidArgument.into()) + } + } + } +} + +fn blocking_resolve(host: &url::Host) -> Result, SocketError> { + match host { + url::Host::Ipv4(v4addr) => Ok(vec![IpAddress::Ipv4(from_ipv4_addr(*v4addr))]), + url::Host::Ipv6(v6addr) => Ok(vec![IpAddress::Ipv6(from_ipv6_addr(*v6addr))]), + url::Host::Domain(domain) => { + // For now use the standard library to perform actual resolution through + // the usage of the `ToSocketAddrs` trait. This is only + // resolving names, not ports, so force the port to be 0. + let addresses = (domain.as_str(), 0) + .to_socket_addrs() + .map_err(|_| ErrorCode::NameUnresolvable)? // If/when we use `getaddrinfo` directly, map the error properly. + .map(|addr| util::to_canonical(&addr.ip()).into()) + .collect(); + + Ok(addresses) + } + } +} diff --git a/crates/wasi/src/preview2/network.rs b/crates/wasi/src/preview2/network.rs index b3b7537b98b3..65c380bd153c 100644 --- a/crates/wasi/src/preview2/network.rs +++ b/crates/wasi/src/preview2/network.rs @@ -1,3 +1,4 @@ +use crate::preview2::bindings::sockets::network::{Ipv4Address, Ipv6Address}; use crate::preview2::bindings::wasi::sockets::network::ErrorCode; use crate::preview2::{TableError, TrappableError}; use cap_std::net::Pool; @@ -34,3 +35,23 @@ pub enum SocketAddressFamily { Ipv4, Ipv6 { v6only: bool }, } + +pub(crate) fn to_ipv4_addr(addr: Ipv4Address) -> std::net::Ipv4Addr { + let (x0, x1, x2, x3) = addr; + std::net::Ipv4Addr::new(x0, x1, x2, x3) +} + +pub(crate) fn from_ipv4_addr(addr: std::net::Ipv4Addr) -> Ipv4Address { + let [x0, x1, x2, x3] = addr.octets(); + (x0, x1, x2, x3) +} + +pub(crate) fn to_ipv6_addr(addr: Ipv6Address) -> std::net::Ipv6Addr { + let (x0, x1, x2, x3, x4, x5, x6, x7) = addr; + std::net::Ipv6Addr::new(x0, x1, x2, x3, x4, x5, x6, x7) +} + +pub(crate) fn from_ipv6_addr(addr: std::net::Ipv6Addr) -> Ipv6Address { + let [x0, x1, x2, x3, x4, x5, x6, x7] = addr.segments(); + (x0, x1, x2, x3, x4, x5, x6, x7) +} diff --git a/crates/wasi/wit/deps/sockets/ip-name-lookup.wit b/crates/wasi/wit/deps/sockets/ip-name-lookup.wit index 5ffe2a217c61..270e70258d89 100644 --- a/crates/wasi/wit/deps/sockets/ip-name-lookup.wit +++ b/crates/wasi/wit/deps/sockets/ip-name-lookup.wit @@ -1,40 +1,30 @@ interface ip-name-lookup { use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; - use network.{network, error-code, ip-address, ip-address-family}; + use network.{network, error-code, ip-address}; /// Resolve an internet host name to a list of IP addresses. /// - /// See the wasi-socket proposal README.md for a comparison with getaddrinfo. - /// - /// # Parameters - /// - `name`: The name to look up. IP addresses are not allowed. Unicode domain names are automatically converted - /// to ASCII using IDNA encoding. - /// - `address-family`: If provided, limit the results to addresses of this specific address family. - /// - `include-unavailable`: When set to true, this function will also return addresses of which the runtime - /// thinks (or knows) can't be connected to at the moment. For example, this will return IPv6 addresses on - /// systems without an active IPv6 interface. Notes: - /// - Even when no public IPv6 interfaces are present or active, names like "localhost" can still resolve to an IPv6 address. - /// - Whatever is "available" or "unavailable" is volatile and can change everytime a network cable is unplugged. + /// Unicode domain names are automatically converted to ASCII using IDNA encoding. + /// If the input is an IP address string, the address is parsed and returned + /// as-is without making any external requests. /// - /// This function never blocks. It either immediately fails or immediately returns successfully with a `resolve-address-stream` - /// that can be used to (asynchronously) fetch the results. + /// See the wasi-socket proposal README.md for a comparison with getaddrinfo. /// - /// At the moment, the stream never completes successfully with 0 items. Ie. the first call - /// to `resolve-next-address` never returns `ok(none)`. This may change in the future. + /// This function never blocks. It either immediately fails or immediately + /// returns successfully with a `resolve-address-stream` that can be used + /// to (asynchronously) fetch the results. /// /// # Typical errors - /// - `invalid-argument`: `name` is a syntactically invalid domain name. - /// - `invalid-argument`: `name` is an IP address. - /// - `not-supported`: The specified `address-family` is not supported. (EAI_FAMILY) + /// - `invalid-argument`: `name` is a syntactically invalid domain name or IP address. /// /// # References: /// - /// - /// - /// - - resolve-addresses: func(network: borrow, name: string, address-family: option, include-unavailable: bool) -> result; + resolve-addresses: func(network: borrow, name: string) -> result; resource resolve-address-stream { /// Returns the next address from the resolver.