Skip to content
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

feat(socket): berkeley socket API #312

Closed
wants to merge 4 commits into from

Conversation

3miliano
Copy link

This PR adds new functions for socket operations such as sock_listen, sock_bind, sock_connect, etc. It also includes a new addr_resolve function for performing DNS resolution. The PR introduces a new WASI handle called address_pool which is passed from the host into the relevant socket call. The address pool is at its core a collection of IP addresses and ports and associated rights. For example, you can say 127.0.0.1/8 with sock_connect right. Meaning if you pass that address pool to sock_connect call you may connect to any IP address within the 127.0.0.1/8 range. This mechanism allows the host to retain control of what the running WASM can do in terms of socket communication.

@sunfishcode FYI.

…lve, sock_open, sock_connect

feat(socket): added port to addr_resolve so DNS SRV records can also be searched

feat(socket): added new rights, removed local socket

feat(socket): added missing sock_bind, sock_listen, sock_accept

feat(socket): add address family

feat(socket): added new rights for bind and accept

feat(socket): removed sock_shutdown perm. might bring it back.

feat(socket): changed recv to not use iovec since there is pread for that

feat(socket): sock_send now uses regular buffer rather than iovecs

feat(socket): added sock_addr_local and sock_addr_remote

feat(socket): added sock_close, sock_set_reuse_addr and sock_set_reuse_port

feat(socket): added sock_set_recv_buf_size and sock_get_recv_buf_size

feat(socket): added sock_set_send_buf_size, sock_get_send_buf_size

feat(socket): added sock_send_to and sock_recv_from for UDP

feat(socket): changed sock_open and addr_resolve to receive a handle to an address pool

feat(socket): moved socket API into ephemeral
@devsnek
Copy link
Member

devsnek commented Aug 18, 2020

cursory glance suggests this needs s/sock/socket/, s/addr/address/, etc

@3miliano
Copy link
Author

@devsnek I just noticed that on ephemeral there is modules now, so I will remove the prefixes.

@devsnek
Copy link
Member

devsnek commented Aug 19, 2020

@3miliano also stuff like recv and buf. we aren't character limited by 1980s c compilers anymore :P

edit: to be clear this only applies to the type/function names, arg names can be whatever

@3miliano
Copy link
Author

@devsnek I tried to follow the naming convention that was already there, like for example recv was already in place before I changed it. The same applies to readdir which ain't read_directory, and I can cite other examples. I don't have a personal preference as to what we name things, I also understand that naming conventions are a personal preference so feel free to update the naming to be consistent with the rest of WASI.

@pchickey
Copy link
Contributor

We have done some work to sanitize the names of posix-derived types and functions, but the work is incomplete. While we're introducing new code, we should give it more verbose names where possible. I think we can also be flexible about updating existing names (recv-> receive is the most obvious) in ephemeral because no-one should be depending on that name yet. You can take that on as part of this PR if you would like, or we can do that as a separate PR into ephemeral.

@3miliano
Copy link
Author

I personally don't like inconsistency. I would rather for it to be on its own PR, and that PR has the specific intention of renaming the functions and not just in sockets API. We should make a global pass-thru all across the board.

Again, I am not against renaming anything. I just don't like mixed naming conventions. If we are serious about this, we should do a PR that renames everything across the board. Whoever starts that work, I'll be more than happy to help rename the socks function or even more there.

@pchickey
Copy link
Contributor

Sounds good!

@ueno
Copy link

ueno commented Aug 23, 2020

@3miliano sorry for the late response in bytecodealliance/wasmtime#70 (comment):

One thing to consider is that TLS doesn't exactly require system calls, and therefore you can of course compile a TLS library such as WoflSSL, OpenSSL, etc in WASM itself.

