Merge pull request #589 from mfazekas/dont-use-defaults-when-keydata

Dont use defaults when keydata

5 files changed, 229 insertions, 46 deletions

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index f8a993f..8137e49 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -74,12 +74,7 @@ Style/FormatStringToken:
 # Configuration parameters: Blacklist.
 # Blacklist: END, (?-mix:EO[A-Z]{1})
 Naming/HeredocDelimiterNaming:
-  Exclude:
-    - 'test/authentication/test_agent.rb'
-    - 'test/authentication/test_certificate.rb'
-    - 'test/authentication/test_ed25519.rb'
-    - 'test/integration/test_agent.rb'
-    - 'test/test_key_factory.rb'
+  Enabled: false
 
 # Offense count: 3
 # Configuration parameters: MaximumRangeSize.
@@ -95,12 +90,7 @@ Lint/MissingCopEnableDirective:
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent
 Layout/IndentHeredoc:
-  Exclude:
-    - 'test/authentication/test_agent.rb'
-    - 'test/authentication/test_certificate.rb'
-    - 'test/authentication/test_ed25519.rb'
-    - 'test/integration/test_agent.rb'
-    - 'test/test_key_factory.rb'
+  Enabled: false
 
 # Offense count: 2
 # Cop supports --auto-correct.
diff --git a/lib/net/ssh/authentication/key_manager.rb b/lib/net/ssh/authentication/key_manager.rb
index 786ec45..df852f1 100644
--- a/lib/net/ssh/authentication/key_manager.rb
+++ b/lib/net/ssh/authentication/key_manager.rb
@@ -182,6 +182,10 @@ module Net
           nil
         end
 
+        def no_keys?
+          key_files.empty? && key_data.empty?
+        end
+
         private
 
         # Prepares identities from user key_files for loading, preserving their order and sources.
diff --git a/lib/net/ssh/authentication/session.rb b/lib/net/ssh/authentication/session.rb
index 9724d8b..fbe6f6f 100644
--- a/lib/net/ssh/authentication/session.rb
+++ b/lib/net/ssh/authentication/session.rb
@@ -8,8 +8,8 @@ require 'net/ssh/authentication/methods/hostbased'
 require 'net/ssh/authentication/methods/password'
 require 'net/ssh/authentication/methods/keyboard_interactive'
 
-module Net 
-  module SSH 
+module Net
+  module SSH
     module Authentication
 
       # Raised if the current authentication method is not allowed
@@ -27,51 +27,51 @@ module Net
         include Loggable
         include Constants
         include Transport::Constants
-    
+
         # transport layer abstraction
         attr_reader :transport
-    
+
         # the list of authentication methods to try
         attr_reader :auth_methods
-    
+
         # the list of authentication methods that are allowed
         attr_reader :allowed_auth_methods
-    
+
         # a hash of options, given at construction time
         attr_reader :options
-    
+
         # Instantiates a new Authentication::Session object over the given
         # transport layer abstraction.
         def initialize(transport, options={})
           self.logger = transport.logger
           @transport = transport
-    
+
           @auth_methods = options[:auth_methods] || Net::SSH::Config.default_auth_methods
           @options = options
-    
+
           @allowed_auth_methods = @auth_methods
         end
-    
+
         # Attempts to authenticate the given user, in preparation for the next
         # service request. Returns true if an authentication method succeeds in
         # authenticating the user, and false otherwise.
         def authenticate(next_service, username, password=nil)
           debug { "beginning authentication of `#{username}'" }
-    
+
           transport.send_message(transport.service_request("ssh-userauth"))
           expect_message(SERVICE_ACCEPT)
-    
+
           key_manager = KeyManager.new(logger, options)
           keys.each { |key| key_manager.add(key) } unless keys.empty?
           key_data.each { |key2| key_manager.add_key_data(key2) } unless key_data.empty?
-    
+
           attempted = []
-    
+
           @auth_methods.each do |name|
             begin
               next unless @allowed_auth_methods.include?(name)
               attempted << name
-              
+
               debug { "trying #{name}" }
               begin
                 auth_class = Methods.const_get(name.split(/\W+/).map { |p| p.capitalize }.join)
@@ -80,48 +80,48 @@ module Net
                 debug {"Mechanism #{name} was requested, but isn't a known type.  Ignoring it."}
                 next
               end
