Raise new exception type when unexpected messages appear

5 files changed, 40 insertions, 7 deletions

diff --git a/paramiko/__init__.py b/paramiko/__init__.py
index b9c47dcc..65148d2a 100644
--- a/paramiko/__init__.py
+++ b/paramiko/__init__.py
@@ -59,6 +59,7 @@ from paramiko.ssh_exception import (
     ConfigParseError,
     CouldNotCanonicalize,
     IncompatiblePeer,
+    MessageOrderError,
     PasswordRequiredException,
     ProxyCommandFailure,
     SSHException,
diff --git a/paramiko/ssh_exception.py b/paramiko/ssh_exception.py
index a1e1cfc3..2b68ebe8 100644
--- a/paramiko/ssh_exception.py
+++ b/paramiko/ssh_exception.py
@@ -238,3 +238,13 @@ class ConfigParseError(SSHException):
     """
 
     pass
+
+
+class MessageOrderError(SSHException):
+    """
+    Out-of-order protocol messages were received, violating "strict kex" mode.
+
+    .. versionadded:: 3.4
+    """
+
+    pass
diff --git a/paramiko/transport.py b/paramiko/transport.py
index 22992e17..44361b0c 100644
--- a/paramiko/transport.py
+++ b/paramiko/transport.py
@@ -110,6 +110,7 @@ from paramiko.ssh_exception import (
     BadAuthenticationType,
     ChannelException,
     IncompatiblePeer,
+    MessageOrderError,
     ProxyCommandFailure,
     SSHException,
 )
@@ -2148,7 +2149,10 @@ class Transport(threading.Thread, ClosingContextManager):
                         continue
                     if len(self._expected_packet) > 0:
                         if ptype not in self._expected_packet:
-                            raise SSHException(
+                            exc_class = SSHException
+                            if self.agreed_on_strict_kex:
+                                exc_class = MessageOrderError
+                            raise exc_class(
                                 "Expecting packet from {!r}, got {:d}".format(
                                     self._expected_packet, ptype
                                 )
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index 33f4ae1b..a709152e 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -18,12 +18,14 @@ Changelog
       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.
-    - Paramiko will raise TK if any protocol messages are received
-      out-of-order during key exchange. Previously, TK.
+    - 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.)
+      encountered, which will usually result in, again, `MessageOrderError`.)
 
   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 12078ba7..060a6cae 100644
--- a/tests/test_transport.py
+++ b/tests/test_transport.py
@@ -35,6 +35,7 @@ from paramiko import (
     AuthHandler,
     ChannelException,
     IncompatiblePeer,
+    MessageOrderError,
     Packetizer,
     RSAKey,
     SSHException,
@@ -69,7 +70,7 @@ from ._util import (
     TestServer as NullServer,
 )
 from ._loop import LoopSocket
-from pytest import skip, mark
+from pytest import skip, mark, raises
 
 
 LONG_BANNER = """\
@@ -1279,5 +1280,20 @@ class TestStrictKex:
     def test_sequence_numbers_reset_on_newkeys(self):
         skip()
 
-    def test_error_raised_on_out_of_order_handshakes(self):
-        skip()
+    def test_MessageOrderError_raised_on_out_of_order_messages(self):
+        with raises(MessageOrderError):
+            with server() as (tc, _):
+                # A bit artificial as it's outside kexinit/handshake, but much
+                # easier to trigger and still in line with behavior under test
+                tc._expect_packet(MSG_KEXINIT)
+                tc.open_session()
+
+    def test_SSHException_raised_on_out_of_order_messages_when_not_strict(self):
+        # This is kind of dumb (either situation is still fatal!) but whatever,
+        # may as well be strict with our new strict flag...
+        with raises(SSHException) as info:  # would be true either way, but
+            with server(client_init=dict(strict_kex=False),
+    ) as (tc, _):
+                tc._expect_packet(MSG_KEXINIT)
+                tc.open_session()
+        assert info.type is SSHException  # NOT MessageOrderError!