-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WASI: Fine-grained network policies #7681
Comments
This all sounds like a great idea to me, thanks for writing this up @badeend! One comment I would have is that I think it would be best to implement this in a way that's not baked-in to I think this is pretty reasonable syntax to add to the CLI though and I definitely agree it'd be best to have more than just an "everything on" switch! |
This all looks really great! Seems like we're headed in the right direction. A few small thoughts and questions:
|
I agree. Continuing on #7694 , I have the following in mind:
This example is for TCP Bind only, but you can imagine the same for other operations. pub trait WasiTcpView: WasiView {
/// Custom state maintained per socket instance.
type Socket;
/// Create new custom socket state. Guaranteed to be called at most once per WASI socket resource.
fn new(&mut self) -> Self::Socket;
/// Called at the moment the actual syscall would have been called. So _after_ the WASI-specific state & input validations.
/// The `bind` parameter represents the actual sycall implementation that can be executed (or ignored) by the interceptor.
/// With this general design, the interceptor:
/// - has full control of what to execute before & after the syscall,
/// - may conditionally execute or reject the syscall depending on the parameters,
/// - can keep track of additional state per socket, that would otherwise not be maintained by wasmtime-wasi itself
/// - (not in this example, but:) maybe manipulate parameters and return values. For example:
/// rewrite the address parameter from port 80 ("inside wasm") to 8080 ("outside wasm")
fn intercept_bind(&mut self, socket: &mut Self::Socket, bind: TcpBind);
}
#[must_use="For clarity, explicitly drop it using one of the consuming methods."]
pub struct TcpBind<'a> {
// ...
}
impl TcpBind<'_> {
/// The address passed in by the user.
pub fn requested_address(&self) -> &SocketAddr {}
/// Consume self and perform the bind on the requested address. Returns the actually bound address.
pub fn execute(mut self) -> std::io::Result<SocketAddr> {
// Call rustix::bind etc.
}
/// Consume self and abort the bind with an error code.
pub fn fail(self, error: TcpBindError) {
}
// The typical consuming methods will be `execute` and `fail`.
// But `UdpSend` could have an `skip` method as well, that pretends to have sent the message, but actually it was dropped.
}
/// Subset of all wasi-sockets errors that are appropriate for `bind` to return.
#[non_exhaustive]
pub enum TcpBindError {
AccessDenied,
AddressInUse,
AddressNotBindable,
}
// Example implementations
impl WasiTcpView for WasiCtx {
type Socket = ();
fn new(&mut self) {}
// Example #1: allow everything
fn intercept_bind(&mut self, socket: &mut Self::Socket, bind: TcpBind) {
bind.execute()
}
// Example #2: deny everything
fn intercept_bind(&mut self, socket: &mut Self::Socket, bind: TcpBind) {
bind.fail(TcpBindError::AccessDenied)
}
// Example #3: arbitrary logic
fn intercept_bind(&mut self, socket: &mut Self::Socket, bind: TcpBind) {
if bind.requested_address().ip().is_loopback() {
bind.execute()
} else {
bind.fail(TcpBindError::AccessDenied)
}
}
}
Ah, yes. The classical "user experience" vs. "security" tradeoff. From an end-user's perspective I don't care how a specific Wasm module decided to implement their network requests. I.e. I want to say "you're allowed to fetch https://example.com/hi.txt" without knowing having to care whether that will be done using TCP/UDP directly or using a wasi-http client. To properly guard this off would effectively be a man-in-the-middle attack. That being said, none of this is really essential for to this issue, so I'm happy to postpone all "implying" to a future iteration.
I agree. At least for TCP and UDP.
A mix of "TBD" and "I don't care" 😛 |
Coming back on my previous comment: Even though the initial refactor may be more work, I think it's more straightforward to forget about "interceptors" completely and put the entire socket implementation behind traits whose implementations can be swapped out. My current train of thought is to create a "vanilla Rust" trait that loosely follows the wasi-sockets interface. With "vanilla" I mean:
#[async_trait]
pub trait TcpSocket {
type InputStream: AsyncRead;
type OutputStream: AsyncWrite;
type AcceptStream: Stream<Item = io::Result<Self>>;
fn new(addr: SocketAddr) -> io::Result<Self>;
async fn bind(&mut self, addr: &SocketAddr) -> io::Result<()>;
async fn connect(&mut self, addr: &SocketAddr) -> io::Result<(Self::InputStream, Self::OutputStream)>;
async fn listen(&mut self) -> io::Result<Self::AcceptStream>;
fn local_address(&self) -> io::Result<SocketAddr>;
fn remote_address(&self) -> io::Result<SocketAddr>;
fn keep_alive_enabled(&self) -> io::Result<bool>;
fn set_keep_alive_enabled(&self, value: bool) -> io::Result<()>;
// ...
} Besides defining that trait, pub struct RestrictedTcpSocket {
inner: SystemTcpSocket, // The default, native implementation
// ...
}
#[async_trait]
impl TcpSocket for RestrictedTcpSocket {
async fn bind(&mut self, addr: &SocketAddr) -> io::Result<()> {
if addr.ip().is_loopback() {
inner.bind(addr)
} else {
return Err(Error::new(ErrorKind::PermissionDenied, "Nope."));
}
}
// ...
} With that in place,
pub trait WasiTcpView {
type Socket: TcpSocket;
} |
@badeend hi, excuse me , do you have plan to achieve this fine-grained network plolices? Is there a specific timestone? |
This is being worked on in #7705. Hoping that it shouldn't be too much longer before this is ready to merge. |
@rylev thanks, but I did not found a "grant" struct as badeend described use pseudo code。 would this be achieve in 7705 in the future? if there is a plan?for now, define a "socket_addr_check" func it's complexing。 |
#7705 contains the preparational work discussed further in this issue. We haven't started on the design in the initial comment |
#7662 is a good first step towards letting users of the wasmtime library customize network behavior. After that change, library users have two options: either use
inherit_network
which grants access to everything, or usesocket_addr_check
which allows the user to define arbitrarily complex rules. The upside of that last one is also its downside: the user must define everything themself. On the command-line, the available choices are quite limited: everything or nothing at all. Guess which one users will pick :PI would like to add a middle ground option for both library & CLI users. This option should provide a default "good enough" option for the majority of users, while still allowing to fall back to fully customized control. My objective is to be able to declare network policies based on:
I tried to capture the gist of it in pseudo code:
Domain-based policy strategy
THe IP and Port-based patterns above should speak for themselves. The domain name policies might need to explanation: the idea is to hook into
ip-name-lookup::resolve-addresses
to keep track of which IP address belongs to which domain names at runtime:ip-name-lookup::resolve-addresses
:Any
or matchingDomain
host pattern.Domain
-based grants: register the resolved addresses inDynamicPolicy::resolved_names
(see below)tcp-socket::bind
: validate that any TcpOutbound or TcpInbound grant exists with a matchinglocal_interface
andlocal_port
tcp-socket::connect
, validate that any TcpOutbound grant matches thelocal_interface
&local_port
. Also match theremote_host
&remote_port
:resolved_names
for that IP.UDP directionality
UDP has no traditional notion of "client" and "server". However, in practice many UDP applications do fit that model. I've modeled the grant types above based on what stateful firewall do. In order to know the directionality (inbound vs outbound) we need to keep track of "who talked first".
CLI syntax
Inbound syntax:
Outbound syntax:
Let me know what you think.
The text was updated successfully, but these errors were encountered: