sshkit icon indicating copy to clipboard operation
sshkit copied to clipboard

Feature: ability to set custom sudo command

Open filimonov opened this issue 11 years ago • 15 comments

sometimes it's more comfortable (because of suduers settings) to use sudo -u user cmd

sometimes it's better to have sudo su user -c 'cmd' sudo su - user -c 'cmd'

or even echo 'cmd' | sudo su - username

it's good to have possibility to change the default wrapper.

At this moment I have a server farm with lot of servers and it looks easier to adjust capistrano script than sudoers on all the servers.

And it's also good to have 'default' sudo user for capistrano. So by deafult it creates files / directories using the same user as was used for ssh connection. But for my case i login using my own account, but do a deploy using account common for all users (and which are not possible to connect via ssh).

filimonov avatar Nov 13 '14 15:11 filimonov

I'd be more than happy for a PR with whatever you had in mind. Mabye you could piggyback this on the "command map" https://github.com/capistrano/sshkit#the-command-map ?

I can't see right now a use-case for otherwise using the command map for "sudo", so it seems like this use-case might be ideal.

The code search (https://github.com/capistrano/sshkit/search?utf8=%E2%9C%93&q=sudo) shows that we're not doing a very good job of being consistent anyway.

It's worth noting, just brought to my attention looking through the code results, that 1.5.1 is now considered to have introduced a sudo bug, as sudo -u is not behaviourally equivalent to sudo su, for which we have a (pending?) PR which I ought to look at properly.

leehambley avatar Nov 13 '14 15:11 leehambley

I have only 'sudo su - username' in sudoers. Getting other sudoers will take some time (security team acceptance, admins work and stuff like that). So currently in batch files it's can be done with that dirty code echo 'commang' | sudo su - username

I've tried command map, but it puts arguments at the end. like echo 'mkdir' | sudo su - username /directory - p instead of echo 'mkdir /directory - p' | sudo su - username

filimonov avatar Nov 13 '14 16:11 filimonov

OK, echoing commands into sudo is kinda a nasty hack.

That said, can't you use Ruby to monkey patch out what we're doing, and replace our sudo implementation with your own, just to prove the concept?

I understand why you can't apparently use the command map, which is a shame, but maybe with the command map, and a patch to the sudo wrapper, you might be able to.

leehambley avatar Nov 13 '14 16:11 leehambley

I don't have enough expirience in ruby (ruby is used only for deployment). How can i override that SSHKit::Command::user function leaving the rest as is?

filimonov avatar Nov 13 '14 16:11 filimonov

Honestly, you will struggle... but you can find the implementation (that you hit most often) here:

https://github.com/capistrano/sshkit/blob/fb5e2df8d000ed48428594fd2f87342cce7b6c85/lib/sshkit/command.rb#L170

If you can clone the code and run the tests, change this line to do whatever you think it should do, and then you'll see all the test failures.

Once you have that method looking how you think it needs to look, report back and I'll help you monkey-patch it just into your deployment, if you like.

Long story short, after all the requires in your capfile, you re-define this class, skipping all the methods you don't need to change, and the implementation of def user (&block) which resides in your Capfile will overwrite the one loaded from SSHKit.

leehambley avatar Nov 13 '14 16:11 leehambley

i meant something like:

# in constructor
# That makes it easy to set other formatter at the config level
sudo_format = SSHKit.config.sudo_format || "sudo -u {user} {environment_string}-- sh -c '{cmd}'"; 

def user(&block)
   return yield unless options[:user]
   environment_string = environment_string + " " unless environment_string.empty?
   cmd.gsub("'", "\\\\'")
   res = sudo_format
   res.gsub('{user}', options[:user])
   res.gsub('{cmd}', %Q{#{yield}})
   res.gsub('{environment_string}',environment_string )
   return res
end

I will try to do that monkey patch

Instead of using placeholders - you can alse set formatter function to be able to overwrite easy

filimonov avatar Nov 13 '14 16:11 filimonov

That doesn't work - it uses sudo from normal code. Capfile

# Load DSL and Setup Up Stages
require 'capistrano/setup'

# Includes default deployment tasks
require 'capistrano/deploy'

module SSHKit
  class Command
    def user(&block)
        return yield unless options[:user]
        "echo '#{environment_string + " " unless environment_string.empty?}%s' | sudo su - #{options[:user]}" % %Q{#{yield}}
    end
  end
end

Should I add something here?

filimonov avatar Nov 13 '14 16:11 filimonov

ok, it works, but that fragment https://github.com/capistrano/sshkit/blob/e60b80ab13f0df89a9843dbca28c8163509e3c18/lib/sshkit/backends/abstract.rb#L100 has one more sudo statement.

filimonov avatar Nov 14 '14 08:11 filimonov

You can do the same to replace as(), but as you can see.. You might be crossing the "cool hack" line into WTF territory.

If you can do it, it'd be a great basis for me to make a more permanent fix

Sent from my Nexus 5. On 14 Nov 2014 09:33, "filimonov" [email protected] wrote:

ok, it works, but that fragment

https://github.com/capistrano/sshkit/blob/e60b80ab13f0df89a9843dbca28c8163509e3c18/lib/sshkit/backends/abstract.rb#L100 has one more sudo statement.

— Reply to this email directly or view it on GitHub https://github.com/capistrano/sshkit/issues/191#issuecomment-63024315.

leehambley avatar Nov 14 '14 08:11 leehambley

ok. My monkey patch. It allows to use custom sudo format and to use sudo by default for built-in tasks if needed (if deploy user and ssh-connected user are not the same).

# not sure where it should go inside SSHKit
def format_sudo_call(user, env, cmd)
  env += " " unless env.empty?
  cmd.gsub!("'", "\'\\\\'\'")
  "echo '#{env}#{cmd}' | sudo su - #{user}"
# for more general - something like:
#  res = SSHKit.config.sudo_format || "sudo -u {user} {environment_string}-- sh -c '{cmd}'";
#  return res.gsub('{user}', user).gsub('{cmd}', cmd).gsub('{environment_string}',env )
end

# that will allow to use other user than logged (for run built-in tasks with sudo) 
SSHKit::Configuration.class_eval do
  attr_accessor :default_sudo_user
end

SSHKit::Command.class_eval do
  # in my version of SSHKit there is a typo in method name 
  if SSHKit::Command.instance_methods.include?(:envivonment_string)
    alias :environment_string :envivonment_string
  end

  def user(&block)
    user_name = options[:user] || SSHKit.config.default_sudo_user 
    if ( !user_name || user_name == '@no_sudo_user')
       # with moved here to avoid doubleing of env vars
        return ( with do yield end)
    end
    return format_sudo_call(user_name, environment_string, yield)
  end

  def to_command
    return command.to_s unless should_map?
    user do
      within do
        umask do
       #   with do
            in_background do
              group do
                to_s
              end
            end
       #   end
        end
      end
    end
  end
end;

# monkey patch of as - to support changing of sudo
SSHKit::Backend::Abstract.class_eval do
  def as(who, &block)
    if who.is_a? Hash
      @user = who[:user] || who["user"]
      @group = who[:group] || who["group"]
    else
      @user = who
      @group = nil
    end
    execute <<-EOTEST, verbosity: Logger::DEBUG
  # only next line was changed
  if ! ( #{format_sudo_call(@user, '', 'whoami') } ) > /dev/null
  then echo "You cannot switch to user '#{@user}' using sudo, please check the sudoers file" 1>&2
  false
  fi
    EOTEST
    yield
  ensure
    remove_instance_variable(:@user)
    remove_instance_variable(:@group)
  end
end

# sample usage
SSHKit.config.default_sudo_user = 'my_user_name'

Hope something like that will appear in new version of capistrano / SSHKit

filimonov avatar Nov 14 '14 09:11 filimonov

Thanks, that's quite the patch :) I hope we can find a way to merge it nicely. It's not likely to be in the next release of Cap/SSHKit, as they're already full of changes that are already a bit more severe than I usually like to release, but it's a prime candidate for some refactoring.

leehambley avatar Nov 14 '14 10:11 leehambley

Two more small problems with that separation of users for login and execution.

  1. Uploaded files still has a owner of user loggen in, so git:wrapper step shoul be rewritten a bit to create wrapper directly on the server not uploading it. So instead of
upload! StringIO.new("#!/bin/sh -e\nexec /usr/bin/ssh -o PasswordAuthentication=no -o StrictHostKeyChecking=no \"$@\"\n"), "#{fetch(:tmp_dir)}/#{fetch(:application)}/git-ssh.sh"

something like

execute "echo -e '#!/bin/sh -e\\nexec /usr/bin/ssh -o PasswordAuthentication=no -o StrictHostKeyChecking=no \"$@\"\\n' >  #{fetch(:tmp_dir)}/#{fetch(:application)}/git-ssh.sh"

should be used.

  1. for that default_sudo_user it still good to have possibility to get back to normal user with 'as'
def user(&block)
  user_name = options[:user] || SSHKit.config.default_sudo_user 
  return yield if ( !user_name || user_name == '@no_sudo_user')
  return format_sudo_call(user_name, environment_string, yield)
end 
# and later
# as '@no_sudo_user' do ... end

filimonov avatar Nov 14 '14 11:11 filimonov

hmmm... overriding of git:wrapper action is not so easy (some problems with dynamic loading)... Found solution here: https://github.com/slowjack2k/cap3_test/blob/test_hook/lib/capistrano/tasks/git.rake

I start thinking about getting back to capistrano 2. :\

filimonov avatar Nov 14 '14 12:11 filimonov

and also i was needed to change order in function to_command (i've updated initial comment)

filimonov avatar Nov 14 '14 13:11 filimonov

Thanks for all the feedback, I understand it's tricky.

leehambley avatar Nov 14 '14 13:11 leehambley