Merge pull request #144 from liggitt/auth_methods_config_parsing

Correctly interpret /etc/ssh_config Authentication settings based on openssh /etc/ssh_config system defaults

7 files changed, 81 insertions, 13 deletions

diff --git a/lib/net/ssh/authentication/session.rb b/lib/net/ssh/authentication/session.rb
index b8650d5..c63bf41 100644
--- a/lib/net/ssh/authentication/session.rb
+++ b/lib/net/ssh/authentication/session.rb
@@ -42,7 +42,7 @@ module Net; module SSH; module Authentication
       self.logger = transport.logger
       @transport = transport
 
-      @auth_methods = options[:auth_methods] || %w(none publickey hostbased password keyboard-interactive)
+      @auth_methods = options[:auth_methods] || Net::SSH::Config.default_auth_methods
       @options = options
 
       @allowed_auth_methods = @auth_methods
diff --git a/lib/net/ssh/config.rb b/lib/net/ssh/config.rb
index b0acf30..fab681b 100644
--- a/lib/net/ssh/config.rb
+++ b/lib/net/ssh/config.rb
@@ -8,6 +8,7 @@ module Net; module SSH
   #
   # Only a subset of OpenSSH configuration options are understood:
   #
+  # * ChallengeResponseAuthentication => maps to the :auth_methods option
   # * Ciphers => maps to the :encryption option
   # * Compression => :compression
   # * CompressionLevel => :compression_level
@@ -25,6 +26,7 @@ module Net; module SSH
   # * Port => :port
   # * PreferredAuthentications => maps to the :auth_methods option
   # * ProxyCommand => maps to the :proxy option
+  # * PubKeyAuthentication => maps to the :auth_methods option
   # * RekeyLimit => :rekey_limit
   # * User => :user
   # * UserKnownHostsFile => :user_known_hosts_file
@@ -35,19 +37,30 @@ module Net; module SSH
   class Config
     class << self
       @@default_files = %w(~/.ssh/config /etc/ssh_config /etc/ssh/ssh_config)
+      # The following defaults follow the openssh client ssh_config defaults.
+      # http://lwn.net/Articles/544640/
+      # "hostbased" is off and "none" is not supported but we allow it since
+      # it's used by some clients to query the server for allowed auth methods
+      @@default_auth_methods = %w(none publickey password keyboard-interactive)
 
       # Returns an array of locations of OpenSSH configuration files
       # to parse by default.
       def default_files
         @@default_files
       end
+      
+      def default_auth_methods
+        @@default_auth_methods
+      end
 
       # Loads the configuration data for the given +host+ from all of the
       # given +files+ (defaulting to the list of files returned by
       # #default_files), translates the resulting hash into the options
       # recognized by Net::SSH, and returns them.
       def for(host, files=default_files)
-        translate(files.inject({}) { |settings, file| load(file, host, settings) })
+        hash = translate(files.inject({}) { |settings, file| 
+          load(file, host, settings)
+        })
       end
 
       # Load the OpenSSH configuration settings in the given +file+ for the
@@ -59,6 +72,8 @@ module Net; module SSH
       def load(path, host, settings={})
         file = File.expand_path(path)
         return settings unless File.readable?(file)
+
+        settings[:auth_methods] ||= default_auth_methods.clone
         
         globals = {}
         matched_host = nil
@@ -119,6 +134,7 @@ module Net; module SSH
       # the returned hash will have Symbols for keys.
       def translate(settings)
         settings.inject({}) do |hash, (key, value)|
+          hash[:auth_methods] ||= settings[:auth_methods] || default_auth_methods.clone
           case key
           when 'bindaddress' then
             hash[:bind_address] = value
@@ -138,8 +154,9 @@ module Net; module SSH
             hash[:global_known_hosts_file] = value
           when 'hostbasedauthentication' then
             if value
-              hash[:auth_methods] ||= []
-              hash[:auth_methods] << "hostbased"
+              (hash[:auth_methods] << "hostbased").uniq!
+            else
+              hash[:auth_methods].delete("hostbased")
             end
           when 'hostkeyalgorithms' then
             hash[:host_key] = value.split(/,/)
@@ -153,8 +170,15 @@ module Net; module SSH
             hash[:hmac] = value.split(/,/)
           when 'passwordauthentication'
             if value
-              hash[:auth_methods] ||= []
-              hash[:auth_methods] << "password"
+              (hash[:auth_methods] << 'password').uniq!
+            else
+              hash[:auth_methods].delete('password')
+            end
+          when 'challengeresponseauthentication'
+            if value
+              (hash[:auth_methods] << 'keyboard-interactive').uniq!
+            else
+              hash[:auth_methods].delete('keyboard-interactive')
             end
           when 'port'
             hash[:port] = value
@@ -165,10 +189,11 @@ module Net; module SSH
               require 'net/ssh/proxy/command'
               hash[:proxy] = Net::SSH::Proxy::Command.new(value)
             end
-	  when 'pubkeyauthentication'
+	        when 'pubkeyauthentication'
             if value
