Wasmtime GitHub notifications bot (Sep 28 2023 at 22:53): Wasmtime GitHub notifications bot (Sep 28 2023 at 22:53):alexcrichton commented on issue #7109:
cc @sunfishcode and @badeend
This is based on https://github.com/bytecodealliance/wasmtime/pull/7029 so only the last commits here are relevant.
I'm particularly keen to get y'alls input and thoughts on the implementation here. Was the intention to be able to use
getaddrinfo? Or was the intention that implementations must avoidgetaddrinfo? For example the documentation of theip-name-lookupfunctions indicates that addresses shouldn't be parsed (which I assume means that resolving"127.0.0.1"should fail), but I couldn't find the option to specify togetaddrinfothat this must fail. I was basically wondering if y'all had an implementation for this in mind ahead of time, and if so I'd be happy to switch to that.
Wasmtime GitHub notifications bot (Sep 28 2023 at 22:54):alexcrichton commented on issue #7109:
Also I'm curious for y'all's thoughts on how to make this capability-based. For example right now I've added a global flag for "allow any name lookups at all" but that's quite coarse. Did y'all have thoughts already about how this might look like to configure from a CLI and how embedders might use it?
Wasmtime GitHub notifications bot (Sep 29 2023 at 20:12):alexcrichton edited a comment on issue #7109:
cc @sunfishcode and @badeend
This is based on https://github.com/bytecodealliance/wasmtime/pull/7029 so only the last commits here are relevant.I'm particularly keen to get y'alls input and thoughts on the implementation here. Was the intention to be able to use
getaddrinfo? Or was the intention that implementations must avoidgetaddrinfo? For example the documentation of theip-name-lookupfunctions indicates that addresses shouldn't be parsed (which I assume means that resolving"127.0.0.1"should fail), but I couldn't find the option to specify togetaddrinfothat this must fail. I was basically wondering if y'all had an implementation for this in mind ahead of time, and if so I'd be happy to switch to that.
Wasmtime GitHub notifications bot (Oct 01 2023 at 09:18):badeend commented on issue #7109:
Hi!
I see the
nameparameter is directly passed on to.to_socket_addrs(). Which is understandable, but that doesn't quite reflect the proposal.was the intention that implementations must avoid getaddrinfo?
No, but to keep the WASI API surface area as small & simple as possible, it isn't a drop-in replacement either. From the Readme:
Why not getaddrinfo?
The proposed wasi-ip-name-lookup module focuses strictly on translating internet domain names to ip addresses and nothing else.
Like BSD sockets,
getaddrinfois very generic and multipurpose by design. The proposed WASI API is not. This eliminates many of the other "hats" getaddrinfo has (and potential security holes), like:
- Mapping service names to port numbers ("https"->443)
- Mapping service names/ports to socket types ("https"->SOCK_STREAM)
- Network interface name translation (%eth0->1)
- IP address deserialization ("127.0.0.1"->Ipv4Address(127, 0, 0, 1))
- IP address string canonicalization ("0:0:0:0:0:0:0:1"->"::1")
- Constants lookup forINADDR_ANY,INADDR_LOOPBACK,IN6ADDR_ANY_INITandIN6ADDR_LOOPBACK_INIT.Many of these functionalities can be shimmed in the libc implementation. Though some require future WASI additions. An example is network interface name translation. That requires a future
if_nametoindex-like syscall.
(...) I've added a global flag for "allow any name lookups at all" but that's quite coarse. Did y'all have thoughts already about how this might look like to configure from a CLI and how embedders might use it?
This issue might be relevant: https://github.com/WebAssembly/wasi-sockets/issues/32
Also, this document exists: https://github.com/WebAssembly/wasi-sockets/blob/main/GrantingAccess.md but keep in mind that it hasn't been updated since the first draft of the proposal.For the initial implementation, a simple boolean (allow/deny everything) seems fine to me. :+1:
Wasmtime GitHub notifications bot (Oct 01 2023 at 12:48):badeend commented on issue #7109:
I've added input validation here: https://github.com/badeend/wasmtime/commits/wasi-ip-name-lookup
Wasmtime GitHub notifications bot (Oct 02 2023 at 14:36):alexcrichton commented on issue #7109:
Ok thanks! I saw the note about getaddrinfo yeah but I wasn't sure whether it was intended that a particular flag was passed or whether parsing was going to be required. I think your branch works well though, and I'll integrate that here, thanks for it!
Also sounds good on access restrictions for now, I like the
GrantingAccess.mdlink and agree that it's probably best to start off with a simple boolean for now and it can be bikeshedded/expanded later.
Wasmtime GitHub notifications bot (Oct 02 2023 at 16:28):fitzgen commented on issue #7109:
Redirecting review to Pat since I'm not really familiar with WASI bits.
Last updated: Nov 22 2024 at 16:03 UTC