File 0006-CVE-2018-1000805-auth_bypass.patch of Package python-paramiko.17745
From 56c96a659658acdbb873aef8809a7b508434dcce Mon Sep 17 00:00:00 2001
From: Jeff Forcier <jeff@bitprophet.org>
Date: Tue, 18 Sep 2018 19:59:16 -0700
Subject: [PATCH] Fix and changelog re #1283
---
paramiko/auth_handler.py | 30 ++++++++++++++++++++--
tests/test_transport.py | 63 +++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 85 insertions(+), 8 deletions(-)
--- a/paramiko/auth_handler.py
+++ b/paramiko/auth_handler.py
@@ -648,13 +648,37 @@ class AuthHandler (object):
self._send_auth_result(
self.auth_username, 'keyboard-interactive', result)
- _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
+
+
--- a/tests/test_transport.py
+++ b/tests/test_transport.py
@@ -28,8 +28,11 @@ import socket
import time
import threading
import random
-from hashlib import sha1
import unittest
+try:
+ from unittest.mock import Mock
+except ImportError:
+ from mock import Mock
from paramiko import (
Transport, SecurityOptions, ServerInterface, RSAKey, DSSKey, SSHException,
@@ -37,11 +40,14 @@ from paramiko import (
)
from paramiko import AUTH_FAILED, AUTH_SUCCESSFUL
from paramiko import OPEN_SUCCEEDED, OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED
+from paramiko.auth_handler import AuthHandler
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,
+ MSG_KEXINIT, cMSG_CHANNEL_WINDOW_ADJUST,
+ cMSG_UNIMPLEMENTED, MIN_PACKET_SIZE, MIN_WINDOW_SIZE,
+ MAX_WINDOW_SIZE, DEFAULT_WINDOW_SIZE,
+ DEFAULT_MAX_PACKET_SIZE, 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
@@ -297,7 +303,7 @@ class TransportTest(unittest.TestCase):
self.assertEqual('Hello there.\n', f.readline())
self.assertEqual('This is on stderr.\n', f.readline())
self.assertEqual('', f.readline())
-
+
def test_6a_channel_can_be_used_as_context_manager(self):
"""
verify that exec_command() does something reasonable.
@@ -927,3 +933,50 @@ class TransportTest(unittest.TestCase):
# sendall() accepts a memoryview instance
chan.sendall(memoryview(data))
self.assertEqual(sfile.read(len(data)), data)
+
+
+ def _send_client_message(self, message_type):
+ self.setup_test_server()
+ 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()
+
+ @unittest.skip("Doesn't seem to work correctly with Mock-ed test server")
+ 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()
