diff -ru a/paramiko/auth_handler.py b/paramiko/auth_handler.py
--- a/paramiko/auth_handler.py 2018-03-13 01:02:51.000000000 +0000
+++ b/paramiko/auth_handler.py 2019-01-12 01:14:32.616477122 +0000
@@ -630,17 +630,41 @@
self.auth_event.set()
return
- _handler_table = {
+ # TODO: do the same to the other tables, in Transport.
+ # TODO 3.0: MAY make sense to make these tables into actual
+ # classes/instances that can be fed a mode bool or whatever. Or,
+ # alternately (both?) make the message types small classes or enums that
+ # embed this info within themselves (which could also then tidy up the
+ # current 'integer -> human readable short string' stuff in common.py).
+ # TODO: if we do that, also expose 'em publicly.
+
+ # Messages which should be handled _by_ servers (sent by clients)
+ _server_handler_table = {
MSG_SERVICE_REQUEST: _parse_service_request,
- MSG_SERVICE_ACCEPT: _parse_service_accept,
MSG_USERAUTH_REQUEST: _parse_userauth_request,
+ MSG_USERAUTH_INFO_RESPONSE: _parse_userauth_info_response,
+ }
+
+ # Messages which should be handled _by_ clients (sent by servers)
+ _client_handler_table = {
+ MSG_SERVICE_ACCEPT: _parse_service_accept,
MSG_USERAUTH_SUCCESS: _parse_userauth_success,
MSG_USERAUTH_FAILURE: _parse_userauth_failure,
MSG_USERAUTH_BANNER: _parse_userauth_banner,
MSG_USERAUTH_INFO_REQUEST: _parse_userauth_info_request,
- MSG_USERAUTH_INFO_RESPONSE: _parse_userauth_info_response,
}
+ # NOTE: prior to the fix for #1283, this was a static dict instead of a
+ # property. Should be backwards compatible in most/all cases.
+ @property
+ def _handler_table(self):
+ if self.transport.server_mode:
+ return self._server_handler_table
+ else:
+ return self._client_handler_table
+
+
+
class GssapiWithMicAuthHandler(object):
"""A specialized Auth handler for gssapi-with-mic
@@ -733,9 +757,15 @@
self._restore_delegate_auth_handler()
return self._delegate._parse_userauth_request(m)
- _handler_table = {
+ __handler_table = {
MSG_SERVICE_REQUEST: _parse_service_request,
MSG_USERAUTH_REQUEST: _parse_userauth_request,
MSG_USERAUTH_GSSAPI_TOKEN: _parse_userauth_gssapi_token,
MSG_USERAUTH_GSSAPI_MIC: _parse_userauth_gssapi_mic,
}
+
+ @property
+ def _handler_table(self):
+ # TODO: determine if we can cut this up like we did for the primary
+ # AuthHandler class.
+ return self.__handler_table
diff -ru a/sites/www/changelog.rst b/sites/www/changelog.rst
--- a/sites/www/changelog.rst 2018-03-13 01:02:51.000000000 +0000
+++ b/sites/www/changelog.rst 2019-01-12 01:22:56.894066134 +0000
@@ -3,6 +3,18 @@
=========
* :release:`2.1.5 <2018-03-12>`
+- :bug:`1283 (1.17+)` Fix exploit (CVE-2018-1000805) in Paramiko's server mode
+ (**not** client mode) where hostile clients could trick the server into
+ thinking they were authenticated without actually submitting valid
+ authentication.
+
+ Specifically, steps have been taken to start separating client and server
+ related message types in the message handling tables within ``Transport`` and
+ ``AuthHandler``; this work is not complete but enough has been performed to
+ close off this particular exploit (which was the only obvious such exploit
+ for this particular channel).
+
+ Thanks to Daniel Hoffman for the detailed report.
* :release:`2.0.8 <2018-03-12>`
* :bug:`1175 (1.17+)` Fix a security flaw (CVE-2018-7750) in Paramiko's server
mode (emphasis on **server** mode; this does **not** impact *client* use!)
diff -ru a/tests/test_transport.py b/tests/test_transport.py
--- a/tests/test_transport.py 2018-03-13 01:02:51.000000000 +0000
+++ b/tests/test_transport.py 2019-01-12 01:38:32.617671204 +0000
@@ -33,15 +33,16 @@
from paramiko import (
Transport, SecurityOptions, ServerInterface, RSAKey, DSSKey, SSHException,
- ChannelException, Packetizer, Channel,
+ ChannelException, Packetizer, Channel, AuthHandler,
)
from paramiko import AUTH_FAILED, AUTH_SUCCESSFUL
from paramiko import OPEN_SUCCEEDED, OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED
from paramiko.common import (
MSG_KEXINIT, cMSG_CHANNEL_WINDOW_ADJUST, MIN_PACKET_SIZE, MIN_WINDOW_SIZE,
- MAX_WINDOW_SIZE, DEFAULT_WINDOW_SIZE, DEFAULT_MAX_PACKET_SIZE,
+ MAX_WINDOW_SIZE, DEFAULT_WINDOW_SIZE, DEFAULT_MAX_PACKET_SIZE, MSG_NAMES,
+ MSG_UNIMPLEMENTED, MSG_USERAUTH_SUCCESS,
)
-from paramiko.py3compat import bytes
+from paramiko.py3compat import bytes, byte_chr
from paramiko.message import Message
from tests import skipUnlessBuiltin
from tests.loop import LoopSocket
@@ -972,3 +973,49 @@
assert "forwarding request denied" in str(e)
else:
assert False, "Did not raise SSHException!"
+
+
+ def _send_client_message(self, message_type):
+ self.setup_test_server(connect_kwargs={})
+ self.ts._send_message = Mock()
+ # NOTE: this isn't 100% realistic (most of these message types would
+ # have actual other fields in 'em) but it suffices to test the level of
+ # message dispatch we're interested in here.
+ msg = Message()
+ # TODO: really not liking the whole cMSG_XXX vs MSG_XXX duality right
+ # now, esp since the former is almost always just byte_chr(the
+ # latter)...but since that's the case...
+ msg.add_byte(byte_chr(message_type))
+ self.tc._send_message(msg)
+ # No good way to actually wait for server action (see above tests re:
+ # MSG_UNIMPLEMENTED). Grump.
+ time.sleep(0.1)
+
+ def _expect_unimplemented(self):
+ # Ensure MSG_UNIMPLEMENTED was sent (implies it hit end of loop instead
+ # of truly handling the given message).
+ # NOTE: When bug present, this will actually be the first thing that
+ # fails (since in many cases actual message handling doesn't involve
+ # sending a message back right away).
+ assert self.ts._send_message.call_count == 1
+ reply = self.ts._send_message.call_args[0][0]
+ reply.rewind() # Because it's pre-send, not post-receive
+ assert reply.get_byte() == cMSG_UNIMPLEMENTED
+
+ def test_server_transports_reject_client_message_types(self):
+ # TODO: handle Transport's own tables too, not just its inner auth
+ # handler's table. See TODOs in auth_handler.py
+ for message_type in AuthHandler._client_handler_table:
+ self._send_client_message(message_type)
+ self._expect_unimplemented()
+ # Reset for rest of loop
+ self.tearDown()
+ self.setUp()
+
+ def test_server_rejects_client_MSG_USERAUTH_SUCCESS(self):
+ self._send_client_message(MSG_USERAUTH_SUCCESS)
+ # Sanity checks
+ assert not self.ts.authenticated
+ assert not self.ts.auth_handler.authenticated
+ # Real fix's behavior
+ self._expect_unimplemented()