The vagrant lxc sudoers is insecure

[tailhook]

Due to how sudo works, the rule /bin/rm /usr/lib/lxc/templates/*, means the following command will work:

rm /usr/lib/lxc/templates/something /etc/passwd

This is what man says:

Wildcards in command line arguments should be used with care. Because command line arguments are matched as a single, concatenated string, a wildcard such as ‘?’ or ‘*’ can match multiple words

The rm command may be changed to unlink since the latter receives single argument only.

But there are also cp commands with wildcard which effectively allow to replace any files with root privileges.

All in all if those vulnerabilities can't be fixed, it's probably better to just set NOPASSWD:ALL, than making false sense of security.

[fgrehm]

Well, I'm not sure there is something we can do about it apart from adding a note on the README saying that it is insecure and should be used for local development only and should be avoided on production environments.

TBH, the only really secure way I can think of is to use user namespaces, but I'm not sure if / when I'll get to that.

/cc @jefmathiot

[tailhook]

So why not just recommend NOPASSWD: ALL, if that provides same sense of security?

[fgrehm]

Wouldn't that allow any kind of command (regardless if it is required by vagrant-lxc or not) to be run without a password? I understand that a malicious user could sudo rm /usr/lib/lxc/templates/something /etc/passwd but at least I will be asked for a password in case I accidentally try to remove another file that is not an lxc template.

The sudoers file is not a requirement for the plugin to work and you don't have to use it if you are worried about the security implications of that. The idea with that is to skip passwords for vagrant-lxc specific operations only, I understand the side effects as you explained but it something I can live with. I think this is a better and less insecure approach than the old dummy wrapper that acted as a simple pass through.

[tailhook]

May be it's beneficial to make safe wrapper script?

[jefmathiot]

@tailhook Totally agree.

I was just discussing the point with @duckmole and @ehartmann. We arrived at the same conclusion:

• create a sudoers file with a single command (wrapper) accepting any number of arguments
• create a wrapper script to check arity, escape the CLI arguments, and build the commands

[fgrehm]

Are you guys willing to work on that safe wrapper script? I had plans to remove that for the final 1.0.0 but if you think it might be useful I can postpone it :-)

[jefmathiot]

@fgrehm I am working on that and following this strategy:

• use the sudoers command to create an vagrant-lxc-wrapper-1.0.0 script in /usr/local/bin
• make this file only modifiable by root
• inside the wrapper, we check the commands and arguments using a whitelist (string litterals or regexp) before executing the commands
• create a NOPASSWD: /usr/local/bin/lxc-wrapper-1.0.0 in a sudoers file
• in the sudo wrapper, if the script has been installed we insert vagrant-lxc-wrapper-1.0.0 between sudo and the actual command. Fallback to sudo otherwise

I've made an inventory of all commands and I guess commands in Driver#compress_rootfs should be kept out of the whitelist. AFAIK it is used only for packaging. The command would allow to chown any file on the file system.

I also plan on adding a no_wrapper: true option to the SudoWrapper#run method.

Are you ok with that ?

[fgrehm]

👍