-              
+
               return true if method.authenticate(next_service, username, password)
             rescue Net::SSH::Authentication::DisallowedMethod
             end
           end
-    
+
           error { "all authorization methods failed (tried #{attempted.join(', ')})" }
           return false
         ensure
           key_manager.finish if key_manager
         end
-    
+
         # Blocks until a packet is received. It silently handles USERAUTH_BANNER
         # packets, and will raise an error if any packet is received that is not
         # valid during user authentication.
         def next_message
           loop do
             packet = transport.next_message
-    
+
             case packet.type
             when USERAUTH_BANNER
               info { packet[:message] }
             # TODO add a hook for people to retrieve the banner when it is sent
-    
+
             when USERAUTH_FAILURE
               @allowed_auth_methods = packet[:authentications].split(/,/)
               debug { "allowed methods: #{packet[:authentications]}" }
               return packet
-    
+
             when USERAUTH_METHOD_RANGE, SERVICE_ACCEPT
               return packet
-    
+
             when USERAUTH_SUCCESS
               transport.hint :authenticated
               return packet
-    
+
             else
               raise Net::SSH::Exception, "unexpected message #{packet.type} (#{packet})"
             end
           end
         end
-    
+
         # Blocks until a packet is received, and returns it if it is of the given
         # type. If it is not, an exception is raised.
         def expect_message(type)
@@ -129,9 +129,9 @@ module Net
           raise Net::SSH::Exception, "expected #{type}, got #{message.type} (#{message})" unless message.type == type
           message
         end
-    
+
         private
-    
+
         # Returns an array of paths to the key files usually defined
         # by system default.
         def default_keys
@@ -142,13 +142,13 @@ module Net
             %w[~/.ssh/id_dsa ~/.ssh/id_rsa ~/.ssh2/id_dsa ~/.ssh2/id_rsa]
           end
         end
-    
+
         # Returns an array of paths to the key files that should be used when
         # attempting any key-based authentication mechanism.
         def keys
-          Array(options[:keys] || default_keys)
+          Array(options[:keys])
         end
-    
+
         # Returns an array of the key data that should be used when
         # attempting any key-based authentication mechanism.
         def key_data
diff --git a/test/authentication/test_session.rb b/test/authentication/test_session.rb
index 3ac6cf7..27b09a7 100644
--- a/test/authentication/test_session.rb
+++ b/test/authentication/test_session.rb
@@ -47,7 +47,7 @@ module Authentication
       Net::SSH::Authentication::Methods::Password.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
       Net::SSH::Authentication::Methods::KeyboardInteractive.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
       Net::SSH::Authentication::Methods::None.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
-      
+
       assert_equal false, session.authenticate("next service", "username", "password")
     end
 
@@ -93,6 +93,79 @@ module Authentication
       assert_equal SERVICE_ACCEPT, session.expect_message(SERVICE_ACCEPT).type
     end
 
+    def test_uses_some_default_keys_if_none_are_provided
+      File.stubs(:file?).returns(false)
+
+      file_on_filesystem("~/.ssh/id_rsa", default_private_key)
+
+      transport.expect do |t, packet|
+        assert_equal SERVICE_REQUEST, packet.type
+        assert_equal "ssh-userauth", packet.read_string
+        t.return(SERVICE_ACCEPT)
+      end
+
+      transport.expect do |t, packet|
+        assert_none_request packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      transport.expect do |t, packet|
+        assert_public_key_request default_public_key, packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      session.authenticate("next service", "username")
+    end
+
+    def test_does_not_use_default_keys_if_keys_are_present_in_options
+      File.stubs(:file?).returns(false)
+
+      file_on_filesystem("~/.ssh/id_rsa", default_private_key)
+      file_on_filesystem("custom_rsa_id", custom_private_key)
+
+      transport.expect do |t, packet|
+        assert_equal SERVICE_REQUEST, packet.type
+        assert_equal "ssh-userauth", packet.read_string
+        t.return(SERVICE_ACCEPT)
+      end
+
+      transport.expect do |t, packet|
+        assert_none_request packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      transport.expect do |t, packet|
+        assert_public_key_request custom_public_key, packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      session(keys: "custom_rsa_id").authenticate("next service", "username")
+    end
+
+    def test_does_not_use_default_keys_if_key_data_are_present_in_options
+      File.stubs(:file?).returns(false)
+
+      file_on_filesystem("~/.ssh/id_rsa", default_private_key)
+
+      transport.expect do |t, packet|
+        assert_equal SERVICE_REQUEST, packet.type
+        assert_equal "ssh-userauth", packet.read_string
+        t.return(SERVICE_ACCEPT)
+      end
+
+      transport.expect do |t, packet|
+        assert_none_request packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      transport.expect do |t, packet|
+        assert_public_key_request custom_public_key, packet
+        t.return(USERAUTH_FAILURE, :string, "publickey")
+      end
+
+      session(key_data: custom_private_key).authenticate("next service", "username")
+    end
+
     private
 
     def session(options={})
