tomaswolf commented on code in PR #565:
URL: https://github.com/apache/mina-sshd/pull/565#discussion_r1693977545
##########
sshd-core/src/main/java/org/apache/sshd/common/session/helpers/KeyExchangeMessageHandler.java:
##########
@@ -236,7 +237,7 @@ public IoWriteFuture writePacket(Buffer buffer, long
timeout, TimeUnit unit) thr
int cmd = bufData[buffer.rpos()] & 0xFF;
boolean enqueued = false;
boolean isLowLevelMessage = cmd <= SshConstants.SSH_MSG_KEX_LAST &&
cmd != SshConstants.SSH_MSG_SERVICE_REQUEST
- && cmd != SshConstants.SSH_MSG_SERVICE_ACCEPT;
+ && cmd != SshConstants.SSH_MSG_SERVICE_ACCEPT && cmd !=
KexExtensions.SSH_MSG_EXT_INFO;
Review Comment:
Crap. I misread the code.
##########
sshd-core/src/main/java/org/apache/sshd/common/session/helpers/KeyExchangeMessageHandler.java:
##########
@@ -236,7 +237,7 @@ public IoWriteFuture writePacket(Buffer buffer, long
timeout, TimeUnit unit) thr
int cmd = bufData[buffer.rpos()] & 0xFF;
boolean enqueued = false;
boolean isLowLevelMessage = cmd <= SshConstants.SSH_MSG_KEX_LAST &&
cmd != SshConstants.SSH_MSG_SERVICE_REQUEST
- && cmd != SshConstants.SSH_MSG_SERVICE_ACCEPT;
+ && cmd != SshConstants.SSH_MSG_SERVICE_ACCEPT && cmd !=
KexExtensions.SSH_MSG_EXT_INFO;
Review Comment:
I disagree with this. Yes, it's message number 7, so technically a
"transport layer generic messages" as RFC 4253 puts it. However, RFC 4253 was
written before RFC 8308, and RFC 4253 excludes already two messages from the
range 1-19.
Let's see: according to RFC 8308, a client may send SSH_MSG_EXT_INFO only
once immediately after its initial SSH_MSG_NEWKEYS. At that point, there is
definitely no KEX ongoing (even if the server required an immediate re-KEX by
sending another SSH_MSG_KEXINIT, that new KEX can only start once the client
has sent _its_ SSH_MSG_KEXINIT, which by definition must be after that
SSH_MSG_EXT_INFO message -- otherwise the client would send SSH_MSG_NEWKEYS -
SSH_MSG_KEXINIT - SSH_MSG_EXT_INFO, which is illegal according to RFC 8308.
A _server_ can likewise send SSH_MSG_EXT_INFO immediately after its initial
SSH_MSG_NEWKEYS. The same reasoning as above applies.
A server can also send the message immediately preceeding its
SSH_MSG_USERAUTH_SUCCESS. But that message cannot be sent while a KEX is
on-going; it would be delayed. So to send it immediately preceeding, the
SSH_MSG_EXT_INFO _must_ equally be delayed, otherwise one could get the
sequence <on-going KEX here> - SSH_MSG_EXT_INFO - SSH_MSG_NEWKEYS -
SSH_MSG_USERAUTH_SUCCESS, which also violates RFC 8308.
Therefore, SSH_MSG_EXT_INFO **must not** be treated as a "transport layer
generic message", and it must never be sent while a KEX is on-going.
The other message from RFC 8308, SSH_MSG_NEWCOMPRESS, also cannot be sent
during a KEX per section 3.2.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
