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

vpn_status: Add formatters for displaying IP addresses of VPNs #2227

Merged
merged 1 commit into from
Jan 12, 2024
Merged

vpn_status: Add formatters for displaying IP addresses of VPNs #2227

merged 1 commit into from
Jan 12, 2024

Conversation

jdholtz
Copy link
Contributor

@jdholtz jdholtz commented Jan 3, 2024

This commit adds the option to display IPv4 and IPv6 addresses of the VPNs in use (if using NetworkManager). I also added support for detecting OpenVPN VPNs because the connection type is tun (NetworkManager does not set the Vpn property to true for OpenVPN connections). I have found this change helpful to easily get my VPN IP when pen-testing.

There could be potential KeyError issues when attempting to retrieve the IP addresses, but I have not run into any problems with this.

In this change, I decided not to filter out when placeholders are None (likely for IPv6) and instead let the user decide to exclude them in the config (e.g. format = "{name} [\?if=ipv6 {ipv6}]").

@lasers
Copy link
Contributor

lasers commented Jan 3, 2024

In this change, I decided not to filter out when placeholders are None (likely for IPv6) and instead let the user decide to exclude them in the config (e.g. format = "{name} [?if=ipv6 {ipv6}]").

That's the way to go.

# shorter version
format = "{name} [{ipv6}]"

# ipv4 and/or ipv6
format = "{name}[ {ipv4}][ {ipv6}]"

# ipv4 and/or ipv6 using soft
format = "{name}[\?soft  ][{ipv4}][\?soft  ][{ipv6}]"

# etc

Not using py3status and from the looks of this history, @ultrabug can test this better than me.

My nitpicks are with existing i3status-related configs (such as pidfile, check_pid) possibly not part of loop now and if it makes sense to fix around some more to support both... such as pidfile now accepting a list of vpn pidfiles + check_pid = True to print VPN: yes, no instead of VPN: yes too.

And if this ought to still be a drop-in replacement? And maybe hardcoded color. Idk.

@jdholtz
Copy link
Contributor Author

jdholtz commented Jan 3, 2024

if it makes sense to fix around some more to support both

That does make sense. However, I think it would be best in a separate PR as adding support for multiple VPNs with check_pid doesn't connect with these changes.

@ultrabug ultrabug merged commit fe39761 into ultrabug:master Jan 12, 2024
5 checks passed
@ultrabug
Copy link
Owner

Thanks a lot @jdholtz ; I don't use this module and can't test it so I trust your user input!

@jdholtz jdholtz deleted the vpn-ips branch January 12, 2024 16:57
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.

3 participants