summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorJeff Forcier <jeff@bitprophet.org>2023-12-17 17:13:53 -0500
committerJeff Forcier <jeff@bitprophet.org>2023-12-17 17:42:11 -0500
commit96db1e2be856eac66631761bae41167a1ebd2b4e (patch)
tree50013470bd5a207af717bd901e63e15277a813ad
parent58785d29c47570fa700e096d16b9a0d3a6069048 (diff)
Raise exception when sequence numbers rollover during initial kex
-rw-r--r--paramiko/packet.py17
-rw-r--r--paramiko/transport.py4
-rw-r--r--sites/www/changelog.rst2
-rw-r--r--tests/test_transport.py32
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...