Skip to content
This repository has been archived by the owner on Mar 19, 2022. It is now read-only.

Using --sudo-command seems buggy, trying to pass environment variables through to the sudo shell #358

Closed
robacarp opened this issue Apr 6, 2014 · 20 comments
Milestone

Comments

@robacarp
Copy link
Contributor

robacarp commented Apr 6, 2014

I keep running into difficulties surrounding the issue of ssh-agent forwarding to clone from git hosted here at github. I've run into this issue in release knife-solo as well as a locally cloned master.

I don't have the bandwidth to dive into it right this moment, but I wanted to document the error here incase someone gets motivated to jump in before I do. :)

In my tests, this resource has been handy:

bash "test github login" do
  code <<-SH
  echo $(whoami)
  ssh git@github.com whoami
  SH
end

When proper authentication credentials are passed, ssh git@github.com will greet you nicely.

I end up getting this output instead:

---- End output of git ls-remote "git@github.com:repo" HEAD ----

STDOUT: 
STDERR: Permission denied (publickey).
fatal: The remote end hung up unexpectedly
---- End output of git ls-remote "git@github.com:repo" HEAD ----

As far as I can tell, the ssh keys are getting passed into the cook, but not up to the sudo. A bit of digging turned up --sudo-command and inspecting the -V -V output of an attempted cook allowed me to assemble this command:

knife solo cook SERVER -V -V --forward-agent --sudo-command "sudo -E -p 'knife sudo password: '"

*For what it is worth, I think this might just do exactly what I am after and forward the ssh-agent on through to the sudoer, *but **

Which results in a request to install Chef on the server:

