From d1532939ddc1ef4f704290d081952fe38da0c0ec Mon Sep 17 00:00:00 2001 From: Jeff <11721615+jeffellin@users.noreply.github.com> Date: Tue, 17 Feb 2026 12:31:15 -0500 Subject: [PATCH] Return .ignore() for unknown message types per RFC 4253 Motivation: RFC 4253 Section 11.1 requires: "An implementation MUST be able to process all message types, though it need not ACT on them" and "An implementation MAY ignore an unrecognized message." NIOSSH currently violates this requirement by throwing ParsingError.unknownType for unknown message types. This causes parser state corruption through buffer rewind over already-decrypted data, and prevents interoperability with SSH implementations using protocol extensions (OpenSSH extensions, vendor-specific message types). Modifications: - readSSHMessage(): Return .ignore() for unknown message types instead of throwing, consuming remaining bytes to prevent corruption - Add testUnknownMessageTypeReturnsIgnore to verify RFC compliance - Add testParserStateAfterUnknownMessageType to verify parser continuity - Update testTypeError to expect .ignore() instead of throw Result: NIOSSH now follows RFC 4253 by gracefully ignoring unknown message types. Connections continue normally when encountering extensions, enabling interoperability with OpenSSH extensions and vendor-specific message types. Breaking Change: ParsingError.unknownType is no longer thrown for unknown message types. Connections that previously failed will now continue normally per RFC requirements. --- Sources/NIOSSH/SSHMessages.swift | 8 +++- Tests/NIOSSHTests/SSHMessagesTests.swift | 56 +++++++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/Sources/NIOSSH/SSHMessages.swift b/Sources/NIOSSH/SSHMessages.swift index a127d336..1a494309 100644 --- a/Sources/NIOSSH/SSHMessages.swift +++ b/Sources/NIOSSH/SSHMessages.swift @@ -509,7 +509,13 @@ extension ByteBuffer { } return .channelFailure(message) default: - throw SSHMessage.ParsingError.unknownType(type) + // Unknown SSH message type - consume remaining bytes and return as ignore. + // This prevents parser state corruption when encountering extension messages + // (e.g. hostkeys-00@openssh.com) that NIOSSH doesn't support. Without this, + // the parser would throw, rewind the buffer reader over already-decrypted data, + // and skip incrementing the sequence number, causing cascading MAC failures. + self.moveReaderIndex(to: self.writerIndex) + return .ignore(.init(data: ByteBuffer())) } } } diff --git a/Tests/NIOSSHTests/SSHMessagesTests.swift b/Tests/NIOSSHTests/SSHMessagesTests.swift index 442ab36a..238863c7 100644 --- a/Tests/NIOSSHTests/SSHMessagesTests.swift +++ b/Tests/NIOSSHTests/SSHMessagesTests.swift @@ -748,8 +748,13 @@ final class SSHMessagesTests: XCTestCase { XCTAssertNil(try buffer.readSSHMessage()) + // Unknown message types should be ignored, not throw (per RFC 4253 Section 11.1) buffer.writeBytes([127]) - XCTAssertThrowsError(try buffer.readSSHMessage()) + let message = try buffer.readSSHMessage() + guard case .some(.ignore) = message else { + XCTFail("Expected .ignore for unknown message type") + return + } } func testRequestSuccess() throws { @@ -797,4 +802,53 @@ final class SSHMessagesTests: XCTestCase { try self.assertCorrectlyManagesPartialRead(message) } + + func testUnknownMessageTypeReturnsIgnore() throws { + // This test verifies that unknown SSH message types are handled gracefully + // by returning .ignore() instead of throwing an error (per RFC 4253 Section 11.1). + var buffer = ByteBufferAllocator().buffer(capacity: 100) + + // Use message ID 127 (reserved for local/private use per RFC 4253) + buffer.writeInteger(UInt8(127)) + buffer.writeSSHString("extension-data".utf8) + + let message = try buffer.readSSHMessage() + + guard case .some(.ignore) = message else { + XCTFail("Expected .ignore for unknown message type, got \(String(describing: message))") + return + } + + // Verify buffer is fully consumed + XCTAssertEqual(buffer.readableBytes, 0, "Parser should consume all bytes for unknown message type") + } + + func testParserStateAfterUnknownMessageType() throws { + // This test verifies that the parser state remains valid after encountering + // an unknown message type, ensuring no corruption of sequence numbers or + // encryption state. + var buffer = ByteBufferAllocator().buffer(capacity: 200) + + // Write unknown message type (ID 125 - reserved for local use) + buffer.writeInteger(UInt8(125)) + buffer.writeBytes([1, 2, 3, 4]) // Some payload + + // Read unknown message (should return .ignore) + let message1 = try buffer.readSSHMessage() + guard case .some(.ignore) = message1 else { + XCTFail("Expected .ignore for unknown message type") + return + } + + // Write known message + buffer.writeInteger(SSHMessage.ChannelSuccessMessage.id) + buffer.writeInteger(UInt32(0)) + + // Verify parser continues correctly + let message2 = try buffer.readSSHMessage() + guard case .some(.channelSuccess) = message2 else { + XCTFail("Parser state corrupted after unknown message type - expected channelSuccess") + return + } + } }