Remove --sudoers, improve --sudoers-no-modify#745
Merged
brianmay merged 1 commit intosshuttle:masterfrom Mar 12, 2022
Merged
Conversation
Allowing sshuttle to add/overwrite sudoers configuration file at locations of the users' choosing adds complexity to the code compared to asking users to install the sudo configuration themselves. It requires sshuttle to make decisions about how much effort we put into ensuring that the file is written to a proper location. The current method relies on the 'realpath' program which is not installed on MacOS by default. There are serious problems when the sudo configuration is used to allow a user to *only* run sshuttle as root (with or without a password). First, that user could then use the --sudoers option to give other users sudo privileges. Second, the user can run any command as root because sshuttle accepts a --ssh-cmd parameter which allows a user to specify a program that sshuttle should run. There may also be additional issues that we have not identified. By removing the --sudoers option (and the associated sudoers-add script), this reduces the problems above. This code keeps the --sudoers-no-modify feature which prints a configuration to stdout for the user to install. It includes a clear warning about how --ssh-cmd could potentially be abused to run other programs. A warning about some of these issues has been in sshuttle since version 1.1.0. This commit also adds that warning to more locations in the documentation.
brianmay
approved these changes
Mar 12, 2022
Member
|
I haven't seen any objections to this proposal in any of the discussions anywhere. Will wait until tomorrow morning, then merge. |
This was referenced Mar 14, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allowing sshuttle to add/overwrite sudoers configuration file at
locations of the users' choosing adds complexity to the code compared
to asking users to install the sudo configuration themselves. It
requires sshuttle to make decisions about how much effort we put into
ensuring that the file is written to a proper location. The current
method relies on the 'realpath' program which is not installed on
MacOS by default.
There are serious problems when the sudo configuration is used to
allow a user to only run sshuttle as root (with or without a
password). First, that user could then use the --sudoers option to
give other users sudo privileges. Second, the user can run any command
as root because sshuttle accepts a --ssh-cmd parameter which allows a
user to specify a program that sshuttle should run. There may also be
additional issues that we have not identified.
By removing the --sudoers option (and the associated sudoers-add
script), this reduces the problems above. This code keeps the
--sudoers-no-modify feature which prints a configuration to stdout for
the user to install. It includes a clear warning about how --ssh-cmd
could potentially be abused to run other programs.
A warning about some of these issues has been in sshuttle since
version 1.1.0. This commit also adds that warning to more locations in
the documentation.