-
Notifications
You must be signed in to change notification settings - Fork 113
Implement support for TSIv3 #410
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
base: main
Are you sure you want to change the base?
Conversation
ad10aec
to
cf52662
Compare
15c9e2b
to
95d5247
Compare
To support sockets beyond AF_INET, TSIv3 generalizes socket addresses behind an abstract sockaddr type, just as the kernel does internally. We need to follow suit here, and luckily we can rely on nix's SockaddrStorage to do most of the abstraction work for us. Signed-off-by: Sergio Lopez <[email protected]>
With TSIv3, we can trivially support other socket addresses with very little additional code. Let's take the opportunity to support both AF_INET6 and AF_UNIX. Signed-off-by: Sergio Lopez <[email protected]>
There are scenarios in which we want to have a vsock device for unix ipc to port mapping, but we don't want to use TSI. Make TSI enablement explicit. Signed-off-by: Sergio Lopez <[email protected]>
For TSI control messages that require an answer (connect, listen, accept and getname), reply with an error message even if the proxy is not present. Until now, under normal circumstances, we should never receive one of those requests without a proxy, but in the next commit we're going to start rejecting the creation of proxies under certain circumstances, so we need to do this first. Signed-off-by: Sergio Lopez <[email protected]>
AF_UNIX transparency is only supported when running in a containerized environment. We detect such case by checking we only have a single virtio-fs device, and that device is pointing to "/", which is the scenario when running under podman+crun. In libkrun 2.x, the whole network configuration, including the TSI features, should be exposed to the users explicitly. Signed-off-by: Sergio Lopez <[email protected]>
Since TcpProxy now supports more than TCP, let's refactor it as TsiStreamProxy. Signed-off-by: Sergio Lopez <[email protected]>
Since UdpProxy now supports more than UDP, let's refactor it as TsiDgramProxy. Signed-off-by: Sergio Lopez <[email protected]>
Since env_logger already indicates the module emitting the message, avoid putting it manually in the message string. Signed-off-by: Sergio Lopez <[email protected]>
TSIv3 is an ABI breaking change, so it's provided by a different major version of libkrunfw. Signed-off-by: Sergio Lopez <[email protected]>
@mtjhrc do you need more changes in this PR? |
Code LGTM! I am also wondering what happens to anonymous namespace unix domain sockets. Does the kernel side attempt to hijack them into TSI too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also wondering what happens to anonymous namespace unix domain sockets. Does the kernel side attempt to hijack them into TSI too?
I have managed to test this, yes they are hijacked too, forwarding them works correctly (works with podman --network host
).
Regarding the PR, there is an issue when resolving paths of unix domain sockets (even within the guest) see containers/libkrunfw#103
To support sockets beyond AF_INET, TSIv3 generalizes socketaddresses behind an abstract sockaddr type, just as the kernel does internally.
We need to follow suit here, and luckily we can rely on nix's SockaddrStorage to do most of the abstraction work for us.
Then, building on top of that, we implement support for AF_INET6 and AF_UNIX.
This is an ABI breaking change, so we also require the libkrunfw counterpart containers/libkrunfw#101