DEBUG: Initial command sudo chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
/...snip../knife-solo-0.4.2/lib/chef/knife/solo_cook.rb:268:in `check_chef_version': Couldn't find Chef >=0.10.4 on SERVER. Please run `knife solo prepare robert@SERVER -o ForwardAgent=yes` to ensure Chef is installed and up to date. (RuntimeError)

I think that is about all of the relevant -V -V output. This part of the issue may be related to issue #318.

A subtle bug is that the -o ForwardAgent=yes should probably not be on there. I believe it was added as the resolution to issue #328 (in pull #347).

Re running the prepare command does indeed install chef-solo again, but doesn't resolve the problem. I've snipped a few here and there:

$ knife solo prepare SERVER -V -V --forward-agent --sudo-command "sudo -E -p 'knife sudo password: '"
Bootstrapping Chef...
DEBUG: Initial command uname -s
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command uname -s
DEBUG: uname -s stdout: Linux

DEBUG: Initial command lsb_release -d -s
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command lsb_release -d -s
DEBUG: lsb_release -d -s stdout: Ubuntu 12.04.2 LTS

DEBUG: Initial command uname -m
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command uname -m
DEBUG: uname -m stdout: x86_64

DEBUG: Distro detection yielded: {:type=>"debianoid_omnibus"}
DEBUG: Initial command sudo apt-get update
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' apt-get update
Enter the password for robert@SERVER: 
DEBUG: sudo -E -p 'knife sudo password: ' apt-get update stdout: 

0% [Working]-E -p 'knife sudo password: ' apt-get update stdout: 

<snip>

DEBUG: Initial command sudo apt-get -y install rsync ca-certificates wget
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' apt-get -y install rsync ca-certificates wget
DEBUG: Initial command           /bin/sh -c "             if command -v curl >/dev/null 2>&1; then               curl -L -o 'install.sh' 'https://www.opscode.com/chef/install.sh';             else               wget -O 'install.sh' 'https://www.opscode.com/chef/install.sh';             fi;           "

DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command           /bin/sh -c "             if command -v curl >/dev/null 2>&1; then               curl -L -o 'install.sh' 'https://www.opscode.com/chef/install.sh';             else               wget -O 'install.sh' 'https://www.opscode.com/chef/install.sh';             fi;           "

DEBUG:           /bin/sh -c "             if command -v curl >/dev/null 2>&1; then               curl -L -o 'install.sh' 'https://www.opscode.com/chef/install.sh';             else               wget -O 'install.sh' 'https://www.opscode.com/chef/install.sh';             fi;           "
DEBUG: Initial command sudo bash install.sh  -v 11.10.4
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' bash install.sh  -v 11.10.4
Downloading Chef 11.10.4 for ubuntu...

Thank you for installing Chef!
DEBUG: Node config 'nodes/SERVER.json' already exists

(for the sake of completeness:)

$ knife solo cook SERVER -V -V --forward-agent --sudo-command "sudo -E -p 'knife sudo password: '"
Starting 'Run'
Running Chef on SERVER...
Checking Chef version...
DEBUG: Initial command sudo chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
DEBUG: Using replacement sudo command: sudo -E -p 'knife sudo password: '
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'

Cheers

@matschaffer
Copy link
Owner

Interesting. Sounds feasible. It's pretty rare that I use ssh forwarding in my deployments. Any chance you have a test case or vagrantfile that demonstrates this?

@robacarp
Copy link
Contributor Author

@matschaffer It looks like you've been busy the past couple weeks! Thanks for that. I finally had time to put together a test case for demonstrating that sudo should probably have the environment forwarded from a user...but I'm a little fuzzy on the details, so this may be completely redundant with the recent updates to the project.

I've packaged and documented a kitchen, vagrantfile, and a workaround here:

https://github.com/robacarp/knife-solo-test/tree/3a172b03bbc8fc68e74ca21f4ea615b62dd755a1

@matschaffer matschaffer added this to the 0.4.2 milestone Apr 26, 2014
@matschaffer
Copy link
Owner

Thanks! I've put this on the list for 0.4.2, though fwiw my progress on knife-solo has been pretty slow lately. Hoping I can get some time once we move into our new place.

@xtoddx
Copy link
Contributor

xtoddx commented May 27, 2014

Did you remember to ssh-add your identity for github? All I can see in your example is using the vagrant insecure key, which I hope isn't authorized to access your repository.

@robacarp
Copy link
Contributor Author

@xtoddx Yes. Forwarding the authentication agent wouldn't do any good if I wasn't using a valid key to begin with. sudo -E is necessary because I'm holding my ssh key on my laptop and not on the server.

@matschaffer
Copy link
Owner

@robacarp I just tried your repo and it seems to work for me with the latest knife-solo master with your work around (adding the -E).

We could probably add the -E automatically when using --forward-agent. Tbh, I'm a little unclear on how ssh forwarding would even work without it.

@matschaffer
Copy link
Owner

Adding -E across the board doesn't seem to cause any issue in unit or integration testing. Will add that for the 0.4.2 release.

@andoq
Copy link

andoq commented Aug 11, 2014

Unfortunately, this breaks when running chef solo under anything but a superuser account on the server. For example, I generally run chef under a 'chef' account, which has sudo permissions for chef-solo only, and all subsquent runs are run under this account. But this account does not have the permissions to run sudo -E - sudo returns an error 'sorry, you are not allowed to preserve the environment.'

I could give this account permissions, but there is no need for it, and it represents an unneeded grant of security privileges.

In general, this seems like it could cause other problems. I haven't tested, but it seems that all my env variables from my local box are now forwarded on for the sudo command, which could easily have unexpected results for many commands. (Does it take the ruby version, rails env, or other env's from the local machine?)

In general, it seems like it would be better for the original user to configure his machines via env_keep in the sudoers file to allow his specific env variable to be forwarded, rather than blanket having all env variables forwarded by default.

Agent forwarding can be used without needing to forward env variables, as far as I know, but I'm not really clear on the original setup that caused the problem in this thread. I use agent forwarding often, generally setup along the guidelines here: https://developer.github.com/guides/using-ssh-agent-forwarding/, and never had issues before. So I'm not sure what scenario caused the problem.

@matschaffer
Copy link
Owner

Any thoughts on @andoq's comment, @robacarp ?

@robacarp
Copy link
Contributor Author

I don't personally believe in the use of rvm or any other environment level language wedge on production systems, so I can't say how that would be effected.

I've never heard of env_keep until today, so it is possible that is a better solution than flagging sudo with -E. Whitelisting sounds like a better idea than blanket importing everything. I'm not opposed to pulling the flag and making that the recommended solution, but I won't have the time to test that for a week at least.

The scenario I'm working under that caused this problem is (as far as I remember it):

  • private repository on github
  • non-su chef-solo user
  • user level access key to clone the repository is forwarded through ssh-agent
  • chef deploy resource targeted at the private github repository

As far as I understand it, SSH knows how to find the agent through the environment variables, so I'm not sure how you would use it without forwarding at least that part of the environment (possibly through env_keep directives in sudo config).

To your point about an unneeded grant of security priviledges, I'm not sure what you mean. It seems to me that forwarding the environment to the sudo user is an information leak at best (the variables established for the chef user now get forwarded to the other user). Unless you mean the exposure of the private key socket? Can you shed a little more light on your meaning here?

Cheers

@xtoddx
Copy link
Contributor

xtoddx commented Aug 11, 2014

Try #394. I agree that using -E always is optimistic.

Setting env_keep would fix the problem, but isn't practical. That is something I'd set with knife-solo bootstrap and the sudo cookbook, so this has to work without requiring any prerequisites on the server.

@robacarp
Copy link
Contributor Author

Setting env_keep would fix the problem, but isn't practical. That is something I'd set with knife-solo bootstrap and the sudo cookbook, so this has to work without requiring any prerequisites on the server.

Agreed. #394 seems like a fine approach, but I shy away from "config option A automatically enforces config option B".

I'm spitballing here to explore the problem space a little but is it practical to simply have a config option dedicated to this? Given @andoq's comment about security, is that allowing people to shoot themselves in the foot? The option exists in sudo so you can shoot yourself in the foot with it anyway.

What about just letting people write that in the sudo command config parameter?

@xtoddx
Copy link
Contributor

xtoddx commented Aug 11, 2014

tl;dr: Flags are for behaviors.

In my original pull request for agent forwarding (#347) I made a note of using --sudo-command and didn't expect to change the internal behavior of the sudo command.

After living with this a while I think coupling the arguments is called for to encourage the right behavior. I knew to type --sudo-command because I was close to the problem space and familiar with all of the tools. I don't think all users would expect --forward-agent to only forward the agent sometimes, or to a certain depth. While it's true that we can decompose the problem into a series of statements about ssh connections and environments and sudo commands, those are just pieces of architecture that should be exposed only in degenerate cases that the behavior and initial assumptions don't hold out for a particular use-case.

Letting --forward-agent modify the ssh command is an effective way to enforce the Principle of Least Surprise.

@andoq
Copy link

andoq commented Aug 11, 2014

Read around a little, and have a better understanding from a stack answer on this problem: http://serverfault.com/questions/107187/ssh-agent-forwarding-and-sudo-to-another-user.

The fix for #394 could still cause a problem, if the user that is running chef-solo isn't a super user and won't have perms to run sudo -E. But in this scenario, agent-forwarding won't work without the -E option, so I guess it's not a valid scenario...

@matschaffer
Copy link
Owner

The automatic -E is feeling a little magical.

Wondering if we should have a warning to encourage folks to set sudo_command via an in-kitchen knife.rb when specifying --forward-agent. Something like:

Warning: --forward-agent was specified without sudo_command, this may not work if sudo isn't configured to keep SSH_AUTH_SOCK. See http://... for instructions

Bonus points if we could somehow detect failure due this condition and only throw the instructions in that case.

@xtoddx
Copy link
Contributor

xtoddx commented Aug 13, 2014

I've updated my pull request (#394) to include a note about setting --sudo-command into the output of --help if you don't want -E. I still believe it should be the default.

@robacarp
Copy link
Contributor Author

I can't fathom how you could reliably detect failures of this nature, but I do like the description note for the flag. That seems like it will be a simple solution for the bulk of users and lead those who need something more complex down the right path as well.

@sld
Copy link

sld commented Sep 7, 2014

I had the error after upgrading to 0.4.2 after 0.4.1.

knife solo cook deploy@my.server.com -VV
DEBUG: Running processed command sudo -E -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
/Users/user/.rvm/rubies/ruby-2.1.2/lib/ruby/gems/2.1.0/gems/knife-solo-0.4.2/lib/chef/knife/solo_cook.rb:278:in `check_chef_version': Couldn't find Chef >=0.10.4 on my.server.com. 
Please run `knife solo prepare deploy@my.server.com` to ensure Chef is installed and up to date. (RuntimeError)

