Conversation
ea6fff4 to
44e49c9
Compare
There was a problem hiding this comment.
@tangyatsu @nklaassen I think we should make a decision on whether VNet is going to be available in plain tsh or not.
I'd argue that the teleport Linux package is used mostly to install agents and clusters. From this perspective, isn't setting up systemd files needed just for VNet both unnecessary and potentially a new attack vector? My whole argument hinges on this so let me know if this isn't correct. But I feel like setting up a new daemon that can affect how DNS works on a machine that's never supposed to run Teleport clients in the first place is both unnecessary and insecure.
We could tell tsh Linux users that if they want to use VNet, they need to install the teleport-connect package too. I suspect it might be fine for desktop users, but it would be potentially frowned upon when someone wants to use VNet without an available desktop env – would that be a valid complaint? The teleport-connect package depends on libgtk-3-0, libnotify4, libnss3, libxss1, libxtst6, xdg-utils, libatspi2.0-0, libuuid1, libsecret-1-0.
An alternative would be to embed those systemd files within the tsh binary and make tsh vnet set them up if they don't exist on the system. Or we could say that VNet on Linux is just not available without using Connect too.
There was a problem hiding this comment.
If you run tsh vnet with sudo, it checks for the presence of a VNet D-Bus daemon. If the daemon is not available, it spawns the admin process directly, so there is a way to run VNet without those systemd files, but the system still needs to have systemd and D-Bus available.
I agree that bundling systemd units into a package whose primary purpose is installing agents and clusters does look unnecessary, but I would prefer to leave the user the option to run VNet without sudo or requiring teleport connect to be installed.
Maybe we could add docs stating something like: “To run VNet as a systemd service, install these files.” then link to the directory in the teleport repo that contains the unit files.
tsh could also return an error that links to that documentation (links tend to break over time though and there are two of them)
There was a problem hiding this comment.
Maybe we could add docs stating something like: “To run VNet as a systemd service, install these files.” then link to the directory in the teleport repo that contains the unit files.
I think something like this would be a good solution for now. We don't really advertise tsh vnet anywhere other than in a small section of the docs.
The way I look at it, we wouldn't even need to add anything to the docs. We could make it so that when tsh vnet on Linux detects that the daemon is not present, it outputs a relevant message to stdout. Something like:
No D-Bus daemon detected. To run VNet without a password prompt, install these systemd files:
and what would follow is a link to https://github.com/gravitational/teleport/tree/v<version>/examples/systemd/vnet/. That dir would just need a readme.md added to explain how to install those files, as it might not be clear to everyone.
A more user-friendly approach would be to make tsh vnet install those files by itself as I mentioned in my comment. We could use the embed package to ship those files within the tsh binary itself.
However, as I mentioned, we don't really advertise tsh vnet that much, so I think that running with sudo and leaving a link to the systemd files would be perfectly fine for now.
ravicious
left a comment
There was a problem hiding this comment.
Sorry, forgot to submit this comment yesterday.
09e8a5c to
ed5d2aa
Compare
ed5d2aa to
d39c94e
Compare
d39c94e to
4e5b0ab
Compare
What
This PR adds files that are required for Linux VNet from the previous PR #63664. These files will be shipped in the teleport-connect package.
changelog: Add VNet support for Teleport Connect on Linux
Installed files:
Manual Test Plan
Test Environment
any Linux distro with systemd, D-Bus and polkit
Test Cases
teleport-connectpackageteleport-connect, verify VNet files are removed.