greshake / i3status-rust

Very resourcefriendly and feature-rich replacement for i3status, written in pure Rust
GNU General Public License v3.0
2.81k stars 472 forks source link

Add Wireguard support to vpn block #1981

Open ppx17 opened 6 months ago

ppx17 commented 6 months ago

Adds a new driver for the VPN block to support Wireguard connections.

I couldn't find any way to control Wireguard without using root permissions, so I hope this route with sudo is OK. The docs include a sudoers configuration that should be safe if your Wireguard configuration files are locked down properly, but please correct me if I'm mistaken here.

The driver supports the same toggle action on click as the other drivers, but I've reduced the country code and flag to empty strings, as they don't make a lot of sense in this context.

We can update the Connected enum to support a state without country information included, but I tried to change as few things as possible on the existing drivers.

MaxVerevkin commented 5 months ago

The driver supports the same toggle action on click as the other drivers, but I've reduced the country code and flag to empty strings, as they don't make a lot of sense in this context.

We can update the Connected enum to support a state without country information included, but I tried to change as few things as possible on the existing drivers.

I would prefer to make them optional, and on include them at all when using this driver.

ppx17 commented 5 months ago

The driver supports the same toggle action on click as the other drivers, but I've reduced the country code and flag to empty strings, as they don't make a lot of sense in this context. We can update the Connected enum to support a state without country information included, but I tried to change as few things as possible on the existing drivers.

I would prefer to make them optional, and on include them at all when using this driver.

Alright, I've implemented a separate 'ConnectedToCountry' state that includes the country and flag. It felt a little cleaner than still having them as Optional in drivers that don't care. Then again, there are now two separate states that both represent connected, not sure what the best approach would be? Happy to switch to a single connected state with optional attributes as well.

MaxVerevkin commented 5 months ago

I think having optional fields is slightly better. First of all, maybe in the future we will add more fields and having one "connected" state will make things easier. Next, it would eliminate duplicated match arms when we don't care about the country info. Finally, optionals work nicely with map! macro:

map! {
    "icon" => Value::icon(status.icon()),
    [if let Some(c) = country] "country" => Value::text(c.into()),
    [if let Some(f) = country_flag] "flag" => Value::text(f.into()),
}
ppx17 commented 5 months ago

Fair points, and neat trick with map! The PR is updated. :+1:

c3Ls1US commented 5 months ago

To clarify, you're invoking the sudo command internally to bring up a WireGuard interface? That signals a red flag to me and this would be the only block that would do it. What other options have you explored?

If I remember correctly, NetworkManager allows bringing up and tearing down network interfaces without sudo permissions so it's possible you can accomplish this with WireGuard interfaces too. I think that would be preferred.

ppx17 commented 5 months ago

Hmm, I don't really see a proper way to do this, even if there is a route through NetworkManager, then that is probably still not how most people have configured their current Wireguard connections, as it doesn't seem to support the default /etc/wireguard/wg0.conf settings.

Looking at the source of wg-quick, it seems to be a pretty hard requirement. Open to any other pointers to look at though!

c3Ls1US commented 5 months ago

wg-quick creates a WireGuard tunnel and sets up a firewall. Those two actions at least are inherently privileged operations that requires root. As far as I can tell, there's no way around that for non-root users and I believe NetworkManager is able to do this because its daemon is running as root. Same with other WireGuard clients such as Cloudflare's warp-cli.

In my opinion, accomplishing this feature set would be relatively straightforward through NetworkManager, safer and more ideal as it would integrated at the system level via libnm. Then for every other network manager, just don't allow managing the WG interface.

Besides sudo, there's also the polkit framework. I don't have an opinion on it, but perhaps that's a possibility?