162 lines
7.2 KiB
Diff
162 lines
7.2 KiB
Diff
From 96db1e2be856eac66631761bae41167a1ebd2b4e Mon Sep 17 00:00:00 2001
|
|
From: Jeff Forcier <jeff@bitprophet.org>
|
|
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<https://terrapin-attack.com/>`_ (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
|