While in theory it works, the compiled code could be insecure and slow as it cannot utilize the constant-time, optimized implementation on the host side. For that purpose, there is a wasi-crypto proposal, but I'd say the situation is a bit far from practical applications such as TLS, because both toolchain and the upstream TLS libs need to be modified to use those syscalls. There is also a prior experiment to transparently enable TLS/DTLS through the POSIX networking APIs, but it turned out that some TLS specific configuration needs to be exposed as syscall (such as handshake()) to my understanding.

Therefore, I would still suggest leaving the socket opening part, that include TLS configuration, to the host side and making it opaque from the guest (as much as possible). IMO encrypted communication over sockets is becoming essential these days and has imminent need, so it would be great if the proposal natively supports it.

@3miliano
Copy link
Author

@ueno I don't think that I disagree with anything you stated. It is true that TLS has become truly ubiquitous, and it makes sense for WASI to provide some sort of support for it. I wasn't aware of the TLSSock effort. Looks interesting and might be something we could take a look at in the future. Especially since it doesn't really change any existing API call, but it adds new ones and new socket options.

The reason that I brought external libraries is one of the targets for WASI is still C and C++, those, in particular, will compile their own TLS library unless you are starting from scratch and you are only targeting WASM+WASI. Regardless of what we do in the realm of TLS support, I believe that is something we need to continue to support.

Bottom line is that we are in agreement here.

@npmccallum
Copy link
Contributor

@3miliano There are other problems with expanding socket for native TCP support.

  1. We definitely want some kind of capability for protocols. For example, TCP should be an actual capability and not just a constant. Wasm instances should be granted a capability for TCP (or not). In particular, we want to grant the ability to create a TLS socket but not a TCP socket.

  2. Emulating the existing name resolution schemes have problems with TLS. For example, you really want to connect(NAME) not connect(ADDR). This is because TLS clients need to validate the hostname against the subAltName of the certificate of the host they are connecting to. But if you resolve the name and then discard it and pass only the address to connect() then it is hard to implicitly validate.

  3. The TLS handshake has a pattern that is hard to coalesce. Specifically, on the listen() side, when a new connection comes in via accept(), is the TLS handshake complete or not? If you answer no, then sockets need an explicit handshake() function. This is nice because it means the server can call listen() on the main thread and then handshake() in a per-connection thread. However, if you don't make handshake() mandatory for all socket types, then many libraries will simply skip it - which means they can't support transparent TLS. OTOH, you can merge the handshake into accept(). But then the expectation that accept() is "fast" is violated. This can be mitigated with non-blocking accept(). But we need a clear strategy and expectation going forward.

@3miliano
Copy link
Author

3miliano commented Aug 27, 2020

@npmccallum Hey Nathaniel. Nice to meet you.

  1. Agreed. One way to do this is to add a handle right per protocol and introduce that into the sock_open call. We could also extend sock_type to include secure_stream for example for TLS. Did you have something else in mind?

  2. I understand where you are coming from with the SAN field of the certificate. I always saw certificate validation as something separate from the connection itself. Whenever that means, validating the CN, the SAN, the expiration, the authority, and even the revocation via OCSP. Having said that, due to SNI we would need access to the domain just to fetch the right certificate. Embedding address resolution inside the connect call is something that I am not fond of because of the same reasons you mention on your point 3. connect is no longer "fast". You could pass both, connect(ADDR, NAME) with name as optional just for SNI.

  3. Is transparent TLS even a goal? There are many aspects of TLS that require a higher degree of control, not sure how "transparent" we can make it. My idea was to have a separate handshake function, I don't like embedding it into accept because of the problems you just stated. Also, error reporting can be problematic if handshake is embedded in accept. If the handshake fails, do we just drop the connection and continue as normal? How does the program executing knows that a handshake failed?

Thanks for your input.

@npmccallum
Copy link
Contributor

npmccallum commented Aug 27, 2020

@3miliano Likewise, nice to meet you. For some background, I'm the founder of the Enarx project and I spearheaded the TLSSock experiment. Let me describe how we want to use these API. That may give better context.

Enarx aims to be a cloud platform which runs a wasm jit inside a TEE (such as SGX or SEV). Tenants submit their payloads as wasm + config and it is executed on their behalf in such a way that the host has little opportunity to view or modify the computation.

