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

Should I use sudo or not? #83

Open
43qcc2cn opened this issue May 16, 2021 · 10 comments
Open

Should I use sudo or not? #83

43qcc2cn opened this issue May 16, 2021 · 10 comments

Comments

@43qcc2cn
Copy link

43qcc2cn commented May 16, 2021

Hi.

I've only just discovered this tool so apologies if this is a basic question.

I couldn't find anyone else asking, so it's probably just me.

In the User Guide it states:
"vopono will call sudo if required, it is recommended to run as the current user and let vopono call sudo so that the configuration directories are correctly inferred and the final command is not run as root."

  1. The first thing I do is sync:
~/bin/vopono sync
 2021-05-16T15:11:56.529Z INFO  vopono::util > Calling sudo for elevated privileges, current user will be used as default user
Which VPN providers do you wish to synchronise? Press Space to select and Enter to continue: NordVPN
 2021-05-16T15:12:02.910Z INFO  vopono::sync > Starting OpenVPN configuration...
Please choose the set of OpenVPN configuration files you wish to install: Default (TCP): These files connect over TCP.
NordVPN username: {User Name}
Password: [hidden]

after this:

ls ~/.config/vopono/
ls: cannot access '/home/user/.config/vopono/': No such file or directory

sudo ls /root/.config/vopono/
nordvpn
  1. I list the servers:
~/bin/vopono servers nordvpn
Error: Config files for NordVPN OpenVpn do not exist, run vopono sync

sudo ~/bin/vopono servers nordvpn
provider	protocol	config_file
NordVPN	openvpn	albania-al18.ovpn
etc.
  1. I try to run firefox:
~/bin/vopono exec --provider nordvpn --server uk firefox

The process hangs in the same way as described in #59. As far as I can see it's not able to create the firewall rules.

sudo ~/bin/vopono exec --provider nordvpn --server uk firefox

Everything works as expected.

I believe I'm using the latest release:

~/bin/vopono --version
vopono 0.7.1

and I'm using Fedora 34, which has the following default sudoers config entries:

Defaults    always_set_home
# Defaults   env_keep += "HOME"

So, should I always run the application via sudo, do I need to change by default sudo behaviour config, or am I simply doing something wrong?

Many thanks in advance.

@jamesmcm
Copy link
Owner

It should be fine without sudo, the issue is the Fedora config meaning that it is writing to the root dir rather than the user that it calls sudo from.

