Fix for Net::SSH Continues to attempt authentication when notified it is not allowed

5 files changed, 60 insertions, 12 deletions

diff --git a/test/authentication/methods/test_keyboard_interactive.rb b/test/authentication/methods/test_keyboard_interactive.rb
index b092263..2ea1a15 100644
--- a/test/authentication/methods/test_keyboard_interactive.rb
+++ b/test/authentication/methods/test_keyboard_interactive.rb
@@ -10,7 +10,7 @@ module Authentication; module Methods
     USERAUTH_INFO_REQUEST  = 60
     USERAUTH_INFO_RESPONSE = 61
 
-    def test_authenticate_should_be_false_when_server_does_not_support_this_method
+    def test_authenticate_should_raise_if_keyboard_interactive_disallowed
       transport.expect do |t,packet|
         assert_equal USERAUTH_REQUEST, packet.type
         assert_equal "jamis", packet.read_string
@@ -22,7 +22,9 @@ module Authentication; module Methods
         t.return(USERAUTH_FAILURE, :string, "password")
       end
 
-      assert_equal false, subject.authenticate("ssh-connection", "jamis")
+      assert_raises Net::SSH::Authentication::DisallowedMethod do
+        subject.authenticate("ssh-connection", "jamis")
+      end
     end
 
     def test_authenticate_should_be_false_if_given_password_is_not_accepted
@@ -33,7 +35,7 @@ module Authentication; module Methods
           assert_equal USERAUTH_INFO_RESPONSE, packet2.type
           assert_equal 1, packet2.read_long
           assert_equal "the-password", packet2.read_string
-          t2.return(USERAUTH_FAILURE, :string, "publickey")
+          t2.return(USERAUTH_FAILURE, :string, "keyboard-interactive")
         end
       end
 
@@ -95,4 +97,4 @@ module Authentication; module Methods
       end
   end
 
-end; end
\ No newline at end of file
+end; end
diff --git a/test/authentication/methods/test_password.rb b/test/authentication/methods/test_password.rb
index 52f4196..4ba8207 100644
--- a/test/authentication/methods/test_password.rb
+++ b/test/authentication/methods/test_password.rb
@@ -7,7 +7,7 @@ module Authentication; module Methods
   class TestPassword < Test::Unit::TestCase
     include Common
 
-    def test_authenticate_when_password_is_unacceptible_should_return_false
+    def test_authenticate_should_raise_if_password_disallowed
       transport.expect do |t,packet|
         assert_equal USERAUTH_REQUEST, packet.type
         assert_equal "jamis", packet.read_string
@@ -19,7 +19,9 @@ module Authentication; module Methods
         t.return(USERAUTH_FAILURE, :string, "publickey")
       end
 
-      assert !subject.authenticate("ssh-connection", "jamis", "the-password")
+      assert_raises Net::SSH::Authentication::DisallowedMethod do
+        subject.authenticate("ssh-connection", "jamis", "the-password")
+      end
     end
 
     def test_authenticate_when_password_is_acceptible_should_return_true
diff --git a/test/authentication/methods/test_publickey.rb b/test/authentication/methods/test_publickey.rb
index 94d7e3e..b475cf2 100644
--- a/test/authentication/methods/test_publickey.rb
+++ b/test/authentication/methods/test_publickey.rb
@@ -31,6 +31,27 @@ module Authentication; module Methods
       assert_equal false, subject.authenticate("ssh-connection", "jamis")
     end
 
+    def test_authenticate_should_raise_if_publickey_disallowed
+      key_manager.expects(:sign).with(&signature_parameters(keys.first)).returns("sig-one")
+
+      transport.expect do |t, packet|
+        assert_equal USERAUTH_REQUEST, packet.type
+        assert verify_userauth_request_packet(packet, keys.first, false)
+        t.return(USERAUTH_PK_OK, :string, keys.first.ssh_type, :string, Net::SSH::Buffer.from(:key, keys.first))
+
+        t.expect do |t2,packet2|
+          assert_equal USERAUTH_REQUEST, packet2.type
+          assert verify_userauth_request_packet(packet2, keys.first, true)
+          assert_equal "sig-one", packet2.read_string
+          t2.return(USERAUTH_FAILURE, :string, "hostbased,password")
+        end
+      end
+
+      assert_raises Net::SSH::Authentication::DisallowedMethod do
+        subject.authenticate("ssh-connection", "jamis")
+      end
+    end
+
     def test_authenticate_should_return_false_if_signature_exchange_fails
       key_manager.expects(:sign).with(&signature_parameters(keys.first)).returns("sig-one")
       key_manager.expects(:sign).with(&signature_parameters(keys.last)).returns("sig-two")