We basically want a system where the wasm payload can identify the "kinds" of connections it wants to establish (via either accept() or connect()). These connections are always guaranteed to be encrypted. Therefore, something close to transparent TLS is absolutely a goal for us; defined as we want applications written to the new WASI sockets API to be able to effortlessly switch between TCP and TLS - we are however willing to tolerate extension of the Berkley API.

Since the wasm payload defines the "kinds" of connections to be made at runtime, we can then map this to a policy that is delivered adjacent to the application. For example, "kind" foo is able to listen and accept a TLS connection using a specific x509 identity and requiring clients to authenticating using the provided set of certification authorities. Our wasm runtime will therefore do all authentication, authorization and auditing.

While the application can define different "kinds" of connections, it is up to the security policy to ensure the correct properties are enforced. This policy is centrally defined, executed and audited.

Let me return to the specific questions.

  1. I view the context for opening a socket to be much more than merely an address pool for resolution. I want the opening context to define a socket "kind" that policy can be attached to by the runtime.

  2. connect() has never been fast. There have always existed network conditions which can cause it to block for a long time (defined in minutes). Adding things like the TLS handshake and even OCSP are likely an acceptable change; especially if the socket is non-blocking. Regardless, we need to deliver the name associated with the address resolution to whatever function does the TLS handshake.

  3. Yes, transparent with the caveat of allowing extensions is a goal for us. In particular, we want to decouple security policy from application logic entirely. In our case, the application doesn't need to know whether or why the handshake failed. All it knows is that it wasn't a valid connection. The runtime will audit the appropriate information for later inspection.

Please note that while I'm opinionated about the problem of decoupling security AAA from application logic, I'm flexible as to what the actual API should look like. We will best succeed if we can balance things like application compatibility and so-forth.

@lygstate
Copy link
Contributor

sendmmsg and recvmmsg are needed for performance udp

@worldhopper
Copy link

@3miliano Why the need to introduce a concept of an "address pool" into WASI, and thereby try to replicate some pieces of functionality that the underlying host/network firewall typically provides? IP addresses are weak identities, and how do you account for the implications of this (e.g., multiple peers sharing the same IP address? NAT, etc.).

@sunfishcode
Copy link
Member

@worldhopper In this context, an IP address isn't about identifying a particular host or service. It's about identifying a request that the program can make of the outside world. How the outside world resolves that request is of course up to, well, the outside world. That's obviously not sufficient for all purposes, but it is the level at which Berkeley-style sockets APIs typically operate.

Underlying host/network firewall rules are often per-network, per-host, or per-host-process; with address pools, we can control access at the granularity of the wasm instance. And when two wasm instances are linked together, they'll be able to easily pass each other handles to delegate access to other instances, without external configuration or IPC protocols for access sharing.

The handle-based design is inspired by the Principle of Least Authority:

  • Wasm instances which do not have access to an address-pool handle cannot generate any network traffic.
  • Handles can represent individual network addresses, so a program can easily grant access to some addresses and not others to other parts of the program.
  • As WASI API virtualization mechanisms mature, they'll allow us to virtualize address pools, allowing wasm instances to present virutalized interfaces to other wasm instances, all within wasm, without requiring host-specific firewall/vlan/etc. configuration.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for the delay here, this is important functionality, and I'm excited for it to progress! I've now read through the API here, and while I expect we'll have more iterating to do, my sense is that this is close to being ready to merge, so we can continue to iterate on this in the tree. I just have a couple of questions below:

@@ -357,6 +375,8 @@
$symbolic_link
;;; The file descriptor or file refers to a FIFO.
$fifo
;;; Address pool
$address_pool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is in $filetype so that fdstat_get can work on address pool handles. That makes sense, but $file_type is closely tied to the filesystem APIs, and I'd like to explore options that would allow us to keep a clean separation between filesystem and other APIs.