I think it happens because sudo -E added.

To avoid this error I have to run cooking with --sudo-command:

knife solo cook deploy@my.server.com --sudo-command sudo

@matschaffer
Copy link
Owner

Interesting. What does the command return if you run it directly?

The introduction of the -E didn't fail any integration tests so it might be
good to pin down the situations where it causes trouble.

-Mat

about.me/matschaffer

On Sun, Sep 7, 2014 at 2:08 AM, sld notifications@github.com wrote:

I had the error after upgrading to 0.4.2 after 0.4.1.

knife solo cook deploy@my.server.com -VV

DEBUG: Running processed command sudo -E -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
/Users/user/.rvm/rubies/ruby-2.1.2/lib/ruby/gems/2.1.0/gems/knife-solo-0.4.2/lib/chef/knife/solo_cook.rb:278:in check_chef_version': Couldn't find Chef >=0.10.4 on my.server.com. Please runknife solo prepare deploy@my.server.com` to ensure Chef is installed and up to date. (RuntimeError)

I think it happens because sudo -E added.

To avoid this error I have to run cooking with --sudo-command:

knife solo cook deploy@my.server.com --sudo-command sudo


Reply to this email directly or view it on GitHub
#358 (comment)
.

@sld
Copy link

sld commented Sep 10, 2014

deploy@my.server: sudo -E -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
deploy@my.server:

That command returns nothing.

deploy@my.server: sudo -p 'knife sudo password: ' chef-solo --version 2>/dev/null | awk '$1 == "Chef:" {print $2}'
11.14.6
deploy@my.server: 

Without -E returns 11.14.6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants