diff --git a/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json b/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json index afdde848..957e4fe9 100644 --- a/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json +++ b/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 190655 + "mallocCountTotal": 190652 } diff --git a/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.OneCommandPerConnection.p90.json b/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.OneCommandPerConnection.p90.json index b6f8bba8..74c47cb0 100644 --- a/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.OneCommandPerConnection.p90.json +++ b/Benchmarks/Thresholds/6.1/NIOSSHBenchmarks.OneCommandPerConnection.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 792051 + "mallocCountTotal": 789051 } diff --git a/Benchmarks/Thresholds/6.2/NIOSSHBenchmarks.OneCommandPerConnection.p90.json b/Benchmarks/Thresholds/6.2/NIOSSHBenchmarks.OneCommandPerConnection.p90.json index dad48cee..21acd905 100644 --- a/Benchmarks/Thresholds/6.2/NIOSSHBenchmarks.OneCommandPerConnection.p90.json +++ b/Benchmarks/Thresholds/6.2/NIOSSHBenchmarks.OneCommandPerConnection.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 752035 + "mallocCountTotal": 751035 } diff --git a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json index 80036777..aecfb54f 100644 --- a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json +++ b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.ManySmallCommandsPerConnection.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 190567 + "mallocCountTotal": 190566 } diff --git a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.OneCommandPerConnection.p90.json b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.OneCommandPerConnection.p90.json index 63e82638..cec23b59 100644 --- a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.OneCommandPerConnection.p90.json +++ b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.OneCommandPerConnection.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 755999 + "mallocCountTotal": 754999 } diff --git a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.StreamingLargeMessageInSmallChunks.p90.json b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.StreamingLargeMessageInSmallChunks.p90.json index ac653ee3..6a026cd1 100644 --- a/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.StreamingLargeMessageInSmallChunks.p90.json +++ b/Benchmarks/Thresholds/6.3/NIOSSHBenchmarks.StreamingLargeMessageInSmallChunks.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal": 40736 + "mallocCountTotal": 40735 } diff --git a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift index a6b286eb..d98cd3e9 100644 --- a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift +++ b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift @@ -490,8 +490,8 @@ extension SSHChildChannel: Channel, ChannelCore { recipientChannel: self.state.remoteChannelIdentifier!, senderChannel: self.state.localChannelIdentifier, initialWindowSize: self.windowManager.targetWindowSize, - maximumPacketSize: 1 << 24 - ) // This is a weirdly hard-coded choice. + maximumPacketSize: Constants.maximumChannelPacketSize + ) self.processOutboundMessage(.channelOpenConfirmation(message), promise: nil) self.writePendingToMultiplexer() } else if !self.state.isClosed { @@ -500,7 +500,7 @@ extension SSHChildChannel: Channel, ChannelCore { type: .init(self.type!), senderChannel: self.state.localChannelIdentifier, initialWindowSize: self.windowManager.targetWindowSize, - maximumPacketSize: 1 << 24 + maximumPacketSize: Constants.maximumChannelPacketSize ) self.processOutboundMessage(.channelOpen(message), promise: nil) self.writePendingToMultiplexer() diff --git a/Sources/NIOSSH/Connection State Machine/Operations/AcceptsVersionMessages.swift b/Sources/NIOSSH/Connection State Machine/Operations/AcceptsVersionMessages.swift index 94bb7f2b..93144e0e 100644 --- a/Sources/NIOSSH/Connection State Machine/Operations/AcceptsVersionMessages.swift +++ b/Sources/NIOSSH/Connection State Machine/Operations/AcceptsVersionMessages.swift @@ -15,62 +15,12 @@ protocol AcceptsVersionMessages {} extension AcceptsVersionMessages { - func receiveVersionMessage(_ banner: String, role: SSHConnectionRole) throws { - try self.validateBanner(banner, role: role) - } - - // From RFC 4253: - // - // > Protocol Version Exchange - // > - // > When the connection has been established, both sides MUST send an - // > identification string. This identification string MUST be - // > SSH-protoversion-softwareversion SP comments CR LF - // > Since the protocol being defined in this set of documents is version - // > 2.0, the 'protoversion' MUST be "2.0". The 'comments' string is - // > OPTIONAL. If the 'comments' string is included, a 'space' character - // > (denoted above as SP, ASCII 32) MUST separate the 'softwareversion' - // > and 'comments' strings. The identification MUST be terminated by a - // > single Carriage Return (CR) and a single Line Feed (LF) character - // > (ASCII 13 and 10, respectively). Implementers who wish to maintain - // > compatibility with older, undocumented versions of this protocol may - // > want to process the identification string without expecting the - // > presence of the carriage return character for reasons described in - // > Section 5 of this document. The null character MUST NOT be sent. - // > The maximum length of the string is 255 characters, including the - // > Carriage Return and Line Feed. - // > - // > The part of the identification string preceding the Carriage Return - // > and Line Feed is used in the Diffie-Hellman key exchange (see Section - // > 8). - // > - // > The server MAY send other lines of data before sending the version - // > string. Each line SHOULD be terminated by a Carriage Return and Line - // > Feed. Such lines MUST NOT begin with "SSH-", and SHOULD be encoded - // > in ISO-10646 UTF-8 [RFC3629] (language is not specified). Clients - // > MUST be able to process such lines. Such lines MAY be silently - // > ignored, or MAY be displayed to the client user. If they are - // > displayed, control character filtering, as discussed in [SSH-ARCH], - // > SHOULD be used. The primary use of this feature is to allow TCP- - // > wrappers to display an error message before disconnecting - private func validateBanner(_ banner: String, role: SSHConnectionRole) throws { - switch role { - case .client: - // split by \n - let lineFeed = UInt8(ascii: "\n") - for line in banner.utf8.split(separator: lineFeed) { - if try self.validateVersion(line) { - return - } - } - throw NIOSSHError.protocolViolation(protocolName: "version exchange", violation: "version string not found") - case .server: - guard try self.validateVersion(Substring(banner).utf8) else { - throw NIOSSHError.protocolViolation( - protocolName: "version exchange", - violation: "version string not found" - ) - } + func receiveVersionMessage(_ banner: String) throws { + guard try self.validateVersion(Substring(banner).utf8) else { + throw NIOSSHError.protocolViolation( + protocolName: "version exchange", + violation: "version string not found" + ) } } diff --git a/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift b/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift index ea3462dc..726bfcdd 100644 --- a/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift +++ b/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift @@ -115,7 +115,7 @@ struct SSHConnectionStateMachine { switch message { case .version(let version): - try state.receiveVersionMessage(version, role: state.role) + try state.receiveVersionMessage(version) let newState = KeyExchangeState( sentVersionState: state, allocator: allocator, diff --git a/Sources/NIOSSH/Constants.swift b/Sources/NIOSSH/Constants.swift index 3540ecf2..9fc41fc9 100644 --- a/Sources/NIOSSH/Constants.swift +++ b/Sources/NIOSSH/Constants.swift @@ -15,6 +15,11 @@ public enum Constants: Sendable { static let version = "SSH-2.0-SwiftNIOSSH_1.0" + /// The maximum size, in bytes, of a channel data payload that we advertise we are willing to + /// receive (the "maximum packet size" of an SSH channel, RFC 4254 §5.1). + /// - Note: "This is a weirdly hard-coded choice." + static let maximumChannelPacketSize: UInt32 = 1 << 24 + public static let bundledTransportProtectionSchemes: [(NIOSSHTransportProtection & _NIOSSHSendableMetatype).Type] = [ AES256GCMOpenSSHTransportProtection.self, AES128GCMOpenSSHTransportProtection.self, diff --git a/Sources/NIOSSH/SSHPacketParser.swift b/Sources/NIOSSH/SSHPacketParser.swift index 314b2143..3e8c3810 100644 --- a/Sources/NIOSSH/SSHPacketParser.swift +++ b/Sources/NIOSSH/SSHPacketParser.swift @@ -23,11 +23,32 @@ struct SSHPacketParser { case encryptedWaitingForBytes(UInt32, NIOSSHTransportProtection) } + /// RFC 4253 §4.2 limits the SSH version string ("SSH-...") to 255 bytes, includes CR and LF. + static var maximumVersionStringLength: Int { 255 } + + /// This bounds each preamble line a client may receive before the version string, matching OpenSSH. + static var maximumBannerLineLength: Int { 8192 } + + /// The maximum number of preamble lines a client accepts before the version string, matching OpenSSH. + static var maximumPreambleLineCount: Int { 1024 } + + /// The maximum length of a cleartext (pre-encryption) packet, i.e. during version and key + /// exchange. The encrypted path negotiates its own (larger) packet bound. + static var maximumPlaintextPacketLength: UInt32 { 256 * 1024 } + + /// The maximum length of an encrypted packet, i.e. its `packet_length` field once key + /// exchange has completed. Includes headroom for the SSH framing and padding (RFC 4253 §6). + static var maximumEncryptedPacketLength: UInt32 { Constants.maximumChannelPacketSize + 1024 } + private let isServer: Bool private var buffer: ByteBuffer private var state: State private(set) var sequenceNumber: UInt32 + /// The number of preamble lines a client has discarded while waiting for the identification + /// string, bounded by `maximumPreambleLineCount`. + private var preambleLineCount: Int + /// Testing only: the number of bytes we can discard from this buffer. internal var _discardableBytes: Int { self.buffer.readerIndex @@ -38,6 +59,7 @@ struct SSHPacketParser { self.buffer = allocator.buffer(capacity: 0) self.state = .initialized self.sequenceNumber = 0 + self.preambleLineCount = 0 } mutating func append(bytes: inout ByteBuffer) { @@ -119,17 +141,57 @@ struct SSHPacketParser { } } + // From RFC 4253: + // + // > Protocol Version Exchange + // > + // > When the connection has been established, both sides MUST send an + // > identification string. This identification string MUST be + // > SSH-protoversion-softwareversion SP comments CR LF + // > Since the protocol being defined in this set of documents is version + // > 2.0, the 'protoversion' MUST be "2.0". The 'comments' string is + // > OPTIONAL. If the 'comments' string is included, a 'space' character + // > (denoted above as SP, ASCII 32) MUST separate the 'softwareversion' + // > and 'comments' strings. The identification MUST be terminated by a + // > single Carriage Return (CR) and a single Line Feed (LF) character + // > (ASCII 13 and 10, respectively). Implementers who wish to maintain + // > compatibility with older, undocumented versions of this protocol may + // > want to process the identification string without expecting the + // > presence of the carriage return character for reasons described in + // > Section 5 of this document. The null character MUST NOT be sent. + // > The maximum length of the string is 255 characters, including the + // > Carriage Return and Line Feed. + // > + // > The part of the identification string preceding the Carriage Return + // > and Line Feed is used in the Diffie-Hellman key exchange (see Section + // > 8). + // > + // > The server MAY send other lines of data before sending the version + // > string. Each line SHOULD be terminated by a Carriage Return and Line + // > Feed. Such lines MUST NOT begin with "SSH-", and SHOULD be encoded + // > in ISO-10646 UTF-8 [RFC3629] (language is not specified). Clients + // > MUST be able to process such lines. Such lines MAY be silently + // > ignored, or MAY be displayed to the client user. If they are + // > displayed, control character filtering, as discussed in [SSH-ARCH], + // > SHOULD be used. The primary use of this feature is to allow TCP- + // > wrappers to display an error message before disconnecting private mutating func readVersion() throws -> String? { let carriageReturn = UInt8(ascii: "\r") let lineFeed = UInt8(ascii: "\n") + let sshPrefix = "SSH-".utf8 // Per RFC 4253 §4.2: - // The server MAY send other lines of data before sending the version string. + // The server MAY send other lines of data before sending the version string. // This means that server does not expect any lines before version so we will return all data before first line feed if self.isServer { // Looking for a string ending with \r\n let slice = self.buffer.readableBytesView if let lfIndex = slice.firstIndex(of: lineFeed), lfIndex < slice.endIndex { + // This must be the version: check the length. The version itself will be validated later. + try self.enforceIdentificationAndBannerLineLimit( + length: slice.startIndex.distance(to: lfIndex) + 1, // add one for line feed + isVersion: true + ) // Guard the `-1` lookup: when the LF is the very first byte of // the slice (e.g., a client sends a bare LF as its first byte // after the TCP handshake), `lfIndex.advanced(by: -1)` would @@ -143,24 +205,76 @@ struct SSHPacketParser { return version } } else { - // Search for version line, which starts with "SSH-". Lines without this prefix may come before the version line. - var slice = self.buffer.readableBytesView - let startIndex = slice.startIndex - while let lfIndex = slice.firstIndex(of: lineFeed), lfIndex < slice.endIndex { - if slice.starts(with: "SSH-".utf8) { + // Search for version line, which starts with "SSH-". Lines without this + // prefix may come before the version line. Discard them to avoid accumulating state. + while true { + // Find the next line and check its length. + let slice = self.buffer.readableBytesView + guard let lfIndex = slice.firstIndex(of: lineFeed) else { + break + } + let lineLength = slice.startIndex.distance(to: lfIndex) + let isVersion = slice.starts(with: sshPrefix) + try self.enforceIdentificationAndBannerLineLimit( + length: lineLength + 1, // add one for line feed + isVersion: isVersion + ) + + // Per RFC other lines in the preamble are not allowed to start with "SSH-". + // This must be the version. + if isVersion { let versionEndIndex = slice[lfIndex.advanced(by: -1)] == carriageReturn ? lfIndex.advanced(by: -1) : lfIndex let version = String(decoding: slice[slice.startIndex.. Self.maximumPreambleLineCount { + throw NIOSSHError.protocolViolation( + protocolName: "version exchange", + violation: "received more than \(Self.maximumPreambleLineCount) lines before the version string" + ) + } + // This just advances the reader index. The calling function (`nextPacket`) will + // free up bytes with `reclaimBytes` when a threshold of read bytes is reached. + self.buffer.moveReaderIndex(forwardBy: lineLength + 1) } } + + // Ensure the buffered data has not grown past the limit, e.g., if the peer sends data without an LF. + let pending = self.buffer.readableBytesView + try self.enforceIdentificationAndBannerLineLimit( + length: pending.count, + isVersion: pending.starts(with: sshPrefix) + ) return nil } + /// Enforces the version-exchange line-length limits used by ``readVersion()``. Identification lines are + /// limited to ``maximumIdentificationStringLength``, others to ``maximumBannerLineLength``. + /// + /// - Throws: NIOSSHError.protocolViolation if the line crosses its length limit. + private func enforceIdentificationAndBannerLineLimit(length: Int, isVersion: Bool) throws { + if isVersion { + if length > Self.maximumVersionStringLength { + throw NIOSSHError.protocolViolation( + protocolName: "version exchange", + violation: "version line exceeded \(Self.maximumVersionStringLength) bytes" + ) + } + } else { + if length > Self.maximumBannerLineLength { + throw NIOSSHError.protocolViolation( + protocolName: "version exchange", + violation: "banner line exceeded \(Self.maximumBannerLineLength) bytes" + ) + } + } + } + private mutating func decryptLength(protection: NIOSSHTransportProtection) throws -> UInt32? { let blockSize = protection.cipherBlockSize guard self.buffer.readableBytes >= blockSize else { @@ -170,11 +284,29 @@ struct SSHPacketParser { try protection.decryptFirstBlock(&self.buffer) // This force unwrap is safe because we must have a block size, and a block size is always going to be more than 4 bytes. - return self.buffer.getInteger(at: self.buffer.readerIndex)! + UInt32(protection.macBytes) + let length = self.buffer.getInteger(at: self.buffer.readerIndex, as: UInt32.self)! + + // Reject an oversized encrypted packet before we commit to buffering toward it. + guard length <= Self.maximumEncryptedPacketLength else { + throw NIOSSHError.protocolViolation( + protocolName: "transport", + violation: "encrypted packet length \(length) exceeds \(Self.maximumEncryptedPacketLength) bytes" + ) + } + + return length + UInt32(protection.macBytes) } private mutating func parsePlaintext(length: UInt32) throws -> SSHMessage? { - try self.buffer.rewindReaderOnError { buffer in + // Reject an oversized cleartext packet before committing to buffering it. + guard length <= Self.maximumPlaintextPacketLength else { + throw NIOSSHError.protocolViolation( + protocolName: "transport", + violation: "cleartext packet length \(length) exceeds \(Self.maximumPlaintextPacketLength) bytes" + ) + } + + return try self.buffer.rewindReaderOnError { buffer in guard var buffer = buffer.readSlice(length: Int(length) + MemoryLayout.size) else { return nil } diff --git a/Tests/NIOSSHTests/SSHHandlerTests.swift b/Tests/NIOSSHTests/SSHHandlerTests.swift index dd211c46..26d19c6a 100644 --- a/Tests/NIOSSHTests/SSHHandlerTests.swift +++ b/Tests/NIOSSHTests/SSHHandlerTests.swift @@ -19,6 +19,19 @@ import XCTest @testable import NIOSSH +/// Records inbound errors and closes the connection. Used to observe parse errors that +/// `NIOSSHHandler` surfaces via `fireErrorCaught`. +private final class ErrorRecordingHandler: ChannelInboundHandler { + typealias InboundIn = Any + + private(set) var errors: [Error] = [] + + func errorCaught(context: ChannelHandlerContext, error: Error) { + self.errors.append(error) + context.close(promise: nil) + } +} + class SSHHandlerTests: XCTestCase { func testHandlerInitializationOnAdd() throws { let allocator = ByteBufferAllocator() @@ -60,4 +73,53 @@ class SSHHandlerTests: XCTestCase { .byteBuffer(allocator.buffer(string: Constants.version + "\r\n")) ) } + + func testPreAuthBannerReachesParserAcrossReadsAndIsCapped() throws { + // 32-byte Ed25519 raw host key (same bytes as FuzzResultTests). + let hostKeyBytes: [UInt8] = [ + 132, 233, 148, 139, 128, 175, 236, 108, 87, 48, 49, 251, 46, 42, 132, 84, + 255, 3, 160, 224, 186, 192, 170, 245, 194, 220, 192, 204, 81, 127, 55, 169, + ] + let handler = try NIOSSHHandler( + role: .server( + .init( + hostKeys: [.init(ed25519Key: .init(rawRepresentation: hostKeyBytes))], + userAuthDelegate: AcceptEverythingDelegate() + ) + ), + allocator: ByteBufferAllocator(), + inboundChildChannelInitializer: nil + ) + let recorder = ErrorRecordingHandler() + let channel = EmbeddedChannel() + try channel.pipeline.syncOperations.addHandlers([handler, recorder]) + _ = try channel.connect(to: .init(unixDomainSocketPath: "/fake")) + + // The server is now mid-handshake: it has emitted its own identification string, pre-auth. + XCTAssertNotNil(try channel.readOutbound(as: IOData.self)) + + let initialChunkSize = 128 + let chunkSize = 1024 * 10 + let chunkCount = 100 + + var firstChunk = channel.allocator.buffer(capacity: chunkSize) + firstChunk.writeString("SSH-") + firstChunk.writeBytes([UInt8](repeating: UInt8(ascii: "A"), count: initialChunkSize - 4)) + XCTAssertNoThrow(try channel.writeInbound(firstChunk)) + XCTAssertTrue(channel.isActive) + XCTAssertTrue(recorder.errors.isEmpty) + + // Keep feeding data until we hit an error without ever adding '\n'. + var next = channel.allocator.buffer(capacity: chunkSize) + next.writeBytes([UInt8](repeating: UInt8(ascii: "A"), count: chunkSize)) + for _ in 1.. 0 { + break + } + } + // Check that an error was propagated and the channel is no longer active. + XCTAssertFalse(recorder.errors.isEmpty) + XCTAssertFalse(channel.isActive) + } } diff --git a/Tests/NIOSSHTests/SSHPacketParserTests.swift b/Tests/NIOSSHTests/SSHPacketParserTests.swift index 7f645c6f..76c6a323 100644 --- a/Tests/NIOSSHTests/SSHPacketParserTests.swift +++ b/Tests/NIOSSHTests/SSHPacketParserTests.swift @@ -58,6 +58,323 @@ final class SSHPacketParserTests: XCTestCase { } } + func feedBannerString( + to parser: inout SSHPacketParser, + ofLength length: Int, + withSSHPrefix: Bool, + withCR: Bool, + lineConut: Int = 1 + ) { + + var buffer = ByteBufferAllocator().buffer(capacity: length) + + var middlePartLength = length - 1 // minus one for LF + if withSSHPrefix { + middlePartLength -= 4 // "SSH-" + buffer.writeString("SSH-") + } + if withCR { + middlePartLength -= 1 + } + + // Append the lines. + for _ in 0.. The maximum length of the string is 255 characters, including the + // > Carriage Return and Line Feed. + func testAcceptsUpTo255BytesOfIdentificationServer() throws { + var parser = SSHPacketParser(isServer: true, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength, + withSSHPrefix: true, + withCR: false + ) + + switch try parser.nextPacket() { + case .version(let string): + XCTAssertEqual(0, parser.sequenceNumber) + XCTAssert(string.starts(with: "SSH-")) + XCTAssertEqual(string.count, SSHPacketParser.maximumVersionStringLength - 1) // line feed + default: + XCTFail("Expecting .version") + } + } + + func testAcceptsUpTo255BytesOfIdentificationWithCRLFServer() throws { + var parser = SSHPacketParser(isServer: true, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength, + withSSHPrefix: true, + withCR: true + ) + + switch try parser.nextPacket() { + case .version(let string): + XCTAssertEqual(0, parser.sequenceNumber) + XCTAssert(string.starts(with: "SSH-")) + XCTAssertEqual(string.count, SSHPacketParser.maximumVersionStringLength - 2) // CR, LF + default: + XCTFail("Expecting .version") + } + } + + func testRejects256BytesOfIdentificationServer() throws { + var parser = SSHPacketParser(isServer: true, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength + 1, + withSSHPrefix: true, + withCR: false + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testRejects256BytesOfIdentificationWithCRLFServer() throws { + var parser = SSHPacketParser(isServer: true, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength + 1, + withSSHPrefix: true, + withCR: true + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testAcceptsUpTo255BytesOfIdentificationClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength, + withSSHPrefix: true, + withCR: false + ) + + switch try parser.nextPacket() { + case .version(let string): + XCTAssertEqual(0, parser.sequenceNumber) + XCTAssert(string.starts(with: "SSH-")) + XCTAssertEqual(string.count, SSHPacketParser.maximumVersionStringLength - 1) // line feed + default: + XCTFail("Expecting .version") + } + } + + func testAcceptsUpTo255BytesOfIdentificationWithCRLFClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength, + withSSHPrefix: true, + withCR: true + ) + + switch try parser.nextPacket() { + case .version(let string): + XCTAssertEqual(0, parser.sequenceNumber) + XCTAssert(string.starts(with: "SSH-")) + XCTAssertEqual(string.count, SSHPacketParser.maximumVersionStringLength - 2) // CR, LF + default: + XCTFail("Expecting .version") + } + } + + func testRejects256BytesOfIdentificationClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength + 1, + withSSHPrefix: true, + withCR: false + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testRejects256BytesOfIdentificationWithCRLFClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumVersionStringLength + 1, + withSSHPrefix: true, + withCR: true + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + // The RFC does not limit the preamble. To avoid peers never sending a '\n' we + // restrict the maximum non-identification preamble length. + func testAcceptsMaximumLengthPreambleClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumBannerLineLength, + withSSHPrefix: false, + withCR: false + ) + + XCTAssertNoThrow(try parser.nextPacket()) + } + + func testAcceptsMultilineMaximumLengthPreambleClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumBannerLineLength, + withSSHPrefix: false, + withCR: false, + lineConut: SSHPacketParser.maximumPreambleLineCount + ) + + XCTAssertNoThrow(try parser.nextPacket()) + } + + func testRejectsOverlongPreambleClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumBannerLineLength + 1, + withSSHPrefix: false, + withCR: false + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testRejectsTooManyLineMultilinePreambleClient() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + + feedBannerString( + to: &parser, + ofLength: SSHPacketParser.maximumBannerLineLength, + withSSHPrefix: false, + withCR: false, + lineConut: SSHPacketParser.maximumPreambleLineCount + 1 + ) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testPlaintextLengthRFCRequiredSize() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + self.feedVersion(to: &parser) + + // RFC 4253 §6.1 + // > All implementations MUST be able to process packets with an + // > uncompressed payload length of 32768 bytes or less and a total packet + // > size of 35000 bytes or less + var header = ByteBuffer.of(bytes: [0x00, 0x00, 0x88, 0xB8]) + parser.append(bytes: &header) + + XCTAssertNoThrow(try parser.nextPacket()) + } + + func testPlaintextLengthImplementationMax() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + self.feedVersion(to: &parser) + + let size = SSHPacketParser.maximumPlaintextPacketLength + let bytes = withUnsafeBytes(of: size.bigEndian) { + Array($0) + } + var header = ByteBuffer.of(bytes: bytes) + parser.append(bytes: &header) + + XCTAssertNoThrow(try parser.nextPacket()) + } + + func testPlaintextLengthImplementationMaxSurpassed() throws { + guard SSHPacketParser.maximumPlaintextPacketLength < UInt32.max else { + throw XCTSkip("Limit is already UInt32.max") + } + + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + self.feedVersion(to: &parser) + + let size = SSHPacketParser.maximumPlaintextPacketLength + 1 + let bytes = withUnsafeBytes(of: size.bigEndian) { + Array($0) + } + var header = ByteBuffer.of(bytes: bytes) + parser.append(bytes: &header) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + func testRejectsOversizedPlaintextLength() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + self.feedVersion(to: &parser) + + // 4-byte big-endian length field = 0x10000000 (~268 MB). No body bytes follow, + // so the test itself allocates nothing large. + var header = ByteBuffer.of(bytes: [0x10, 0x00, 0x00, 0x00]) + parser.append(bytes: &header) + + XCTAssertThrowsError(try parser.nextPacket()) + } + + // Check that the decrypted packet length (decryptLength) has an upper bound. + func testRejectsOversizedEncryptedLength() throws { + var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator()) + self.feedVersion(to: &parser) + + // addEncryption is allowed directly from the post-feedVersion .cleartextWaitingForLength state. + let inboundEncryptionKey = SymmetricKey(size: .bits128) + let outboundEncryptionKey = inboundEncryptionKey + let inboundMACKey = SymmetricKey(size: .bits128) + let outboundMACKey = inboundMACKey + let protection = TestTransportProtection( + initialKeys: .init( + initialInboundIV: [], + initialOutboundIV: [], + inboundEncryptionKey: inboundEncryptionKey, + outboundEncryptionKey: outboundEncryptionKey, + inboundMACKey: inboundMACKey, + outboundMACKey: outboundMACKey + ) + ) + parser.addEncryption(protection) + + // One 128-byte cipher block whose first 4 (big-endian) bytes decode to ~256 MB. + var plaintextBlock = ByteBufferAllocator().buffer(capacity: 128) + plaintextBlock.writeInteger(UInt32(0x1000_0000)) // packet length field + plaintextBlock.writeBytes([UInt8](repeating: 0, count: 124)) // padding + + // Encrypt with the SAME inbound key bytes the protection decrypts with (XOR cipher is symmetric). + let keyBuffer = inboundEncryptionKey.withUnsafeBytes { ByteBuffer(bytes: $0) } + var ciphertext = InsecureEncryptionAlgorithm.encrypt(key: keyBuffer, plaintext: plaintextBlock) + parser.append(bytes: &ciphertext) + + XCTAssertThrowsError(try parser.nextPacket()) + } + func testReadVersionWithExtraLinesOnClient() throws { var parser = SSHPacketParser(isServer: false, allocator: ByteBufferAllocator())