Skip to content

Commit

Permalink
Raise exception when sequence numbers rollover during initial kex
Browse files Browse the repository at this point in the history
  • Loading branch information
bitprophet committed Dec 17, 2023
1 parent 58785d2 commit 96db1e2
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
17 changes: 13 additions & 4 deletions paramiko/packet.py
Expand Up @@ -86,6 +86,7 @@ def __init__(self, socket):
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
Expand Down Expand Up @@ -425,9 +426,12 @@ def send_message(self, data):
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)
Expand Down Expand Up @@ -531,7 +535,12 @@ def read_message(self):

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
Expand Down
4 changes: 3 additions & 1 deletion paramiko/transport.py
Expand Up @@ -2818,7 +2818,9 @@ def _parse_newkeys(self, m):
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()
Expand Down
2 changes: 2 additions & 0 deletions sites/www/changelog.rst
Expand Up @@ -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.
Expand Down
32 changes: 32 additions & 0 deletions tests/test_transport.py
Expand Up @@ -28,6 +28,7 @@
import time
import threading
import random
import sys
import unittest
from unittest.mock import Mock

Expand Down Expand Up @@ -1368,3 +1369,34 @@ def test_sequence_numbers_not_reset_on_newkeys_when_not_strict(self):
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...

0 comments on commit 96db1e2

Please sign in to comment.