@@ -102,6 +175,122 @@ module Authentication
     def transport(options={})
       @transport ||= MockTransport.new(options)
     end
+
+    def assert_none_request(packet)
+      assert_equal "username", packet.read_string
+      assert_equal "next service", packet.read_string
+      assert_equal "none", packet.read_string
+    end
+
+    def assert_public_key_request(public_key, packet)
+      assert_equal "username", packet.read_string
+      assert_equal "next service", packet.read_string
+      assert_equal "publickey", packet.read_string
+      assert_equal false, packet.read_bool
+      assert_equal "ssh-rsa", packet.read_string
+      key_in_packet = Net::SSH::Buffer.new(packet.read_string).read_key
+      assert_equal public_key, key_in_packet.to_pem
+    end
+
+    def file_on_filesystem(name, contents)
+      path = File.expand_path(name)
+      File.stubs(:read).with(path).returns(contents)
+      File.stubs(:file?).with(path).returns(true)
+      File.stubs(:readable?).with(path).returns(true)
+    end
+
+    def custom_private_key
+      <<-EOF
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC3id5gZ6bglJth
+yli8JNaRxhsqKwwPlReEI/mplzz5IP6gWQ92LogXbdBXtHf9ZpA53BeLmtcNBEY0
+Ygd7sPBhlHABS5D5///zltSSX2+L5GCEiC6dpfGsySjqymWF+SZ2PaqfZbkWLmCD
+9u4ysueaHf7xbF6txGprNp69efttWxdy+vU5tno7HVxemMZQUalpShFrdAYKKXEo
+cV7MtbkQjzubS14gaWGpWCXIl9uNKQeHpLKtre1Qn5Ft/zVpCHmhLQcYDuB1LAj9
+7eoev4rIiOE2sfdkvKDlmFxvzq3myYH4o27WwAg9OZ5SBusn2zesKkRCBBEZ55rl
+uVknOGHXAgMBAAECggEAZE0U2OxsNxkfXS6+lXswQ5PW7pF90towcsdSPgrniGIu
+pKRnHbfKKbuaewOl+zZcpTIRL/rbgUKPtzrHSiJlC36aQyrvvJ/ZWV5ZJvC+vd19
+nY/qob65NyrrkHwxRSjmiwGiR9/IaUXI+vUsMUqx5Ph1hawqhZ3sZlEAKR4LeDO8
+M+OguG77jLaqj5/SNfi+GwyUDe85de4VfEG4S9HrMQk2Cp66rx0BqDnCLacyFQaI
+R0VczMXTU52q0uETmgUr8G9A1SaRc5ZWKAfZwxJTvqdIImWC9E+CY7wm+mZD4FE6
+iVzVC0ngcdEd596kTDdU2BPVMluWzLkfqIrTt/5CeQKBgQDzgRzCPNxFtai6RAIi
+ekBSHqrDnrbeTaw32GVq5ACk1Zfk2I0svctz1iQ9qJ2SRINpygQhcyJKQ4r/LXi1
+7Av9H/d6QV4T2AZzS4WcqBkxxRXFUfARtnKChzuCzNt9tNz4EZiv75RyQmztGZjV
+i94+ZvCyqup5be4Svf4MBxin9QKBgQDA9P4nHzFWZakTMei78LGb/4Auc+r0rZp7
+8xg8Z92tvrDeJjMdesdhiFrPP1qiSYHnQ81MSWpn6BycBsHZqitejQmYnYput/s4
+qG+m7SrkN8WL6rijYsbB+U14VDjMlBlOgcEgjlSNU2oeS+68u+uVI/fgyXcXn4Jq
+33TSWSgfGwKBgA2tRdE/G9wqfOShZ0FKfoxePpcoNfs8f5zPYbrkPYkEmjh3VU6b
+Bm9mKrjv3JHXmU3608qRLe7f5lG42xvUu0OnZP4P59nTe2FEb6fB5VBfUn63wHUu
+OzZLpDMPkJB59SNV0a6oFT1pr7aNhoEQDxaQL5rJcMwLOaEB3OAOEft1AoGASz7+
+4Zi7b7rDPVYIMUpCqNfxT6wqovIUPWPmPqAuhXPIm0kAQ+2+VN2MtCc7m+/Ydawu
+IiK7GPweNAY6kDxZH00WweolstmSYVzl9Y2lXUwWgGKvUB/T7I7g1Bzb7YOPftsA
+ykZW2Kn/xwLLfdQ2oXleT82g4Jh2jmDHuMPF7qMCgYEA6QF45PvOgnrJessgmwO/
+dEmkLl07PQYJPGZLaZteuWrvfMrn+AiW5aAdHzhzNaOtNy5B3T7zGUHtgxXegqgd
+/QdCVCJgnZUO/zdAxkr22dDn+WEXkL4wgBVStQvvnQp9C2NJcoOExvex5PLzKWQg
+WEKt5v3QsUEgVrzkM4K9UbI=
+-----END PRIVATE KEY-----
+      EOF
+    end
+
+    def custom_public_key
+      <<-EOF
+-----BEGIN PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAt4neYGem4JSbYcpYvCTW
+kcYbKisMD5UXhCP5qZc8+SD+oFkPdi6IF23QV7R3/WaQOdwXi5rXDQRGNGIHe7Dw
+YZRwAUuQ+f//85bUkl9vi+RghIgunaXxrMko6splhfkmdj2qn2W5Fi5gg/buMrLn
+mh3+8WxercRqazaevXn7bVsXcvr1ObZ6Ox1cXpjGUFGpaUoRa3QGCilxKHFezLW5
+EI87m0teIGlhqVglyJfbjSkHh6Syra3tUJ+Rbf81aQh5oS0HGA7gdSwI/e3qHr+K
+yIjhNrH3ZLyg5Zhcb86t5smB+KNu1sAIPTmeUgbrJ9s3rCpEQgQRGeea5blZJzhh
+1wIDAQAB
+-----END PUBLIC KEY-----
+      EOF
+    end
+
+    def default_private_key
+      <<-EOF
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEAxbz0rp+Z7MklMtSkfiRfceOeTOhOgkGqonCL1B0MRzSjA3yf
+onvEobQNYv7uyQ+ZMGT9RL7AlUSUxeWF00A/O6kuwfs4JlPS/FMPy/B2V0UmoteT
+p40LmclZHpKZs9yKmgkfa5j8Jjvd/VvV1r/DbkHjZetIe07pSnP3EOAG7sjyV7yr
+HPvkgG5h/Vn2U19vTsvYIENcj5OCLF7eUSJZ/6m4qem+wZ4/9cau5E2t57oS8bTd
+5k00Jn0E+qRVionLVLtHXKnr0nWlGPinL+UhKBMhLA6Olm5Y8W77sYcUSvlJMy4G
+mpIvnWFKQE5vim4zKt3dBF256QPmRCWPTQ+sxwIDAQABAoIBAQCG+vrILVjEo3ZK
+IY/8L9Ybh3arJzVYg3z4j/1TmVSlUtAodC0AnJ5Yh/FPb5kPFR/MQlQFVnVeL8ei
+45Ab6dKAZnftoRDuUPBIoGa7H3WZEzJRnPlFOen+W80DKq3TcqwGhE23hGIzs1BR
+QBxUEOlWXZHeI+OBkRd9ZHX2RgdVfhGK1eCRGkxVUx6lygK7RcLSDJgPGvTUL+Gz
+xF4D03pDo2r6oghNk3Fbw4GMXMBLfrKfiee/QyBLEkq+nykVioxXO16ShJfxOzM4
+Pt6/7XJW7uMBGSblS89svrsn7i+29wcgkX4rGWyswV6xicpLNBEmkYx119QKLGBk
+a1QebsYxAoGBAOJL/KEGyO8z9l+b2xzWlAURCNJEqPgeAk6ck7woA+nnj2KH+/51
+1vvbQlvdwN1eP753g3eACvro7XQmVpJzqwBqAGyxtgPoI4F+HR++3lIOA+zfZs3p
+1R1/4AEN0E16sV54gVmvkoSm9UCUDM4RXDdC/YgjpVyXla7HFp2KSrTvAoGBAN+x
+VzK/7hCFod5KZXq/Nfy4/Wg/1GTwzg8eQCUbRJ3jqk0UWvNnhwWTEHfa8ywDSJFi
+bNMlTKtdlGKPHB9dGMt9izGoyeybz0RJz8aCLODN9PBr1GSAhWfqFNYgksScEOy9
+c7eEn25Q91tanmni38Y0KZU9iOYAcKJR5Xulw3WpAoGAO/3lBVNlJXTjFcmdtvFz
+4Dv52LR3Dv/1oJ2F1NXO482Nh5OBTJ401iP0XaJWJNl9kKLiaWW6g3YIrUgUn1Km
+vL9dSXN7S2HZN9UVJ3tUOPCaPcuj12bsJpvl6KGe3UtvhhnwQLR45U3Vqr8U/fRA
+PC44REUe64MMHX+OEUm+MGUCgYB/4UgyURruAxdIl0twYsOgWLk10dfAZRHH/sk4
+7V/Ky45eRlbAc9zyyOJPQrJl5PKlepkwFFDCXtsnhRzUqUo1eu4KU64sP9678V6A
+44Z4dgWjNGHVmsupXl7PEwwUrgvW62+t6HmkfVELvsB1VCgNjWCAWw9aPcImaZ9B
+ksAtEQKBgQCs2ZwTMQOX0IyBMRxVDD8JLCYMNZPBisGTjtYQv6F1ITZOzAPwJjI8
+3FzbcqCWtmbCe6rCd9p9NDU42cuizlSZfO+2emM5CnKdvb0IeHqODfBZm2vYYJ6p
+Euy/YLiXxrwHUo1KecuH04+/s6OxEzMnrYxXqvcK9SwcNTwAkDaBUw==
+-----END RSA PRIVATE KEY-----
+      EOF
+    end
+
+    def default_public_key
+      <<-EOF
+-----BEGIN PUBLIC KEY-----
+MIIBMDANBgkqhkiG9w0BAQEFAAOCAR0AMIIBGAKCARQHc3NoLXJzYQAAAAMBAAEA
+AAEBAMW89K6fmezJJTLUpH4kX3HjnkzoToJBqqJwi9QdDEc0owN8n6J7xKG0DWL+
+7skPmTBk/US+wJVElMXlhdNAPzupLsH7OCZT0vxTD8vwdldFJqLXk6eNC5nJWR6S
+mbPcipoJH2uY/CY73f1b1da/w25B42XrSHtO6Upz9xDgBu7I8le8qxz75IBuYf1Z
+9lNfb07L2CBDXI+Tgixe3lEiWf+puKnpvsGeP/XGruRNree6EvG03eZNNCZ9BPqk
+VYqJy1S7R1yp69J1pRj4py/lISgTISwOjpZuWPFu+7GHFEr5STMuBpqSL51hSkBO
+b4puMyrd3QRduekD5kQlj00PrMc=
+-----END PUBLIC KEY-----
+      EOF
+    end
   end
 
 end
diff --git a/test/common.rb b/test/common.rb
index 05b6110..cdf5627 100644
--- a/test/common.rb
+++ b/test/common.rb
@@ -91,7 +91,7 @@ class MockTransport < Net::SSH::Transport::Session
     self.logger = options[:logger]
     self.host_as_string = "net.ssh.test,127.0.0.1"
     self.server_version = OpenStruct.new(version: "SSH-2.0-Ruby/Net::SSH::Test")
-    @expectation = nil
+    @expectations = []
     @queue = []
     @hints = {}
     @socket = options[:socket]
@@ -101,10 +101,10 @@ class MockTransport < Net::SSH::Transport::Session
 
   def send_message(message)
     buffer = Net::SSH::Buffer.new(message.to_s)
-    if @expectation.nil?
+    if @expectations.empty?
       raise "got #{message.to_s.inspect} but was not expecting anything"
     else
-      block, @expectation = @expectation, nil
+      block = @expectations.shift
       block.call(self, Net::SSH::Packet.new(buffer))
     end
   end
@@ -134,7 +134,7 @@ class MockTransport < Net::SSH::Transport::Session
   end
 
   def expect(&block)
-    @expectation = block
+    @expectations << block
   end
 
   def expect!