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

Improve use of sudo for NFS export manipulation #3638

Merged
merged 5 commits into from
May 6, 2014

Conversation

jtopper
Copy link
Contributor

@jtopper 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.

@@ -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"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this missing a sudo before tee?

@mitchellh
Copy link
Contributor

Looks good! Quick question on the Linux usage above.

@jtopper
Copy link
Contributor Author

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
Copy link
Contributor

AWESOME! LGMT>

mitchellh added a commit that referenced this pull request May 6, 2014
Improve use of sudo for NFS export manipulation
@mitchellh mitchellh merged commit 771f951 into hashicorp:master May 6, 2014
@Ramblurr
Copy link

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

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants