'postup'/'predown': Quotation marks are not handled correctly.

[d4h0]

I've found the cause of the following error (mentioned in another issue):

usage: sudo -h | -K | -k | -V
usage: sudo -v [-ABknS] [-g group] [-h host] [-p prompt] [-u user]
usage: sudo -l [-ABknS] [-g group] [-h host] [-p prompt] [-U user] [-u user] [command]
usage: sudo [-ABbEHknPS] [-C num] [-D directory] [-g group] [-h host] [-p prompt] [-R directory] [-T timeout] [-u user]
            [VAR=value] [-i|-s] [<command>]
usage: sudo -e [-ABknS] [-C num] [-D directory] [-g group] [-h host] [-p prompt] [-R directory] [-T timeout] [-u user] file ...

The following post-up callback reproduces it:

cargo run -- exec --custom /tmp/openvpn_test.ovpn --protocol openvpn --postup "echo '=> VPN is ready'" --firewall NfTables 'ls'

Using --postup 'echo "=> VPN is ready!"' fails as well.

So it looks like, that quotation marks are not handled properly.

Have you seen the xshell crate?

It can handle cases like that:

use xshell::{cmd, Shell};

fn main() {
    let sh = Shell::new().unwrap();

    // Even works with both kinds of quotation marks included:
    let cmd = r#"echo '=> "VPN is ready"'"#;

    cmd!(sh, "bash -c {cmd}").run().unwrap();
}

Output:

$ bash -c "echo \'=> \"VPN is ready\"\'"
=> "VPN is ready"

Printing out the executed command can be deactivated via .quiet().

I highly recommend this crate for any "shell scripting" via Rust. It has many useful features for that.

I guess, Vopono would have at least 15 to 30% less lines of code, if xshell was used.

This change is a bit to large for me to implement (I have no clue about most things that happen in Vopono), but I definitely would use xshell for something like Vopono.

Maybe using xshell for postup, predown and the executed command would be enough, at the beginning.

[d4h0]

It looks like regular shell commands are not supported in general (e.g. --postup 'echo READY' doesn't work). I think, that could be supported via executing callbacks via sh -c '$CALLBACK', which would work for scripts and shell commands.

[jamesmcm]

Thanks, the xshell suggestion is good, I'll try to take a look at it sometime - it'd also help to clearly highlight which parts still shell out.

As for the POSTUP/PREDOWN, they come from this issue - #71

There the aim was just to support bash scripts as files - i.e. the argument is the path to the bash script.

[d4h0]

Thanks, the xshell suggestion is good, I'll try to take a look at it sometime - it'd also help to clearly highlight which parts still shell out.

Yes, I think xshell is pretty amazing.

It also would make adding environ variables easier, and it's easy to make the variables usable as arguments of scripts/applications (which I described in my last PR):

use xshell::{cmd, Shell};

fn main() {
    let sh = Shell::new().unwrap();
    let cmd = r#"echo "=> 'VPN is ready. The NS IP is: $IP'""#;
    cmd!(sh, "bash -c {cmd}")
        .env("IP", "127.0. 0.1")
        .run()
        .unwrap();
}

Output:

$ bash -c "echo \"=> \'VPN is ready. The NS IP is: $IP\'\""
=> 'VPN is ready. The NS IP is: 127.0. 0.1'

And bash -c SCRIPT_OR_APPLICATION (as demonstrated above) could be used to support script arguments.

Another advantage is/might be that xshell doesn't change the actual environment (changes are "virtual", which is the reason, why the Shell instance has to be passed to the cmd! macro).

Interpolation of Option<x> (only adds something if it is Some) and Vec<x> (adds every x) is also supported, which probably would come in handy.

[d4h0]

Shell::env might also make it possible to not use sudo -E which seems to have possible security implications, and adds the requirement to combine NOPASSWD with SETENV.

However, I'm not sure if sudo -E is actually a problem for Vopono (besides having to add SETENV).

I've seen warnings to not use SETENV, because it can be used to attain root privileges.