summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.rdoc6
-rw-r--r--lib/net/ssh.rb1
-rw-r--r--lib/net/ssh/authentication/key_manager.rb2
-rw-r--r--lib/net/ssh/authentication/methods/hostbased.rb4
-rw-r--r--lib/net/ssh/authentication/methods/keyboard_interactive.rb4
-rw-r--r--lib/net/ssh/authentication/methods/password.rb4
-rw-r--r--lib/net/ssh/authentication/methods/publickey.rb4
-rw-r--r--lib/net/ssh/authentication/session.rb19
-rw-r--r--test/authentication/methods/test_keyboard_interactive.rb10
-rw-r--r--test/authentication/methods/test_password.rb6
-rw-r--r--test/authentication/methods/test_publickey.rb25
-rw-r--r--test/authentication/test_key_manager.rb18
-rw-r--r--test/authentication/test_session.rb13
13 files changed, 97 insertions, 19 deletions
diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc
index 2a18292..5a87cda 100644
--- a/CHANGELOG.rdoc
+++ b/CHANGELOG.rdoc
@@ -1,4 +1,10 @@
+=== 2.1.1 / 1 Mar 2011
+
+* Fix for Net::SSH Continues to attempt authentication when notified it is not allowed [Eric Hodel]
+ (see: http://net-ssh.lighthouseapp.com/projects/36253-net-ssh/tickets/26)
+* Fix for transport won't be closed if authentication fails [Patrick Marchi]
+
=== 2.1 / 19 Jan 2011
diff --git a/lib/net/ssh.rb b/lib/net/ssh.rb
index acc23a7..9f03a98 100644
--- a/lib/net/ssh.rb
+++ b/lib/net/ssh.rb
@@ -193,6 +193,7 @@ module Net
return connection
end
else
+ transport.closed
raise AuthenticationFailed, user
end
end
diff --git a/lib/net/ssh/authentication/key_manager.rb b/lib/net/ssh/authentication/key_manager.rb
index c96e18c..7a0692f 100644
--- a/lib/net/ssh/authentication/key_manager.rb
+++ b/lib/net/ssh/authentication/key_manager.rb
@@ -135,7 +135,7 @@ module Net
if info[:key].nil? && info[:from] == :file
begin
info[:key] = KeyFactory.load_private_key(info[:file], options[:passphrase])
- rescue Exception => e
+ rescue Exception, OpenSSL::OpenSSLError => e
raise KeyManagerError, "the given identity is known, but the private key could not be loaded: #{e.class} (#{e.message})"
end
end
diff --git a/lib/net/ssh/authentication/methods/hostbased.rb b/lib/net/ssh/authentication/methods/hostbased.rb
index 43c3eac..610b64e 100644
--- a/lib/net/ssh/authentication/methods/hostbased.rb
+++ b/lib/net/ssh/authentication/methods/hostbased.rb
@@ -51,6 +51,10 @@ module Net
return true
when USERAUTH_FAILURE
info { "hostbased failed (#{identity.fingerprint})" }
+
+ raise Net::SSH::Authentication::DisallowedMethod unless
+ message[:authentications].split(/,/).include? 'hostbased'
+
return false
else
raise Net::SSH::Exception, "unexpected server response to USERAUTH_REQUEST: #{message.type} (#{message.inspect})"
diff --git a/lib/net/ssh/authentication/methods/keyboard_interactive.rb b/lib/net/ssh/authentication/methods/keyboard_interactive.rb
index 1ab2459..bf17e4d 100644
--- a/lib/net/ssh/authentication/methods/keyboard_interactive.rb
+++ b/lib/net/ssh/authentication/methods/keyboard_interactive.rb
@@ -27,6 +27,10 @@ module Net
return true
when USERAUTH_FAILURE
debug { "keyboard-interactive failed" }
+
+ raise Net::SSH::Authentication::DisallowedMethod unless
+ message[:authentications].split(/,/).include? 'keyboard-interactive'
+
return false
when USERAUTH_INFO_REQUEST
name = message.read_string
diff --git a/lib/net/ssh/authentication/methods/password.rb b/lib/net/ssh/authentication/methods/password.rb
index 9b1c095..ad1eed7 100644
--- a/lib/net/ssh/authentication/methods/password.rb
+++ b/lib/net/ssh/authentication/methods/password.rb
@@ -23,6 +23,10 @@ module Net
return true
when USERAUTH_FAILURE
debug { "password failed" }
+
+ raise Net::SSH::Authentication::DisallowedMethod unless
+ message[:authentications].split(/,/).include? 'password'
+
return false
when USERAUTH_PASSWD_CHANGEREQ
debug { "password change request received, failing" }
diff --git a/lib/net/ssh/authentication/methods/publickey.rb b/lib/net/ssh/authentication/methods/publickey.rb
index b453def..d06af5c 100644
--- a/lib/net/ssh/authentication/methods/publickey.rb
+++ b/lib/net/ssh/authentication/methods/publickey.rb
@@ -70,6 +70,10 @@ module Net
return true
when USERAUTH_FAILURE
debug { "publickey failed (#{identity.fingerprint})" }
+
+ raise Net::SSH::Authentication::DisallowedMethod unless
+ message[:authentications].split(/,/).include? 'publickey'
+
return false
else
raise Net::SSH::Exception,
diff --git a/lib/net/ssh/authentication/session.rb b/lib/net/ssh/authentication/session.rb
index 8e7d3df..180ae46 100644
--- a/lib/net/ssh/authentication/session.rb
+++ b/lib/net/ssh/authentication/session.rb
@@ -9,6 +9,10 @@ require 'net/ssh/authentication/methods/keyboard_interactive'
module Net; module SSH; module Authentication
+ # Raised if the current authentication method is not allowed
+ class DisallowedMethod < Net::SSH::Exception
+ end
+
# Represents an authentication session. It manages the authentication of
# a user over an established connection (the "transport" object, see
# Net::SSH::Transport::Session).
@@ -59,13 +63,16 @@ module Net; module SSH; module Authentication
attempted = []
@auth_methods.each do |name|
- next unless @allowed_auth_methods.include?(name)
- attempted << name
+ begin
+ next unless @allowed_auth_methods.include?(name)
+ attempted << name
- debug { "trying #{name}" }
- method = Methods.const_get(name.split(/\W+/).map { |p| p.capitalize }.join).new(self, :key_manager => key_manager)
+ debug { "trying #{name}" }
+ method = Methods.const_get(name.split(/\W+/).map { |p| p.capitalize }.join).new(self, :key_manager => key_manager)
- return true if method.authenticate(next_service, username, password)
+ return true if method.authenticate(next_service, username, password)
+ rescue Net::SSH::Authentication::DisallowedMethod
+ end
end
error { "all authorization methods failed (tried #{attempted.join(', ')})" }
@@ -131,4 +138,4 @@ module Net; module SSH; module Authentication
Array(options[:key_data])
end
end
-end; end; end \ No newline at end of file
+end; end; end
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
View Lorry Controller Status