@@ -44,7 +65,7 @@ module Authentication; module Methods
           assert_equal USERAUTH_REQUEST, packet2.type
           assert verify_userauth_request_packet(packet2, keys.first, true)
           assert_equal "sig-one", packet2.read_string
-          t2.return(USERAUTH_FAILURE, :string, "hostbased,password")
+          t2.return(USERAUTH_FAILURE, :string, "publickey")
 
           t2.expect do |t3, packet3|
             assert_equal USERAUTH_REQUEST, packet3.type
@@ -55,7 +76,7 @@ module Authentication; module Methods
               assert_equal USERAUTH_REQUEST, packet4.type
               assert verify_userauth_request_packet(packet4, keys.last, true)
               assert_equal "sig-two", packet4.read_string
-              t4.return(USERAUTH_FAILURE, :string, "hostbased,password")
+              t4.return(USERAUTH_FAILURE, :string, "publickey")
             end
           end
         end
diff --git a/test/authentication/test_key_manager.rb b/test/authentication/test_key_manager.rb
index ad171ac..3e8c9b0 100644
--- a/test/authentication/test_key_manager.rb
+++ b/test/authentication/test_key_manager.rb
@@ -32,7 +32,7 @@ module Authentication
       manager.stubs(:agent).returns(nil)
 
       stub_file_key "/first", rsa
-      stub_file_key "/second", dsa      
+      stub_file_key "/second", dsa
 
       identities = []
       manager.each_identity { |identity| identities << identity }
@@ -40,7 +40,7 @@ module Authentication
       assert_equal 2, identities.length
       assert_equal rsa.to_blob, identities.first.to_blob
       assert_equal dsa.to_blob, identities.last.to_blob
-      
+
       assert_equal({:from => :file, :file => "/first", :key => rsa}, manager.known_identities[rsa])
       assert_equal({:from => :file, :file => "/second", :key => dsa}, manager.known_identities[dsa])
     end
@@ -82,15 +82,25 @@ module Authentication
 
     def test_sign_with_file_originated_key_should_load_private_key_and_sign_with_it
       manager.stubs(:agent).returns(nil)
-      stub_file_key "/first", rsa(512), true
+      stub_file_key "/first", rsa(512)
       rsa.expects(:ssh_do_sign).with("hello, world").returns("abcxyz123")
       manager.each_identity { |identity| } # preload the known_identities
       assert_equal "\0\0\0\assh-rsa\0\0\0\011abcxyz123", manager.sign(rsa, "hello, world")
     end
 
+    def test_sign_with_file_originated_key_should_raise_key_manager_error_if_unloadable
+      manager.known_identities[rsa] = { :from => :file, :file => "/first" }
+
+      Net::SSH::KeyFactory.expects(:load_private_key).raises(OpenSSL::PKey::RSAError)
+
+      assert_raises Net::SSH::Authentication::KeyManagerError do
+        manager.sign(rsa, "hello, world")
+      end
+    end
+
     private
 
-      def stub_file_key(name, key, also_private=false)
+      def stub_file_key(name, key)
         manager.add(name)
         File.expects(:readable?).with(name).returns(true)
         File.expects(:readable?).with(name + ".pub").returns(false)
diff --git a/test/authentication/test_session.rb b/test/authentication/test_session.rb
index 4cc9bd5..46145fa 100644
--- a/test/authentication/test_session.rb
+++ b/test/authentication/test_session.rb
@@ -12,6 +12,19 @@ module Authentication
       assert_equal session.auth_methods, session.allowed_auth_methods
     end
 
+    def test_authenticate_should_continue_if_method_disallowed
+      transport.expect do |t, packet|
+        assert_equal SERVICE_REQUEST, packet.type
+        assert_equal "ssh-userauth", packet.read_string
+        t.return(SERVICE_ACCEPT)
+      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)
+
+      assert session.authenticate("next service", "username", "password")
+    end
+
     def test_authenticate_should_raise_error_if_service_request_fails
       transport.expect do |t, packet|
         assert_equal SERVICE_REQUEST, packet.type