-              hash[:auth_methods] ||= []
-              hash[:auth_methods] << "publickey"
+              (hash[:auth_methods] << 'publickey').uniq!
+            else
+              hash[:auth_methods].delete('publickey')
             end
           when 'rekeylimit'
             hash[:rekey_limit] = interpret_size(value)
diff --git a/test/authentication/test_session.rb b/test/authentication/test_session.rb
index ab233da..ab29640 100644
--- a/test/authentication/test_session.rb
+++ b/test/authentication/test_session.rb
@@ -8,7 +8,7 @@ module Authentication
     include Net::SSH::Authentication::Constants
 
     def test_constructor_should_set_defaults
-      assert_equal %w(none publickey hostbased password keyboard-interactive), session.auth_methods
+      assert_equal %w(none publickey password keyboard-interactive), session.auth_methods
       assert_equal session.auth_methods, session.allowed_auth_methods
     end
 
@@ -20,7 +20,7 @@ module Authentication
       end
 
       Net::SSH::Authentication::Methods::Publickey.any_instance.expects(:authenticate).with("next service", "username", "password").raises(Net::SSH::Authentication::DisallowedMethod)
-      Net::SSH::Authentication::Methods::Hostbased.any_instance.expects(:authenticate).with("next service", "username", "password").returns(true)
+      Net::SSH::Authentication::Methods::Password.any_instance.expects(:authenticate).with("next service", "username", "password").returns(true)
       Net::SSH::Authentication::Methods::None.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
 
       assert session.authenticate("next service", "username", "password")
@@ -44,7 +44,6 @@ module Authentication
       end
 
       Net::SSH::Authentication::Methods::Publickey.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
-      Net::SSH::Authentication::Methods::Hostbased.any_instance.expects(:authenticate).with("next service", "username", "password").returns(false)
       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)
diff --git a/test/configs/auth_off b/test/configs/auth_off
new file mode 100644
index 0000000..6b1b6ef
--- /dev/null
+++ b/test/configs/auth_off
@@ -0,0 +1,4 @@
+HostBasedAuthentication no
+PasswordAuthentication no
+PubKeyAuthentication no
+ChallengeResponseAuthentication no
\ No newline at end of file
diff --git a/test/configs/auth_on b/test/configs/auth_on
new file mode 100644
index 0000000..97d20bc
--- /dev/null
+++ b/test/configs/auth_on
@@ -0,0 +1,4 @@
+HostBasedAuthentication yes
+PasswordAuthentication yes
+PubKeyAuthentication yes
+ChallengeResponseAuthentication yes
\ No newline at end of file
diff --git a/test/configs/empty b/test/configs/empty
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/configs/empty
diff --git a/test/test_config.rb b/test/test_config.rb
index e9961ad..cb462de 100644
--- a/test/test_config.rb
+++ b/test/test_config.rb
@@ -97,7 +97,7 @@ class TestConfig < Test::Unit::TestCase
     assert_equal 6,         net_ssh[:compression_level]
     assert_equal 100,       net_ssh[:timeout]
     assert_equal true,      net_ssh[:forward_agent]
-    assert_equal %w(hostbased password publickey), net_ssh[:auth_methods].sort
+    assert_equal %w(hostbased keyboard-interactive none password publickey), net_ssh[:auth_methods].sort
     assert_equal %w(d e f), net_ssh[:host_key]
     assert_equal %w(g h i), net_ssh[:keys]
     assert_equal %w(j k l), net_ssh[:hmac]
@@ -106,6 +106,42 @@ class TestConfig < Test::Unit::TestCase
     assert_equal "127.0.0.1", net_ssh[:bind_address]
     assert_equal [/^LC_.*$/], net_ssh[:send_env]
   end
+
+  def test_translate_should_turn_off_authentication_methods
+    open_ssh = {
+      'hostbasedauthentication'         => false,
+      'passwordauthentication'          => false,
+      'pubkeyauthentication'            => false,
+      'challengeresponseauthentication' => false
+    }
+
+    net_ssh = Net::SSH::Config.translate(open_ssh)
+
+    assert_equal %w(none), net_ssh[:auth_methods].sort
+  end
+
+  def test_translate_should_turn_on_authentication_methods
+    open_ssh = {
+      'hostbasedauthentication'         => true,
+      'passwordauthentication'          => true,
+      'pubkeyauthentication'            => true,
+      'challengeresponseauthentication' => true
+    }
+
+    net_ssh = Net::SSH::Config.translate(open_ssh)
+
+    assert_equal %w(hostbased keyboard-interactive none password publickey), net_ssh[:auth_methods].sort
+  end
+
+  def test_for_should_turn_off_authentication_methods
+    config = Net::SSH::Config.for("test.host", [config(:empty), config(:auth_off), config(:auth_on)])
+    assert_equal %w(none), config[:auth_methods].sort
+  end
+
+  def test_for_should_turn_on_authentication_methods
+    config = Net::SSH::Config.for("test.host", [config(:empty), config(:auth_on), config(:auth_off)])
+    assert_equal %w(hostbased keyboard-interactive none password publickey), config[:auth_methods].sort
+  end
   
   def test_load_with_plus_sign_hosts
     config = Net::SSH::Config.load(config(:host_plus), "test.host")