diff options
author | Jeff Forcier <jeff@bitprophet.org> | 2023-12-17 17:13:53 -0500 |
---|---|---|
committer | Jeff Forcier <jeff@bitprophet.org> | 2023-12-17 17:42:11 -0500 |
commit | 96db1e2be856eac66631761bae41167a1ebd2b4e (patch) | |
tree | 50013470bd5a207af717bd901e63e15277a813ad | |
parent | 58785d29c47570fa700e096d16b9a0d3a6069048 (diff) |
Raise exception when sequence numbers rollover during initial kex
-rw-r--r-- | paramiko/packet.py | 17 | ||||
-rw-r--r-- | paramiko/transport.py | 4 | ||||
-rw-r--r-- | sites/www/changelog.rst | 2 | ||||
-rw-r--r-- | tests/test_transport.py | 32 |
4 files changed, 50 insertions, 5 deletions
diff --git a/paramiko/packet.py b/paramiko/packet.py index fc3d2de1..1274a23c 100644 --- a/paramiko/packet.py +++ b/paramiko/packet.py @@ -86,6 +86,7 @@ class Packetizer: 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 @@ -425,9 +426,12 @@ class Packetizer: 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) @@ -531,7 +535,12 @@ class Packetizer: 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 f3925861..30d855d7 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -2818,7 +2818,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 87feaa77..8f745b42 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -31,6 +31,8 @@ Changelog -- 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). Thanks to Fabian Bäumer, Marcus Brinkmann, and Jörg Schwenk for submitting details on the CVE prior to release. diff --git a/tests/test_transport.py b/tests/test_transport.py index f9bb89db..ecf0a184 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -28,6 +28,7 @@ import socket import time import threading import random +import sys import unittest from unittest.mock import Mock @@ -1368,3 +1369,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... |