diff options
Diffstat (limited to 'paramiko/transport.py')
| -rw-r--r-- | paramiko/transport.py | 102 |
1 files changed, 94 insertions, 8 deletions
diff --git a/paramiko/transport.py b/paramiko/transport.py index 14a26333..83019170 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -107,11 +107,12 @@ from paramiko.ecdsakey import ECDSAKey from paramiko.server import ServerInterface from paramiko.sftp_client import SFTPClient from paramiko.ssh_exception import ( - SSHException, BadAuthenticationType, ChannelException, IncompatiblePeer, + MessageOrderError, ProxyCommandFailure, + SSHException, ) from paramiko.util import ( ClosingContextManager, @@ -334,6 +335,8 @@ class Transport(threading.Thread, ClosingContextManager): gss_deleg_creds=True, disabled_algorithms=None, server_sig_algs=True, + strict_kex=True, + packetizer_class=None, ): """ Create a new SSH session over an existing socket, or socket-like @@ -400,6 +403,13 @@ class Transport(threading.Thread, ClosingContextManager): Whether to send an extra message to compatible clients, in server mode, with a list of supported pubkey algorithms. Default: ``True``. + :param bool strict_kex: + Whether to advertise (and implement, if client also advertises + support for) a "strict kex" mode for safer handshaking. Default: + ``True``. + :param packetizer_class: + Which class to use for instantiating the internal packet handler. + Default: ``None`` (i.e.: use `Packetizer` as normal). .. versionchanged:: 1.15 Added the ``default_window_size`` and ``default_max_packet_size`` @@ -410,10 +420,16 @@ class Transport(threading.Thread, ClosingContextManager): Added the ``disabled_algorithms`` kwarg. .. versionchanged:: 2.9 Added the ``server_sig_algs`` kwarg. + .. versionchanged:: 3.4 + Added the ``strict_kex`` kwarg. + .. versionchanged:: 3.4 + Added the ``packetizer_class`` kwarg. """ self.active = False self.hostname = None self.server_extensions = {} + self.advertise_strict_kex = strict_kex + self.agreed_on_strict_kex = False # TODO: these two overrides on sock's type should go away sometime, too # many ways to do it! @@ -457,7 +473,7 @@ class Transport(threading.Thread, ClosingContextManager): self.sock.settimeout(self._active_check_timeout) # negotiated crypto parameters - self.packetizer = Packetizer(sock) + self.packetizer = (packetizer_class or Packetizer)(sock) self.local_version = "SSH-" + self._PROTO_ID + "-" + self._CLIENT_ID self.remote_version = "" self.local_cipher = self.remote_cipher = "" @@ -2086,6 +2102,20 @@ class Transport(threading.Thread, ClosingContextManager): # be empty.) return reply + def _enforce_strict_kex(self, ptype): + """ + Conditionally raise `MessageOrderError` during strict initial kex. + + This method should only be called inside code that handles non-KEXINIT + messages; it does not interrogate ``ptype`` besides using it to log + more accurately. + """ + if self.agreed_on_strict_kex and not self.initial_kex_done: + name = MSG_NAMES.get(ptype, f"msg {ptype}") + raise MessageOrderError( + f"In strict-kex mode, but was sent {name!r}!" + ) + def run(self): # (use the exposed "run" method, because if we specify a thread target # of a private method, threading.Thread will keep a reference to it @@ -2130,16 +2160,21 @@ class Transport(threading.Thread, ClosingContextManager): except NeedRekeyException: continue if ptype == MSG_IGNORE: + self._enforce_strict_kex(ptype) continue elif ptype == MSG_DISCONNECT: self._parse_disconnect(m) break elif ptype == MSG_DEBUG: + self._enforce_strict_kex(ptype) self._parse_debug(m) continue if len(self._expected_packet) > 0: if ptype not in self._expected_packet: - raise SSHException( + exc_class = SSHException + if self.agreed_on_strict_kex: + exc_class = MessageOrderError + raise exc_class( "Expecting packet from {!r}, got {:d}".format( self._expected_packet, ptype ) @@ -2363,12 +2398,18 @@ class Transport(threading.Thread, ClosingContextManager): ) else: available_server_keys = self.preferred_keys - # Signal support for MSG_EXT_INFO. + # Signal support for MSG_EXT_INFO so server will send it to us. # NOTE: doing this here handily means we don't even consider this # value when agreeing on real kex algo to use (which is a common # pitfall when adding this apparently). kex_algos.append("ext-info-c") + # Similar to ext-info, but used in both server modes, so done outside + # of above if/else. + if self.advertise_strict_kex: + which = "s" if self.server_mode else "c" + kex_algos.append(f"kex-strict-{which}-v00@openssh.com") + m = Message() m.add_byte(cMSG_KEXINIT) m.add_bytes(os.urandom(16)) @@ -2409,7 +2450,8 @@ class Transport(threading.Thread, ClosingContextManager): def _get_latest_kex_init(self): return self._really_parse_kex_init( - Message(self._latest_kex_init), ignore_first_byte=True + Message(self._latest_kex_init), + ignore_first_byte=True, ) def _parse_kex_init(self, m): @@ -2448,11 +2490,39 @@ class Transport(threading.Thread, ClosingContextManager): self._log(DEBUG, "kex follows: {}".format(kex_follows)) self._log(DEBUG, "=== Key exchange agreements ===") - # Strip out ext-info "kex algo" + # Record, and strip out, ext-info and/or strict-kex non-algorithms self._remote_ext_info = None + self._remote_strict_kex = None + to_pop = [] for i, algo in enumerate(kex_algo_list): if algo.startswith("ext-info-"): - self._remote_ext_info = kex_algo_list.pop(i) + self._remote_ext_info = algo + to_pop.insert(0, i) + elif algo.startswith("kex-strict-"): + # NOTE: this is what we are expecting from the /remote/ end. + which = "c" if self.server_mode else "s" + expected = f"kex-strict-{which}-v00@openssh.com" + # Set strict mode if agreed. + self.agreed_on_strict_kex = ( + algo == expected and self.advertise_strict_kex + ) + self._log( + DEBUG, f"Strict kex mode: {self.agreed_on_strict_kex}" + ) + to_pop.insert(0, i) + for i in to_pop: + kex_algo_list.pop(i) + + # CVE mitigation: expect zeroed-out seqno anytime we are performing kex + # init phase, if strict mode was negotiated. + if ( + self.agreed_on_strict_kex + and not self.initial_kex_done + and m.seqno != 0 + ): + raise MessageOrderError( + "In strict-kex mode, but KEXINIT was not the first packet!" + ) # as a server, we pick the first item in the client's list that we # support. @@ -2653,6 +2723,13 @@ class Transport(threading.Thread, ClosingContextManager): ): self._log(DEBUG, "Switching on inbound compression ...") self.packetizer.set_inbound_compressor(compress_in()) + # Reset inbound sequence number if strict mode. + if self.agreed_on_strict_kex: + self._log( + DEBUG, + "Resetting inbound seqno after NEWKEYS due to strict mode", + ) + self.packetizer.reset_seqno_in() def _activate_outbound(self): """switch on newly negotiated encryption parameters for @@ -2660,6 +2737,13 @@ class Transport(threading.Thread, ClosingContextManager): m = Message() m.add_byte(cMSG_NEWKEYS) self._send_message(m) + # Reset outbound sequence number if strict mode. + if self.agreed_on_strict_kex: + self._log( + DEBUG, + "Resetting outbound seqno after NEWKEYS due to strict mode", + ) + self.packetizer.reset_seqno_out() block_size = self._cipher_info[self.local_cipher]["block-size"] if self.server_mode: IV_out = self._compute_key("B", block_size) @@ -2750,7 +2834,9 @@ class Transport(threading.Thread, ClosingContextManager): self.auth_handler = AuthHandler(self) if not self.initial_kex_done: # this was the first key exchange - self.initial_kex_done = True + # (also signal to packetizer as it sometimes wants to know this + # status as well, eg when seqnos rollover) + self.initial_kex_done = self.packetizer._initial_kex_done = True # send an event? if self.completion_event is not None: self.completion_event.set() |