One option might be to have fdstat_get return $unknown in this case. Would that work? Since address pools aren't in POSIX or Windows, existing code calling fdstat_get wouldn't expect to encounter them. And in theory, code that is expecting an address pool shouldn't need to do fdstat_get to find out if that's what it got. Does that make sense?

@@ -276,8 +276,26 @@
;;; If `rights::fd_read` is set, includes the right to invoke `poll_oneoff` to subscribe to `eventtype::fd_read`.
;;; If `rights::fd_write` is set, includes the right to invoke `poll_oneoff` to subscribe to `eventtype::fd_write`.
$poll_fd_readwrite
;;; The right to invoke `sock_shutdown`.
$sock_shutdown
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on why sock_shutdown is being removed? shutdown is in the traditional Berkeley sockets API.

Comment on lines +44 to +64
;;; Open a socket
;;;
;;; The first argument to this function is a handle to an
;;; address pool. The address pool determines what actions can
;;; be performed and at which addresses they can be performed to.
;;;
;;; The address pool cannot be re-assigned. You will need to close
;;; the socket and open a new one to use a different address pool.
;;;
;;; Note: This is similar to `socket` in POSIX using PF_INET
(@interface func (export "open")
;;; Address pool file descriptor
(param $pool $fd)
;;; Address family
(param $af $address_family)
;;; Socket type, either datagram or stream
(param $socktype $sock_type)
(result $error $errno)
;;; The opened socket
(result $fd $fd)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I suggest keeping the pool fd, but dropping the "address pool" concept and leaving it up to the host implementation how to decide which calls to open should succeed.

Also, what is the rationale behind not being able to call open multiple times on the same pool fd?

Suggested change
;;; Open a socket
;;;
;;; The first argument to this function is a handle to an
;;; address pool. The address pool determines what actions can
;;; be performed and at which addresses they can be performed to.
;;;
;;; The address pool cannot be re-assigned. You will need to close
;;; the socket and open a new one to use a different address pool.
;;;
;;; Note: This is similar to `socket` in POSIX using PF_INET
(@interface func (export "open")
;;; Address pool file descriptor
(param $pool $fd)
;;; Address family
(param $af $address_family)
;;; Socket type, either datagram or stream
(param $socktype $sock_type)
(result $error $errno)
;;; The opened socket
(result $fd $fd)
)
;;; Open a socket
;;;
;;; The first argument to this function is a handle to an capability descriptor.
;;; The capability descriptor determines what actions can
;;; be performed on the newly created socket.
;;;
;;; Note: This is similar to `socket` in POSIX using PF_INET
(@interface func (export "open")
;;; Capability descriptor
(param $capability $fd)
;;; Address family
(param $af $address_family)
;;; Socket type, either datagram or stream
(param $socktype $sock_type)
(result $error $errno)
;;; The opened socket
(result $fd $fd)
)

Comment on lines +170 to +177
;;; Accept a connection on a socket
;;; Note: This is similar to `accept`
(@interface func (export "accept")
;;; File descriptor of the socket to be bind
(param $fd $fd)
(result $error $errno)
(result $childfd $fd)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The POSIX accept() function also takes addr and addrlen parameters. Are these omitted on purpose?

Comment on lines +160 to +168
;;; Listen for connections on a socket
;;; Note: This is similar to `listen`
(@interface func (export "listen")
;;; File descriptor of the socket to be bind
(param $fd $fd)
;;; Maximum size of the queue for pending connections
(param $backlog $size)
(result $error $errno)
)
Copy link
Contributor

@badeend badeend Dec 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How useful are the server related functions (bind, listen, accept) for now?

Wasm is currently single-threaded so child connections can't be delegated to threads.

Comment on lines +66 to +72
;;; Close a socket (this is an alias for `fd_close`)
;;; Note: This is similar to `close` in POSIX.
(@interface func (export "close")
;;; Socket descriptor
(param $fd $fd)
(result $error $errno)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it desirable to have file descriptors be used interchangeably between different APIs? For example: should a socket fd be closeable by the close function of the filesystem API and vice-versa?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, it is desirable, because WASI is closely following POSIX in this respect. You can close(2) a socket, so the same should work in WASI.

In the future, when the abstract types mechanism in wasm is more developed, I expect we'll switch to making more type distinctions between different kinds of handles, and only provide the close(2) abstraction at the libc level.

@badeend badeend mentioned this pull request Dec 28, 2020
Base automatically changed from master to main January 19, 2021 23:08
@abbec
Copy link

abbec commented Feb 23, 2021

What is the status of this? To say I am eagerly awaiting it would be a massive understatement :)

@jacobfriedman
Copy link

Bump

@sascha1337
Copy link

wen?

@ShadowJonathan
Copy link

@3miliano what is this blocked on?

@jacobfriedman
Copy link

jacobfriedman commented Sep 23, 2021 via email

@0x1074
Copy link

0x1074 commented Sep 23, 2021

It's been more than half a year since I posted. I would have expected some sort of spec by now tbh

On Thu., Sep. 23, 2021, 7:44 a.m. Jonathan de Jong, < @.***> wrote: @3miliano https://github.com/3miliano what is this blocked on? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#312 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFKHXXV7UUTK5WZWQL5IZDUDMHIXANCNFSM4QDV573A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

this !

@tristangodfrey
Copy link

Is there any talk about this going on elsewhere? Something tells me this PR isn't where the discussion is happening, would love to know where the working group is at with this :)

@john-sharratt
Copy link

I am also super interested in where this is headed?

Does anyone know if its moved somewhere else, or if someone knows of a fork - I am looking at continuing the idea of https://webassembly.sh/ and its clear that the lack of sockets (even if its websocket limited sockets) is a serious limitation to WASI that needs to be fixed before it can become more mainstream.

Any thoughts on this?

Copy link

@dvc94ch dvc94ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it'll be a long time before libp2p runs in wasi.

(result $error $errno)
;;; The opened socket
(result $fd $fd)
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these sockets dual stack, or platform defined? maybe a set_only_v6 is required?

(result $error $errno)
)

;;; Accept a connection on a socket
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to have a join/leave multicast and maybe broadcast too.

@sascha1337
Copy link

sascha1337 commented Nov 6, 2021 via email

@ranile ranile mentioned this pull request Nov 6, 2021
@appcypher
Copy link

@3miliano What's up?

@ericmillin
Copy link

@IhostVlad @dvc94ch @3miliano

What is the status of this PR?

@badeend
Copy link
Contributor

badeend commented Jan 25, 2022

To those who are interested; I've created a draft proposal at: https://github.com/badeend/WASI-Networking

@jacobfriedman
Copy link

jacobfriedman commented Jan 25, 2022 via email

@Zheaoli
Copy link

Zheaoli commented Jul 9, 2022

Ping, Is there any update about this proposal?

@devsnek
Copy link
Member

devsnek commented Jul 9, 2022

some more progress happened here https://github.com/WebAssembly/wasi-sockets/

@linclark
Copy link
Member

Thanks for providing that redirect, @devsnek.

I'm going to go ahead and close this out since it has been inactive for quite a while. The work on sockets is now captured at https://github.com/WebAssembly/wasi-sockets/.

@linclark linclark closed this Jul 11, 2022
@StevenACoffman
Copy link

Agreed on closing it out. For those watching this issue, the linked proposal for WASI-sockets has this:

Phase 4 Advancement Criteria

At least two independent production implementations.
Implementations available for at least Windows, Linux & MacOS.
A testsuite that passes on the platforms and implementations mentioned above.

And I believe this counts as the first implementation:
bytecodealliance/wasm-micro-runtime#1036

@linclark
Copy link
Member

From what I understand, the WAMR implementation is not the same as the proposal in wasi-sockets. They would need to update the implementation (once the proposal is in the proper state for that) if WAMR is to be counted as one of the implementations used to advance the proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.