Improve use of sudo for NFS export manipulation #3638

Merged
merged 5 commits into from May 6, 2014

Projects

None yet

3 participants

@jtopper
Contributor
jtopper commented May 5, 2014

Some users (myself and the denizens of #2642 included) want to allow Vagrant to manipulate NFS configs without requesting a sudo password each time.

The current approach, using "sudo -s" requires that we permit the whole shell to be run passwordless in sudoers, and that's rather too open to be safe.

This patch uses an alternative idiom, using 'tee' instead of output redirection to append to the exports file. On application of this patch, the following sudoers config (on OSX, at least), permits fully passwordless operation in a much safer manner:

Cmnd_Alias VAGRANT_EXPORTS_ADD = /usr/bin/tee -a /etc/exports
Cmnd_Alias VAGRANT_NFSD = /sbin/nfsd restart
Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /usr/bin/sed -E -e /*/ d -ibak /etc/exports
%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE

This feels like a good interim step if work on the suid helper has paused for now.

NB: I've been unable to test the patch to the Linux plugin at this time.

@mitchellh mitchellh commented on an outdated diff May 6, 2014
plugins/hosts/linux/cap/nfs.rb
@@ -40,7 +40,7 @@ def self.nfs_export(env, ui, id, ips, folders)
output.split("\n").each do |line|
# This should only ask for administrative permission once, even
# though its executed in multiple subshells.
- system(%Q[sudo su root -c "echo '#{line}' >> /etc/exports"])
+ system(%Q[echo '#{line}' | tee -a /etc/exports >/dev/null"])
@mitchellh
mitchellh May 6, 2014 Owner

Is this missing a sudo before tee?

@mitchellh
Owner

Looks good! Quick question on the Linux usage above.

@jtopper
Contributor
jtopper commented May 6, 2014

Ah yes, that'll teach me to commit untested stuff late at night ;)

I've made sure the Linux stuff works now too. Linux sudoers entry should look like this for passwordless operation:

Cmnd_Alias VAGRANT_EXPORTS_ADD = /usr/bin/tee -a /etc/exports
Cmnd_Alias VAGRANT_NFSD_CHECK = /etc/init.d/nfs-kernel-server status
Cmnd_Alias VAGRANT_NFSD_START = /etc/init.d/nfs-kernel-server start
Cmnd_Alias VAGRANT_NFSD_APPLY = /usr/sbin/exportfs -ar
Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /bin/sed -r -e * d -ibak /etc/exports
%sudo ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD_CHECK, VAGRANT_NFSD_START, VAGRANT_NFSD_APPLY, VAGRANT_EXPORTS_REMOVE

I've added example sudoers configs to contrib/ which seems like the right thing to do. It would be good to keep these up to date if the commands in the plugin change.

@mitchellh
Owner

AWESOME! LGMT>

@mitchellh mitchellh merged commit 771f951 into mitchellh:master May 6, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@Ramblurr

FYI, for those on fedora the 2nd and third lines should be:

Cmnd_Alias VAGRANT_NFSD_CHECK = /usr/bin/systemctl status nfs-server
Cmnd_Alias VAGRANT_NFSD_START = /usr/bin/systemctl start nfs-server

Also I add my user to the vagrant group and change the last line to:

%vagrant ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD_CHECK, VAGRANT_NFSD_START, VAGRANT_NFSD_APPLY, VAGRANT_EXPORTS_REMOVE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment