diff options
| -rw-r--r-- | paramiko/common.py | 1 | ||||
| -rw-r--r-- | paramiko/transport.py | 45 | ||||
| -rw-r--r-- | tests/test_transport.py | 55 |
3 files changed, 94 insertions, 7 deletions
diff --git a/paramiko/common.py b/paramiko/common.py index 0012372a..11c4121d 100644 --- a/paramiko/common.py +++ b/paramiko/common.py @@ -32,6 +32,7 @@ MSG_USERAUTH_INFO_REQUEST, MSG_USERAUTH_INFO_RESPONSE = range(60, 62) MSG_USERAUTH_GSSAPI_RESPONSE, MSG_USERAUTH_GSSAPI_TOKEN = range(60, 62) MSG_USERAUTH_GSSAPI_EXCHANGE_COMPLETE, MSG_USERAUTH_GSSAPI_ERROR,\ MSG_USERAUTH_GSSAPI_ERRTOK, MSG_USERAUTH_GSSAPI_MIC = range(63, 67) +HIGHEST_USERAUTH_MESSAGE_ID = 79 MSG_GLOBAL_REQUEST, MSG_REQUEST_SUCCESS, MSG_REQUEST_FAILURE = range(80, 83) MSG_CHANNEL_OPEN, MSG_CHANNEL_OPEN_SUCCESS, MSG_CHANNEL_OPEN_FAILURE, \ MSG_CHANNEL_WINDOW_ADJUST, MSG_CHANNEL_DATA, MSG_CHANNEL_EXTENDED_DATA, \ diff --git a/paramiko/transport.py b/paramiko/transport.py index c4305f44..39c6eb89 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -50,7 +50,7 @@ from paramiko.common import ( MSG_CHANNEL_FAILURE, MSG_CHANNEL_DATA, MSG_CHANNEL_EXTENDED_DATA, MSG_CHANNEL_WINDOW_ADJUST, MSG_CHANNEL_REQUEST, MSG_CHANNEL_EOF, MSG_CHANNEL_CLOSE, MIN_WINDOW_SIZE, MIN_PACKET_SIZE, MAX_WINDOW_SIZE, - DEFAULT_WINDOW_SIZE, DEFAULT_MAX_PACKET_SIZE, + DEFAULT_WINDOW_SIZE, DEFAULT_MAX_PACKET_SIZE, HIGHEST_USERAUTH_MESSAGE_ID, ) from paramiko.compress import ZlibCompressor, ZlibDecompressor from paramiko.dsskey import DSSKey @@ -1768,6 +1768,43 @@ class Transport(threading.Thread, ClosingContextManager): max_packet_size = self.default_max_packet_size return clamp_value(MIN_PACKET_SIZE, max_packet_size, MAX_WINDOW_SIZE) + def _ensure_authed(self, ptype, message): + """ + Checks message type against current auth state. + + If server mode, and auth has not succeeded, and the message is of a + post-auth type (channel open or global request) an appropriate error + response Message is crafted and returned to caller for sending. + + Otherwise (client mode, authed, or pre-auth message) returns None. + """ + if ( + not self.server_mode + or ptype <= HIGHEST_USERAUTH_MESSAGE_ID + or self.is_authenticated() + ): + return None + # WELP. We must be dealing with someone trying to do non-auth things + # without being authed. Tell them off, based on message class. + reply = Message() + # Global requests have no details, just failure. + if ptype == MSG_GLOBAL_REQUEST: + reply.add_byte(cMSG_REQUEST_FAILURE) + # Channel opens let us reject w/ a specific type + message. + elif ptype == MSG_CHANNEL_OPEN: + kind = message.get_text() + chanid = message.get_int() + reply.add_byte(cMSG_CHANNEL_OPEN_FAILURE) + reply.add_int(chanid) + reply.add_int(OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED) + reply.add_string('') + reply.add_string('en') + # NOTE: Post-open channel messages do not need checking; the above will + # reject attemps to open channels, meaning that even if a malicious + # user tries to send a MSG_CHANNEL_REQUEST, it will simply fall under + # the logic that handles unknown channel IDs (as the channel list will + # be empty.) + return reply def run(self): # (use the exposed "run" method, because if we specify a thread target @@ -1828,7 +1865,11 @@ class Transport(threading.Thread, ClosingContextManager): continue if ptype in self._handler_table: - self._handler_table[ptype](self, m) + error_msg = self._ensure_authed(ptype, m) + if error_msg: + self._send_message(error_msg) + else: + self._handler_table[ptype](self, m) elif ptype in self._channel_handler_table: chanid = m.get_int() chan = self._channels.get(chanid) diff --git a/tests/test_transport.py b/tests/test_transport.py index 3e352919..99cbc3e0 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -33,7 +33,7 @@ import unittest from paramiko import ( Transport, SecurityOptions, ServerInterface, RSAKey, DSSKey, SSHException, - ChannelException, Packetizer, + ChannelException, Packetizer, Channel, ) from paramiko import AUTH_FAILED, AUTH_SUCCESSFUL from paramiko import OPEN_SUCCEEDED, OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED @@ -91,7 +91,11 @@ class NullServer (ServerInterface): def check_global_request(self, kind, msg): self._global_request = kind - return False + # NOTE: for w/e reason, older impl of this returned False always, even + # tho that's only supposed to occur if the request cannot be served. + # For now, leaving that the default unless test supplies specific + # 'acceptable' request kind + return kind == 'acceptable' def check_channel_x11_request(self, channel, single_connection, auth_protocol, auth_cookie, screen_number): self._x11_single_connection = single_connection @@ -129,7 +133,9 @@ class TransportTest(unittest.TestCase): self.socks.close() self.sockc.close() - def setup_test_server(self, client_options=None, server_options=None): + def setup_test_server( + self, client_options=None, server_options=None, connect_kwargs=None, + ): host_key = RSAKey.from_private_key_file(test_path('test_rsa.key')) public_host_key = RSAKey(data=host_key.asbytes()) self.ts.add_server_key(host_key) @@ -143,8 +149,13 @@ class TransportTest(unittest.TestCase): self.server = NullServer() self.assertTrue(not event.is_set()) self.ts.start_server(event, self.server) - self.tc.connect(hostkey=public_host_key, - username='slowdive', password='pygmalion') + if connect_kwargs is None: + connect_kwargs = dict( + hostkey=public_host_key, + username='slowdive', + password='pygmalion', + ) + self.tc.connect(**connect_kwargs) event.wait(1.0) self.assertTrue(event.is_set()) self.assertTrue(self.ts.is_active()) @@ -927,3 +938,37 @@ class TransportTest(unittest.TestCase): # sendall() accepts a memoryview instance chan.sendall(memoryview(data)) self.assertEqual(sfile.read(len(data)), data) + + def test_server_rejects_open_channel_without_auth(self): + try: + self.setup_test_server(connect_kwargs={}) + self.tc.open_session() + except ChannelException as e: + assert e.code == OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED + else: + assert False, "Did not raise ChannelException!" + + def test_server_rejects_arbitrary_global_request_without_auth(self): + self.setup_test_server(connect_kwargs={}) + # NOTE: this dummy global request kind would normally pass muster + # from the test server. + self.tc.global_request('acceptable') + # Global requests never raise exceptions, even on failure (not sure why + # this was the original design...ugh.) Best we can do to tell failure + # happened is that the client transport's global_response was set back + # to None; if it had succeeded, it would be the response Message. + err = "Unauthed global response incorrectly succeeded!" + assert self.tc.global_response is None, err + + def test_server_rejects_port_forward_without_auth(self): + # NOTE: at protocol level port forward requests are treated same as a + # regular global request, but Paramiko server implements a special-case + # method for it, so it gets its own test. (plus, THAT actually raises + # an exception on the client side, unlike the general case...) + self.setup_test_server(connect_kwargs={}) + try: + self.tc.request_port_forward('localhost', 1234) + except SSHException as e: + assert "forwarding request denied" in str(e) + else: + assert False, "Did not raise SSHException!" |