Support socket address family VSOCK #409
Conversation
|
Can you please add your name to CONTRIBUTORS.txt? Thanks! I'm not familiar with VSOCK sockets, gonna need to wait for @bertjwregeer to review this. Also note the failing ci/cd jobs that need to be fixed. |
Sure thing.
From waitress' point of view, they should be identical to UNIX sockets. The vsock address family is available to Linux VMs and the host machine when using the KVM virtualisation module. It allows the host machine and any VMs running on it to communicate without using the network. So its only available locally on Linux.
Fixed. |
|
Fixed the new failure by limiting this feature to only CPython on Linux as it is unavailable elsewhere. |
|
Adding a bit of context here... there's an upcoming paradigm called secure enclaves, where cloud providers set up specially isolated and secured VMs with no writable disks, no network access, and signed code execution. These special VMs are suitable for processing very sensitive data like medical records, credit cards or anything else that too sensitive to want to process on a normal machine. When running these VMs have no TCP sockets in them. The only way to communicate with them from the host machine is over a VSOCK socket. Works pretty much the same as any other socket, but is a different address family (integer, not IP address) and port scheme. We want to run a Python server inside this secure VM, and asking waitress to listen on a VSOCK is by far the cleanest solution we've seen so far. We're dogfooding this PR internally. |
| raise ValueError("Only Internet or UNIX stream sockets may be used.") | ||
| if sock.type != socket.SOCK_STREAM or sock.family not in supported_families: | ||
| raise ValueError( | ||
| "Only Internet, UNIX, or VSOCK stream sockets may be used." |
There was a problem hiding this comment.
Why does this restriction exist btw? Is there an issue listening to multiple types of sockets simultaneously?
There was a problem hiding this comment.
I think it's a fair question - I don't know what the history is here but it doesn't feel like anything would fundamentally break by using multiple different socket types. Things that come to mind are any WSGI environ settings that should be dynamic that aren't right now based on the source connection.
There was a problem hiding this comment.
Each new socket type added later on will increase the number of combined checks needed. I could try removing this restriction and seeing if anything breaks.
Removing this restriction would also mean better systemd socket activation support. Systemd can pass multiple different types of sockets to apps quite easily and waitress should support that too.
There was a problem hiding this comment.
The combinatorial explosion can be prevented by just a better loop and more complicated error message generation but again that’s assuming there’s a technical reason for keeping the checks.
There was a problem hiding this comment.
Yes. Technically there is no basis for this check. The only question is if any waitress code assumes so or depends on this.
There was a problem hiding this comment.
Part of it is that there is a different expectation with being able to receive from a UNIX socket (local, defined by file system permissions) and a TCP/IP socket.
There is no remote peer in the case of a UNIX socket that has a port number, certain values can't be automatically provided instead they are guessed at (such as the Host header if none is provided by the remote client over a UNIX socket).
Keeping the socket types separate may not be strictly required, it's not something that has been tested or validated and adding the additional testing may not be worth it. If you want to listen on both a UNIX socket and TCP/IP then that can be two different instances of waitress (or a single front-end proxy that does support that).
| except Exception: | ||
| raise ValueError("Invalid host/port specified.") | ||
| except Exception as exc: | ||
| raise ValueError("Invalid host/port specified.") from exc |
There was a problem hiding this comment.
fwiw I do not believe that the from exc is necessary in these cases you have modified - it is an explicit form of what already happens when raising a new exception from within an except block.
There was a problem hiding this comment.
I setup a new linter (not sure which one) and it suggested I try this.
Add support for VSOCK stream sockets.
Check for and use vsock sockets only on Linux. Added tests and re-formatted some files too.
|
I've merged |
|
@digitalresistor I can fix those tests if you're still interested. |
Add support for VSOCK stream sockets.
Fixes: #408