From a4dcfed853d38f60714e55d632c4c7d782eb2332 Mon Sep 17 00:00:00 2001 From: Noel Georgi <18496730+frezbo@users.noreply.github.com> Date: Mon, 25 Jun 2018 21:33:05 +0530 Subject: [PATCH 1/4] Adding proper bastion support --- .gitignore | 1 + lib/train/transports/ssh.rb | 13 ++- lib/train/transports/ssh_connection.rb | 36 ++++-- test/unit/transports/ssh_test.rb | 147 ++++++++++++++++++++++++- 4 files changed, 186 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index a1abf27e..679c4dbf 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ train-*.gem r-train-*.gem Gemfile.lock .kitchen/ +TAGS diff --git a/lib/train/transports/ssh.rb b/lib/train/transports/ssh.rb index 33b29502..e978309f 100644 --- a/lib/train/transports/ssh.rb +++ b/lib/train/transports/ssh.rb @@ -58,6 +58,10 @@ class SSH < Train.plugin(1) # rubocop:disable Metrics/ClassLength option :max_wait_until_ready, default: 600 option :compression, default: false option :pty, default: false + option :custom_proxy_command, default: nil + option :bastion_host, default: nil + option :bastion_user, default: 'root' + option :bastion_port, default: 22 option :compression_level do |opts| # on nil or false: set compression level to 0 @@ -109,6 +113,10 @@ def validate_options(options) logger.warn('[SSH] PTY requested: stderr will be merged into stdout') end + if [options[:custom_proxy_command], options[:bastion_host]].all? { |type| !type.nil? } + fail Train::ClientError, 'Either one of custom_proxy_command or bastion_host needs to be specified' + end + super self end @@ -150,7 +158,10 @@ def connection_options(opts) keys: opts[:key_files], password: opts[:password], forward_agent: opts[:forward_agent], - proxy_command: opts[:proxy_command], + custom_proxy_command: opts[:custom_proxy_command], + bastion_host: opts[:bastion_host], + bastion_user: opts[:bastion_user], + bastion_port: opts[:bastion_port], transport_options: opts, } diff --git a/lib/train/transports/ssh_connection.rb b/lib/train/transports/ssh_connection.rb index 5e1a321d..84042006 100644 --- a/lib/train/transports/ssh_connection.rb +++ b/lib/train/transports/ssh_connection.rb @@ -43,6 +43,10 @@ def initialize(options) @session = nil @transport_options = @options.delete(:transport_options) @cmd_wrapper = nil + @custom_proxy_command = @options.delete(:custom_proxy_command) + @bastion_host = @options.delete(:bastion_host) + @bastion_user = @options.delete(:bastion_user) + @bastion_port = @options.delete(:bastion_port) @cmd_wrapper = CommandWrapper.load(self, @transport_options) end @@ -55,8 +59,7 @@ def close @session = nil end - # (see Base::Connection#login_command) - def login_command + def ssh_opts level = logger.debug? ? 'VERBOSE' : 'ERROR' fwd_agent = options[:forward_agent] ? 'yes' : 'no' @@ -65,13 +68,33 @@ def login_command args += %w{ -o IdentitiesOnly=yes } if options[:keys] args += %W( -o LogLevel=#{level} ) args += %W( -o ForwardAgent=#{fwd_agent} ) if options.key?(:forward_agent) - args += %W( -o ProxyCommand='#{options[:proxy_command]}' ) unless options[:proxy_command].nil? Array(options[:keys]).each do |ssh_key| args += %W( -i #{ssh_key} ) end + args + end + + def check_proxy + [@custom_proxy_command, @bastion_host].any? { |type| !type.nil? } + end + + def generate_proxy_command + return @custom_proxy_command unless @custom_proxy_command.nil? + args = %w{ ssh } + args += ssh_opts + args += %W( #{@bastion_user}@#{@bastion_host} ) + args += %W( -p #{@bastion_port} ) + args += %w{ -W %h:%p } + args.join(' ') + end + + # (see Base::Connection#login_command) + def login_command + args = ssh_opts + args += %W( -o ProxyCommand='#{generate_proxy_command}' ) if check_proxy + # args += %W( -o ProxyCommand='#{generate_proxy_command}' ) unless @bastion_host.nil? args += %W( -p #{@port} ) args += %W( #{@username}@#{@hostname} ) - LoginCommand.new('ssh', args) end @@ -145,10 +168,9 @@ def uri # @api private def establish_connection(opts) logger.debug("[SSH] opening connection to #{self}") - if @options[:proxy_command] + if check_proxy require 'net/ssh/proxy/command' - @options[:proxy] = Net::SSH::Proxy::Command.new(@options[:proxy_command]) - @options.delete(:proxy_command) + @options[:proxy] = Net::SSH::Proxy::Command.new(generate_proxy_command) end Net::SSH.start(@hostname, @username, @options.clone.delete_if { |_key, value| value.nil? }) rescue *RESCUE_EXCEPTIONS_ON_ESTABLISH => e diff --git a/test/unit/transports/ssh_test.rb b/test/unit/transports/ssh_test.rb index 47d71918..0c804dde 100644 --- a/test/unit/transports/ssh_test.rb +++ b/test/unit/transports/ssh_test.rb @@ -14,7 +14,7 @@ host: rand.to_s, password: rand.to_s, key_files: rand.to_s, - proxy_command: 'ssh root@127.0.0.1 -W %h:%p', + custom_proxy_command: 'ssh root@127.0.0.1 -W %h:%p', }} let(:cls_agent) { cls.new({ host: rand.to_s }) } @@ -96,8 +96,8 @@ "-o", "IdentitiesOnly=yes", "-o", "LogLevel=VERBOSE", "-o", "ForwardAgent=no", - "-o", "ProxyCommand='ssh root@127.0.0.1 -W %h:%p'", "-i", conf[:key_files], + "-o", "ProxyCommand='ssh root@127.0.0.1 -W %h:%p'", "-p", "22", "root@#{conf[:host]}", ]) @@ -169,9 +169,150 @@ it 'wont connect if it is not possible' do conf[:host] = 'localhost' conf[:port] = 1 - conf.delete :proxy_command + conf.delete :custom_proxy_command conn = cls.new(conf).connection proc { conn.run_command('uname') }.must_raise Train::Transports::SSHFailed end end end + +describe 'ssh transport with bastion' do + let(:cls) do + plat = Train::Platforms.name('mock').in_family('linux') + plat.add_platform_methods + Train::Platforms::Detect.stubs(:scan).returns(plat) + Train::Transports::SSH + end + + let(:conf) {{ + host: rand.to_s, + password: rand.to_s, + key_files: rand.to_s, + bastion_host: 'bastion_dummy', + }} + let(:cls_agent) { cls.new({ host: rand.to_s }) } + + describe 'bastion' do + describe 'default options' do + let(:ssh) { cls.new({ bastion_host: 'bastion_dummy' }) } + + it 'configures the host' do + ssh.options[:bastion_host].must_equal 'bastion_dummy' + end + + it 'has default port' do + ssh.options[:bastion_port].must_equal 22 + end + + it 'has default user' do + ssh.options[:bastion_user].must_equal 'root' + end + end + + describe 'opening a connection' do + let(:ssh) { cls.new(conf) } + let(:connection) { ssh.connection } + + it 'provides a run_command_via_connection method' do + methods = connection.class.private_instance_methods(false) + methods.include?(:run_command_via_connection).must_equal true + end + + it 'provides a file_via_connection method' do + methods = connection.class.private_instance_methods(false) + methods.include?(:file_via_connection).must_equal true + end + + it 'gets the connection' do + connection.must_be_kind_of Train::Transports::SSH::Connection + end + + it 'provides a uri' do + connection.uri.must_equal "ssh://root@#{conf[:host]}:22" + end + + it 'must respond to wait_until_ready' do + connection.must_respond_to :wait_until_ready + end + + it 'can be closed' do + connection.close.must_be_nil + end + + it 'has a login command == ssh' do + connection.login_command.command.must_equal 'ssh' + end + + it 'has login command arguments' do + connection.login_command.arguments.must_equal([ + "-o", "UserKnownHostsFile=/dev/null", + "-o", "StrictHostKeyChecking=no", + "-o", "IdentitiesOnly=yes", + "-o", "LogLevel=VERBOSE", + "-o", "ForwardAgent=no", + "-i", conf[:key_files], + "-o", "ProxyCommand='ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -o LogLevel=VERBOSE -o ForwardAgent=no -i #{conf[:key_files]} root@bastion_dummy -p 22 -W %h:%p'", + "-p", "22", + "root@#{conf[:host]}", + ]) + end + + it 'sets the right auth_methods when password is specified' do + conf[:key_files] = nil + cls.new(conf).connection.method(:options).call[:auth_methods].must_equal ["none", "password", "keyboard-interactive"] + end + + it 'sets the right auth_methods when keys are specified' do + conf[:password] = nil + cls.new(conf).connection.method(:options).call[:auth_methods].must_equal ["none", "publickey"] + end + + it 'sets the right auth_methods for agent auth' do + cls_agent.stubs(:ssh_known_identities).returns({:some => 'rsa_key'}) + cls_agent.connection.method(:options).call[:auth_methods].must_equal ['none', 'publickey'] + end + + it 'works with ssh agent auth' do + cls_agent.stubs(:ssh_known_identities).returns({:some => 'rsa_key'}) + cls_agent.connection + end + + it 'sets up a proxy when ssh proxy command is specified' do + mock = MiniTest::Mock.new + mock.expect(:call, true) do |hostname, username, options| + options[:proxy].kind_of?(Net::SSH::Proxy::Command) && + "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -o LogLevel=VERBOSE -o ForwardAgent=no -i #{conf[:key_files]} root@bastion_dummy -p 22 -W %h:%p" == options[:proxy].command_line_template + end + connection.stubs(:run_command) + Net::SSH.stub(:start, mock) do + connection.wait_until_ready + end + mock.verify + end + end + end +end + +describe 'ssh transport with bastion and proxy' do + let(:cls) do + plat = Train::Platforms.name('mock').in_family('linux') + plat.add_platform_methods + Train::Platforms::Detect.stubs(:scan).returns(plat) + Train::Transports::SSH + end + + let(:conf) {{ + host: rand.to_s, + password: rand.to_s, + key_files: rand.to_s, + bastion_host: 'bastion_dummy', + custom_proxy_command: 'dummy' + }} + let(:cls_agent) { cls.new({ host: rand.to_s }) } + + describe 'bastion and proxy' do + it 'will throw an exception when both custom_proxy_command and bastion_host is specified' do + proc { cls.new(conf).connection }.must_raise Train::ClientError + end + end +end From 8c54cb9f93f6b5760e5f668a89108bc19e027bfb Mon Sep 17 00:00:00 2001 From: Noel Georgi <18496730+frezbo@users.noreply.github.com> Date: Mon, 25 Jun 2018 21:36:30 +0530 Subject: [PATCH 2/4] Removing commented line Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com> --- lib/train/transports/ssh_connection.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/train/transports/ssh_connection.rb b/lib/train/transports/ssh_connection.rb index 84042006..6df5420f 100644 --- a/lib/train/transports/ssh_connection.rb +++ b/lib/train/transports/ssh_connection.rb @@ -92,7 +92,6 @@ def generate_proxy_command def login_command args = ssh_opts args += %W( -o ProxyCommand='#{generate_proxy_command}' ) if check_proxy - # args += %W( -o ProxyCommand='#{generate_proxy_command}' ) unless @bastion_host.nil? args += %W( -p #{@port} ) args += %W( #{@username}@#{@hostname} ) LoginCommand.new('ssh', args) From abc11eee2c1d7bdd8acc3df40e9390bb8a30a31b Mon Sep 17 00:00:00 2001 From: Noel Georgi <18496730+frezbo@users.noreply.github.com> Date: Wed, 27 Jun 2018 17:20:14 +0530 Subject: [PATCH 3/4] Updating code to use proxy_command as per PR review Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com> --- lib/train/transports/ssh.rb | 8 ++++---- lib/train/transports/ssh_connection.rb | 6 +++--- test/unit/transports/ssh_test.rb | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/train/transports/ssh.rb b/lib/train/transports/ssh.rb index e978309f..8568fc63 100644 --- a/lib/train/transports/ssh.rb +++ b/lib/train/transports/ssh.rb @@ -58,7 +58,7 @@ class SSH < Train.plugin(1) # rubocop:disable Metrics/ClassLength option :max_wait_until_ready, default: 600 option :compression, default: false option :pty, default: false - option :custom_proxy_command, default: nil + option :proxy_command, default: nil option :bastion_host, default: nil option :bastion_user, default: 'root' option :bastion_port, default: 22 @@ -113,8 +113,8 @@ def validate_options(options) logger.warn('[SSH] PTY requested: stderr will be merged into stdout') end - if [options[:custom_proxy_command], options[:bastion_host]].all? { |type| !type.nil? } - fail Train::ClientError, 'Either one of custom_proxy_command or bastion_host needs to be specified' + if [options[:proxy_command], options[:bastion_host]].all? { |type| !type.nil? } + fail Train::ClientError, 'Only one of proxy_command or bastion_host needs to be specified' end super @@ -158,7 +158,7 @@ def connection_options(opts) keys: opts[:key_files], password: opts[:password], forward_agent: opts[:forward_agent], - custom_proxy_command: opts[:custom_proxy_command], + proxy_command: opts[:proxy_command], bastion_host: opts[:bastion_host], bastion_user: opts[:bastion_user], bastion_port: opts[:bastion_port], diff --git a/lib/train/transports/ssh_connection.rb b/lib/train/transports/ssh_connection.rb index 6df5420f..9cd843bc 100644 --- a/lib/train/transports/ssh_connection.rb +++ b/lib/train/transports/ssh_connection.rb @@ -43,7 +43,7 @@ def initialize(options) @session = nil @transport_options = @options.delete(:transport_options) @cmd_wrapper = nil - @custom_proxy_command = @options.delete(:custom_proxy_command) + @proxy_command = @options.delete(:proxy_command) @bastion_host = @options.delete(:bastion_host) @bastion_user = @options.delete(:bastion_user) @bastion_port = @options.delete(:bastion_port) @@ -75,11 +75,11 @@ def ssh_opts end def check_proxy - [@custom_proxy_command, @bastion_host].any? { |type| !type.nil? } + [@proxy_command, @bastion_host].any? { |type| !type.nil? } end def generate_proxy_command - return @custom_proxy_command unless @custom_proxy_command.nil? + return @proxy_command unless @proxy_command.nil? args = %w{ ssh } args += ssh_opts args += %W( #{@bastion_user}@#{@bastion_host} ) diff --git a/test/unit/transports/ssh_test.rb b/test/unit/transports/ssh_test.rb index 0c804dde..a69517b7 100644 --- a/test/unit/transports/ssh_test.rb +++ b/test/unit/transports/ssh_test.rb @@ -14,7 +14,7 @@ host: rand.to_s, password: rand.to_s, key_files: rand.to_s, - custom_proxy_command: 'ssh root@127.0.0.1 -W %h:%p', + proxy_command: 'ssh root@127.0.0.1 -W %h:%p', }} let(:cls_agent) { cls.new({ host: rand.to_s }) } @@ -169,7 +169,7 @@ it 'wont connect if it is not possible' do conf[:host] = 'localhost' conf[:port] = 1 - conf.delete :custom_proxy_command + conf.delete :proxy_command conn = cls.new(conf).connection proc { conn.run_command('uname') }.must_raise Train::Transports::SSHFailed end @@ -306,12 +306,12 @@ password: rand.to_s, key_files: rand.to_s, bastion_host: 'bastion_dummy', - custom_proxy_command: 'dummy' + proxy_command: 'dummy' }} let(:cls_agent) { cls.new({ host: rand.to_s }) } describe 'bastion and proxy' do - it 'will throw an exception when both custom_proxy_command and bastion_host is specified' do + it 'will throw an exception when both proxy_command and bastion_host is specified' do proc { cls.new(conf).connection }.must_raise Train::ClientError end end From b00f090ce8783ca2d8ab9ccfd9bfda4e40a30ee9 Mon Sep 17 00:00:00 2001 From: Noel Georgi <18496730+frezbo@users.noreply.github.com> Date: Wed, 27 Jun 2018 17:23:18 +0530 Subject: [PATCH 4/4] Formatting fix Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com> --- lib/train/transports/ssh.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/train/transports/ssh.rb b/lib/train/transports/ssh.rb index 8568fc63..0f94d32d 100644 --- a/lib/train/transports/ssh.rb +++ b/lib/train/transports/ssh.rb @@ -158,7 +158,7 @@ def connection_options(opts) keys: opts[:key_files], password: opts[:password], forward_agent: opts[:forward_agent], - proxy_command: opts[:proxy_command], + proxy_command: opts[:proxy_command], bastion_host: opts[:bastion_host], bastion_user: opts[:bastion_user], bastion_port: opts[:bastion_port],