From 96db1e2be856eac66631761bae41167a1ebd2b4e Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 17 Dec 2023 17:13:53 -0500 Subject: [PATCH] Raise exception when sequence numbers rollover during initial kex Reference:https://github.com/paramiko/paramiko/commit/96db1e2be856eac66631761bae41167a1ebd2b4e Conflict:Context adaptation exists in the changelog.rst file due to version inconsistency. --- paramiko/packet.py | 17 +++++++++++++---- paramiko/transport.py | 4 +++- sites/www/changelog.rst | 28 ++++++++++++++++++++++++++++ tests/test_transport.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/paramiko/packet.py b/paramiko/packet.py index 1fc06d9..8b9e6d6 100644 --- a/paramiko/packet.py +++ b/paramiko/packet.py @@ -86,6 +86,7 @@ class Packetizer(object): self.__need_rekey = False self.__init_count = 0 self.__remainder = bytes() + self._initial_kex_done = False # used for noticing when to re-key: self.__sent_bytes = 0 @@ -431,9 +432,12 @@ class Packetizer(object): out += compute_hmac( self.__mac_key_out, payload, self.__mac_engine_out )[: self.__mac_size_out] - self.__sequence_number_out = ( - self.__sequence_number_out + 1 - ) & xffffffff + next_seq = (self.__sequence_number_out + 1) & xffffffff + if next_seq == 0 and not self._initial_kex_done: + raise SSHException( + "Sequence number rolled over during initial kex!" + ) + self.__sequence_number_out = next_seq self.write_all(out) self.__sent_bytes += len(out) @@ -537,7 +541,12 @@ class Packetizer(object): msg = Message(payload[1:]) msg.seqno = self.__sequence_number_in - self.__sequence_number_in = (self.__sequence_number_in + 1) & xffffffff + next_seq = (self.__sequence_number_in + 1) & xffffffff + if next_seq == 0 and not self._initial_kex_done: + raise SSHException( + "Sequence number rolled over during initial kex!" + ) + self.__sequence_number_in = next_seq # check for rekey raw_packet_size = packet_size + self.__mac_size_in + 4 diff --git a/paramiko/transport.py b/paramiko/transport.py index 0c68668..750f9b4 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -2785,7 +2785,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 + # staus 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() diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 0675d04..6a2e4c0 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -5,6 +5,34 @@ Changelog - :feature:`-` `Transport` grew a new ``packetizer_class`` kwarg for overriding the packet-handler class used internally. Mostly for testing, but advanced users may find this useful when doing deep hacks. +- :bug:`-` Address `CVE 2023-48795`_ (aka the + "Terrapin Attack", a vulnerability found in the SSH protocol re: treatment of + packet sequence numbers) as follows: + - The vulnerability only impacts encrypt-then-MAC digest algorithms in + tandem with CBC ciphers, and ChaCha20-poly1305; of these, Paramiko + currently only implements ``hmac-sha2-(256|512)-etm`` in tandem with + ``AES-CBC``. If you are unable to upgrade to Paramiko versions containing + the below fixes right away, you may instead use the + ``disabled_algorithms`` connection option to disable the ETM MACs and/or + the CBC ciphers (this option is present in Paramiko >=2.6). + - As the fix for the vulnerability requires both ends of the connection to + cooperate, the below changes will only take effect when the remote end is + OpenSSH >= TK (or equivalent, such as Paramiko in server mode, as of this + patch version) and configured to use the new "strict kex" mode. Paramiko + will always attempt to use "strict kex" mode if offered by the server, + unless you override this by specifying ``strict_kex=False`` in + `Transport.__init__`. + - Paramiko will now raise an `SSHException` subclass (`MessageOrderError`) + when protocol messages are received in unexpected order. (This is not + *really* a change in behavior, as most such cases already raised vanilla + `SSHException` anyways.) + - Key (re)negotiation -- i.e. ``MSG_NEWKEYS``, whenever it is encountered + -- now resets packet sequence numbers. (This should be invisible to users + during normal operation, only causing exceptions if the exploit is + encountered, which will usually result in, again, `MessageOrderError`.) + - Sequence number rollover will now raise `SSHException` if it occurs + during initial key exchange (regardless of strict mode status). + - :bug:`-` Tweak ``ext-info-(c|s)`` detection during KEXINIT protocol phase; the original implementation made assumptions based on an OpenSSH implementation detail. diff --git a/tests/test_transport.py b/tests/test_transport.py index 9c3e8f5..4ed712e 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -30,6 +30,7 @@ import socket import time import threading import random +import sys import unittest from mock import Mock @@ -1571,3 +1572,34 @@ class TestStrictKex: assert tc.packetizer._Packetizer__sequence_number_out != 0 assert ts.packetizer._Packetizer__sequence_number_in != 0 assert ts.packetizer._Packetizer__sequence_number_out != 0 + + def test_sequence_number_rollover_detected(self): + class RolloverTransport(Transport): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Induce an about-to-rollover seqno, such that it rolls over + # during initial kex. + setattr( + self.packetizer, + f"_Packetizer__sequence_number_in", + sys.maxsize, + ) + setattr( + self.packetizer, + f"_Packetizer__sequence_number_out", + sys.maxsize, + ) + + with raises( + SSHException, + match=r"Sequence number rolled over during initial kex!", + ): + with server( + client_init=dict( + # Disable strict kex - this should happen always + strict_kex=False, + ), + # Transport which tickles its packetizer seqno's + transport_factory=RolloverTransport, + ): + pass # kexinit happens at connect... -- 2.33.0