The code is:

    let _res: () = if let Some(base_dirs) = BaseDirs::new() {
        pathbuf.push(base_dirs.config_dir());
        Ok(())
    } else if let Ok(user) = std::env::var("SUDO_USER") {
        // TODO: DRY
        let confpath = format!("/home/{}/.config", user);
        let path = Path::new(&confpath);
        if path.exists() {
            pathbuf.push(path);
            Ok(())
        } else {
            Err(anyhow!("Could not find valid config directory!"))
        }

Could you please check if Fedora is setting the XDG_CONFIG_DIR for root (i.e. when call from sudo)? Or not setting the SUDO_USER env var when using sudo?

One of these must differ from Arch Linux, causing it to write to the root home dir directly.

@43qcc2cn
Copy link
Author

Thanks for the quick response.

XDG_CONFIG_DIR is not set but SUDO_USER is:

sudo env | egrep "SUDO_USER|XDG_CONFIG_DIR"
SUDO_USER=user

@jamesmcm
Copy link
Owner

It might be XDG_CONFIG_HOME too sorry.

But yeah if that isn't set then it should be the same as Arch 🤔

@43qcc2cn
Copy link
Author

None of the XDG env vars are set when using sudo:

sudo env | egrep "SUDO|XDG"
SUDO_COMMAND=/usr/bin/env
SUDO_USER=user
SUDO_UID=1000
SUDO_GID=1000

Only the following are set for a non root user under Gnome:

env | egrep "SUDO|XDG"
XDG_MENU_PREFIX=gnome-
XDG_SESSION_DESKTOP=gnome
XDG_SESSION_TYPE=wayland
XDG_CURRENT_DESKTOP=GNOME
XDG_SESSION_CLASS=user
XDG_RUNTIME_DIR=/run/user/1000
XDG_DATA_DIRS=/home/user/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/usr/local/share/:/usr/share/

I don't know Rust (or any programming languages for that matter) but, is the first test in your code checking that it can get the sudo user's standard directories? If yes then, won't this pass and base the config dir on the sudo user's home directory, which in my case is:

sudo env | grep HOME
HOME=/root

when it should base the config dir on the calling user's home directory:

sudo getent passwd $(sudo env | grep SUDO_USER | awk -F= '{print $2}') | cut -d: -f6
/home/user

I'm not sure if that code is running as the calling user or the sudo user.

@jamesmcm
Copy link
Owner

Yeah, there it looks fine. It should be getting the directory from the SUDO_USER variable.

Could you please try building from the newissues branch and running with debug output:

git clone git@github.com:jamesmcm/vopono.git
cd vopono
git checkout newissues
cargo build
cd target/debug/
./vopono -v sync

And copy the output here? As it seems it might be some issue with the XDG base_dirs crate maybe.

@43qcc2cn
Copy link
Author

As requested:

 ./vopono -v sync
 2021-05-19T11:03:26.093Z INFO  vopono::util > Calling sudo for elevated privileges, current user will be used as default user
 2021-05-19T11:03:26.094Z DEBUG vopono::util > Args: ["./vopono", "-v", "sync"]
Which VPN providers do you wish to synchronise? Press Space to select and Enter to continue: NordVPN
 2021-05-19T11:03:33.403Z INFO  vopono::sync > Starting OpenVPN configuration...
 2021-05-19T11:03:33.403Z DEBUG vopono::util > Using config dir from XDG dirs: /root/.config
Please choose the set of OpenVPN configuration files you wish to install: Default (TCP): These files connect over TCP.
 2021-05-19T11:03:37.268Z DEBUG reqwest::connect > starting new connection: https://downloads.nordcdn.com/
 2021-05-19T11:03:37.298Z DEBUG rustls::client::hs > No cached session for DNSNameRef("downloads.nordcdn.com")
 2021-05-19T11:03:37.298Z DEBUG rustls::client::hs > Not resuming any session
 2021-05-19T11:03:37.310Z DEBUG rustls::client::hs > Using ciphersuite TLS13_CHACHA20_POLY1305_SHA256
 2021-05-19T11:03:37.311Z DEBUG rustls::client::tls13 > Not resuming
 2021-05-19T11:03:37.311Z DEBUG rustls::client::tls13 > TLS1.3 encrypted extensions: [ServerNameAck, Protocols([PayloadU8([104, 50])])]
 2021-05-19T11:03:37.311Z DEBUG rustls::client::hs    > ALPN protocol is Some(b"h2")
 2021-05-19T11:03:37.321Z DEBUG rustls::client::tls13 > Ticket saved
 2021-05-19T11:03:37.321Z DEBUG rustls::client::tls13 > Ticket saved
 2021-05-19T11:03:37.343Z DEBUG reqwest::async_impl::client > response '200 OK' for https://downloads.nordcdn.com/configs/archives/servers/ovpn.zip
 2021-05-19T11:03:38.492Z DEBUG vopono::providers::nordvpn::openvpn > Reading file: ovpn_tcp/al18.nordvpn.com.tcp.ovpn
 etc.
NordVPN username: user
Password: [hidden]
 2021-05-19T11:05:09.643Z DEBUG vopono::util                        > Using config dir from XDG dirs: /root/.config
 2021-05-19T11:05:09.650Z DEBUG vopono::util                        > Using config dir from XDG dirs: /root/.config

Out of curiosity, should sudo even be used just to perform the sync?

@jamesmcm
Copy link
Owner

Thanks, these issues are fixed on the newissues branch - so that the root XDG settings never take precedence like that. I also removed the use of sudo just for the sync (I think it was there just to be sure that it can overwrite the permissions if some file belonged to root in the vopono config dir, but that is an edge case really).

0013d01

I want to finish fixing the openfortivpn and pppd issues before making a new release though.

@43qcc2cn
Copy link
Author

Thanks for the update. I pulled the updates and tried to recompile but it fails with:

error[E0425]: cannot find value `errbuffer` in this scope
  --> src/openfortivpn.rs:68:54
   |
68 |                 get_remote_peer(std::str::from_utf8(&errbuffer).expect("Non UTF8 stderr"))
   |                                                      ^^^^^^^^^ help: a local variable with a similar name exists: `buffer`

warning: unused import: `Write`
 --> src/openfortivpn.rs:7:21
  |
7 | use std::io::{Read, Write};
  |                     ^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0425`.
error: could not compile `vopono`

@jamesmcm
Copy link
Owner

Oops, it's fixed now.

And the tl;dr is you should never need to use sudo directly. Note there is still an issue around running as root directly (e.g. in system-level systemd unit files) due to #84

@43qcc2cn
Copy link
Author

I get the following warnings when compiling:

warning: unused import: `Write`
 --> src/openfortivpn.rs:7:21
  |
7 | use std::io::{Read, Write};
  |                     ^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused variable: `stderr`
  --> src/openfortivpn.rs:52:13
   |
52 |         let mut stderr = handle.stderr.take().unwrap();
   |             ^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_stderr`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: variable does not need to be mutable
  --> src/openfortivpn.rs:52:13
   |
52 |         let mut stderr = handle.stderr.take().unwrap();
   |             ----^^^^^^
   |             |
   |             help: remove this `mut`
   |
   = note: `#[warn(unused_mut)]` on by default

warning: function is never used: `get_peer_route`
   --> src/openfortivpn.rs:115:8
    |
115 | pub fn get_peer_route() -> anyhow::Result<Ipv4Addr> {
    |        ^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

warning: 4 warnings emitted

but it does compile, and I can confirm that the config is now written to the non root user's home directory.

Many thanks.

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

No branches or pull requests

2 participants