diff --git a/Sources/NIOSSH/Child Channels/SSHChannelMultiplexer.swift b/Sources/NIOSSH/Child Channels/SSHChannelMultiplexer.swift index 1638460..f07e99f 100644 --- a/Sources/NIOSSH/Child Channels/SSHChannelMultiplexer.swift +++ b/Sources/NIOSSH/Child Channels/SSHChannelMultiplexer.swift @@ -34,7 +34,9 @@ final class SSHChannelMultiplexer { /// Whether new channels are allowed. Set to `false` once the parent channel is shut down at the TCP level. private var canCreateNewChannels: Bool - init(delegate: SSHMultiplexerDelegate, allocator: ByteBufferAllocator, childChannelInitializer: SSHChildChannel.Initializer?) { + private let maximumPacketSize: Int + + init(delegate: SSHMultiplexerDelegate, allocator: ByteBufferAllocator, childChannelInitializer: SSHChildChannel.Initializer?, maximumPacketSize: Int = 1 << 17) { self.channels = [:] self.channels.reserveCapacity(8) self.erroredChannels = [] @@ -43,6 +45,7 @@ final class SSHChannelMultiplexer { self.allocator = allocator self.childChannelInitializer = childChannelInitializer self.canCreateNewChannels = true + self.maximumPacketSize = maximumPacketSize } // Time to clean up. We drop references to things that may be keeping us alive. @@ -190,14 +193,16 @@ extension SSHChannelMultiplexer { self.nextChannelID = 0 } - // TODO: Make the window management parameters configurable + // `maximumPacketSize` can be safely cast, because overrides of the default by implementations are expected to have sensible values + // These are also asserted in SSHPackageParser.init let channel = SSHChildChannel(allocator: self.allocator, parent: parentChannel, multiplexer: self, initializer: initializer, localChannelID: channelID, - targetWindowSize: 1 << 24, - initialOutboundWindowSize: 0) // The initial outbound window size is presumed to be 0 until we're told otherwise. + targetWindowSize: Int32(self.maximumPacketSize), + initialOutboundWindowSize: 0, + maximumPacketSize: self.maximumPacketSize) // The initial outbound window size is presumed to be 0 until we're told otherwise. self.channels[channelID] = channel return channel diff --git a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift index 32d2fec..4864f5c 100644 --- a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift +++ b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift @@ -98,6 +98,8 @@ final class SSHChildChannel { private var type: SSHChannelType? + private let maximumPacketSize: Int + /// A promise from the user that will be fired when the channel goes active. private var userActivatePromise: EventLoopPromise? @@ -107,14 +109,16 @@ final class SSHChildChannel { initializer: Initializer?, localChannelID: UInt32, targetWindowSize: Int32, - initialOutboundWindowSize: UInt32) { + initialOutboundWindowSize: UInt32, + maximumPacketSize: Int) { self.init(allocator: allocator, parent: parent, multiplexer: multiplexer, initializer: initializer, initialState: .init(localChannelID: localChannelID), targetWindowSize: targetWindowSize, - initialOutboundWindowSize: initialOutboundWindowSize) + initialOutboundWindowSize: initialOutboundWindowSize, + maximumPacketSize: maximumPacketSize) } private init(allocator: ByteBufferAllocator, @@ -123,7 +127,8 @@ final class SSHChildChannel { initializer: Initializer?, initialState: ChildChannelStateMachine, targetWindowSize: Int32, - initialOutboundWindowSize: UInt32) { + initialOutboundWindowSize: UInt32, + maximumPacketSize: Int) { self.allocator = allocator self.closePromise = parent.eventLoop.makePromise() self.parent = parent @@ -136,6 +141,7 @@ final class SSHChildChannel { self.state = initialState self.writabilityManager = ChildChannelWritabilityManager(initialWindowSize: initialOutboundWindowSize, parentIsWritable: parent.isWritable) + self.maximumPacketSize = maximumPacketSize self.peerMaxMessageSize = 0 // To begin with we initialize autoRead and halfClosure to false, but we are going to fetch it from our parent before we @@ -470,7 +476,7 @@ extension SSHChildChannel: Channel, ChannelCore { let message = SSHMessage.ChannelOpenConfirmationMessage(recipientChannel: self.state.remoteChannelIdentifier!, senderChannel: self.state.localChannelIdentifier, initialWindowSize: self.windowManager.targetWindowSize, - maximumPacketSize: 1 << 24) // This is a weirdly hard-coded choice. + maximumPacketSize: UInt32(self.maximumPacketSize)) self.processOutboundMessage(.channelOpenConfirmation(message), promise: nil) self.writePendingToMultiplexer() } else if !self.state.isClosed { @@ -478,7 +484,7 @@ extension SSHChildChannel: Channel, ChannelCore { let message = SSHMessage.ChannelOpenMessage(type: .init(self.type!), senderChannel: self.state.localChannelIdentifier, initialWindowSize: self.windowManager.targetWindowSize, - maximumPacketSize: 1 << 24) + maximumPacketSize: UInt32(self.maximumPacketSize)) self.processOutboundMessage(.channelOpen(message), promise: nil) self.writePendingToMultiplexer() } else { diff --git a/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift b/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift index f40925b..71a560b 100644 --- a/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift +++ b/Sources/NIOSSH/Connection State Machine/SSHConnectionStateMachine.swift @@ -691,7 +691,7 @@ struct SSHConnectionStateMachine { switch message { case .version: try state.serializer.serialize(message: message, to: &buffer) - self.state = .sentVersion(.init(idleState: state, allocator: allocator)) + self.state = .sentVersion(.init(idleState: state, allocator: allocator, maximumPacketSize: state.role.maximumPacketSize)) case .disconnect: try state.serializer.serialize(message: message, to: &buffer) self.state = .sentDisconnect(state.role) diff --git a/Sources/NIOSSH/Connection State Machine/States/SentVersionState.swift b/Sources/NIOSSH/Connection State Machine/States/SentVersionState.swift index a7a83fb..8257aa7 100644 --- a/Sources/NIOSSH/Connection State Machine/States/SentVersionState.swift +++ b/Sources/NIOSSH/Connection State Machine/States/SentVersionState.swift @@ -30,12 +30,12 @@ extension SSHConnectionStateMachine { private let allocator: ByteBufferAllocator - init(idleState state: IdleState, allocator: ByteBufferAllocator) { + init(idleState state: IdleState, allocator: ByteBufferAllocator, maximumPacketSize: Int) { self.role = state.role self.serializer = state.serializer self.protectionSchemes = state.protectionSchemes - self.parser = SSHPacketParser(allocator: allocator) + self.parser = SSHPacketParser(allocator: allocator, maximumPacketSize: maximumPacketSize) self.allocator = allocator } } diff --git a/Sources/NIOSSH/NIOSSHError.swift b/Sources/NIOSSH/NIOSSHError.swift index 9090dd2..9d1085c 100644 --- a/Sources/NIOSSH/NIOSSHError.swift +++ b/Sources/NIOSSH/NIOSSHError.swift @@ -44,6 +44,8 @@ extension NIOSSHError { internal static let invalidNonceLength = NIOSSHError(type: .invalidNonceLength, diagnostics: nil) + internal static let excessiveVersionLength = NIOSSHError(type: .excessiveVersionLength, diagnostics: nil) + internal static let invalidEncryptedPacketLength = NIOSSHError(type: .invalidEncryptedPacketLength, diagnostics: nil) internal static let invalidDecryptedPlaintextLength = NIOSSHError(type: .invalidDecryptedPlaintextLength, diagnostics: nil) @@ -179,6 +181,7 @@ extension NIOSSHError { case invalidHostKeyForKeyExchange case invalidOpenSSHPublicKey case invalidCertificate + case excessiveVersionLength } private var base: Base @@ -196,6 +199,9 @@ extension NIOSSHError { /// The length of the nonce provided to a cipher is invalid for that cipher. public static let invalidNonceLength: ErrorType = .init(.invalidNonceLength) + /// The version length sent by a client was excessively large. + public static let excessiveVersionLength: ErrorType = .init(.excessiveVersionLength) + /// The encrypted packet received has an invalid length for the negotiated encyption scheme public static let invalidEncryptedPacketLength: ErrorType = .init(.invalidEncryptedPacketLength) diff --git a/Sources/NIOSSH/NIOSSHHandler.swift b/Sources/NIOSSH/NIOSSHHandler.swift index ca00b15..6854fb9 100644 --- a/Sources/NIOSSH/NIOSSHHandler.swift +++ b/Sources/NIOSSH/NIOSSHHandler.swift @@ -64,7 +64,7 @@ public final class NIOSSHHandler { self.pendingChannelInitializations = CircularBuffer(initialCapacity: 4) self.pendingGlobalRequests = CircularBuffer(initialCapacity: 4) self.pendingGlobalRequestResponses = CircularBuffer(initialCapacity: 4) - self.multiplexer = SSHChannelMultiplexer(delegate: self, allocator: allocator, childChannelInitializer: inboundChildChannelInitializer) + self.multiplexer = SSHChannelMultiplexer(delegate: self, allocator: allocator, childChannelInitializer: inboundChildChannelInitializer, maximumPacketSize: role.maximumPacketSize) } } diff --git a/Sources/NIOSSH/Role.swift b/Sources/NIOSSH/Role.swift index 2af2818..4c0424b 100644 --- a/Sources/NIOSSH/Role.swift +++ b/Sources/NIOSSH/Role.swift @@ -34,4 +34,13 @@ public enum SSHConnectionRole { return true } } + + internal var maximumPacketSize: Int { + switch self { + case .client(let client): + return client.maximumPacketSize + case .server(let server): + return server.maximumPacketSize + } + } } diff --git a/Sources/NIOSSH/SSHClientConfiguration.swift b/Sources/NIOSSH/SSHClientConfiguration.swift index 7ecbbc4..b91d765 100644 --- a/Sources/NIOSSH/SSHClientConfiguration.swift +++ b/Sources/NIOSSH/SSHClientConfiguration.swift @@ -23,6 +23,9 @@ public struct SSHClientConfiguration { /// The global request delegate to be used with this client. public var globalRequestDelegate: GlobalRequestDelegate + /// The maximum packet size that this NIOSSH client will accept + public var maximumPacketSize = SSHPacketParser.defaultMaximumPacketSize + public init(userAuthDelegate: NIOSSHClientUserAuthenticationDelegate, serverAuthDelegate: NIOSSHClientServerAuthenticationDelegate, globalRequestDelegate: GlobalRequestDelegate? = nil) { diff --git a/Sources/NIOSSH/SSHPacketParser.swift b/Sources/NIOSSH/SSHPacketParser.swift index 68f3727..c5a1b6c 100644 --- a/Sources/NIOSSH/SSHPacketParser.swift +++ b/Sources/NIOSSH/SSHPacketParser.swift @@ -25,15 +25,22 @@ struct SSHPacketParser { private var buffer: ByteBuffer private var state: State + private let maximumPacketSize: Int + internal static let defaultMaximumPacketSize = 1 << 17 /// Testing only: the number of bytes we can discard from this buffer. internal var _discardableBytes: Int { self.buffer.readerIndex } - init(allocator: ByteBufferAllocator) { + init(allocator: ByteBufferAllocator, maximumPacketSize: Int = defaultMaximumPacketSize) { + // Assert that users don't provide a packet size lower than allowed by spec + precondition(maximumPacketSize >= 32768, "Maximum Packet Size is below minimum requirement as specified by RFC 4254") + precondition(maximumPacketSize <= (1 << 24), "Maximum Packet Size is set abnormally high") + self.buffer = allocator.buffer(capacity: 0) self.state = .initialized + self.maximumPacketSize = maximumPacketSize } mutating func append(bytes: inout ByteBuffer) { @@ -71,6 +78,10 @@ struct SSHPacketParser { return nil case .cleartextWaitingForLength: if let length = self.buffer.getInteger(at: self.buffer.readerIndex, as: UInt32.self) { + if length >= self.maximumPacketSize { + throw NIOSSHError.invalidEncryptedPacketLength + } + if let message = try self.parsePlaintext(length: length) { self.state = .cleartextWaitingForLength return message @@ -111,15 +122,30 @@ struct SSHPacketParser { } } + internal static let maximumAllowedVersionSize = 4096 private mutating func readVersion() throws -> String? { // Looking for a string ending with \r\n let slice = self.buffer.readableBytesView - if let cr = slice.firstIndex(of: 13), cr.advanced(by: 1) < slice.endIndex, slice[cr.advanced(by: 1)] == 10 { - let version = String(decoding: slice[slice.startIndex ..< cr], as: UTF8.self) - // read \r\n - self.buffer.moveReaderIndex(forwardBy: slice.startIndex.distance(to: cr).advanced(by: 2)) - return version + + // Prevent the consumed bytes for a version from exceeding the defined maximum allowed size + // In practice, if SSH version packets come anywhere near this it's already likely an attack + // More data cannot be blindly regarded as malicious though, since this might contain multiple packets + let maxIndex = slice.index(slice.startIndex, offsetBy: min(slice.count, Self.maximumAllowedVersionSize)) + + for index in slice.startIndex ..< slice.endIndex { + if index > maxIndex { + // Does not account for `CRLF` + throw NIOSSHError.excessiveVersionLength + } + + if slice[index] == 13, index.advanced(by: 1) < slice.endIndex, slice[index.advanced(by: 1)] == 10 { + let version = String(decoding: slice[slice.startIndex ..< index], as: UTF8.self) + // read \r\n + self.buffer.moveReaderIndex(forwardBy: slice.startIndex.distance(to: index).advanced(by: 2)) + return version + } } + return nil } @@ -132,7 +158,14 @@ 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 packetLength = self.buffer.getInteger(at: self.buffer.readerIndex, as: UInt32.self)! + let decryptedLength = packetLength + UInt32(protection.macBytes) + + if decryptedLength >= self.maximumPacketSize { + throw NIOSSHError.invalidEncryptedPacketLength + } + + return decryptedLength } private mutating func parsePlaintext(length: UInt32) throws -> SSHMessage? { diff --git a/Sources/NIOSSH/SSHServerConfiguration.swift b/Sources/NIOSSH/SSHServerConfiguration.swift index fcd8ed0..f33af07 100644 --- a/Sources/NIOSSH/SSHServerConfiguration.swift +++ b/Sources/NIOSSH/SSHServerConfiguration.swift @@ -23,6 +23,9 @@ public struct SSHServerConfiguration { /// The host keys for this server. public var hostKeys: [NIOSSHPrivateKey] + /// The maximum packet size that this NIOSSH server will accept + public var maximumPacketSize = SSHPacketParser.defaultMaximumPacketSize + /// The ssh banner to display to clients upon authentication public var banner: UserAuthBanner? diff --git a/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift b/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift index 0f816c2..d87c8a3 100644 --- a/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift +++ b/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift @@ -193,14 +193,14 @@ final class ChildChannelMultiplexerTests: XCTestCase { } } - private func openRequest(channelID: UInt32, initialWindowSize: UInt32 = 1 << 24, maxPacketSize: UInt32 = 1 << 24) -> SSHMessage { + private func openRequest(channelID: UInt32, initialWindowSize: UInt32 = UInt32(SSHPacketParser.defaultMaximumPacketSize), maxPacketSize: UInt32 = UInt32(SSHPacketParser.defaultMaximumPacketSize)) -> SSHMessage { .channelOpen(.init(type: .session, senderChannel: channelID, initialWindowSize: initialWindowSize, maximumPacketSize: maxPacketSize)) } - private func openConfirmation(originalChannelID: UInt32, peerChannelID: UInt32, initialWindowSize: UInt32 = 1 << 24, maxPacketSize: UInt32 = 1 << 24) -> SSHMessage { + private func openConfirmation(originalChannelID: UInt32, peerChannelID: UInt32, initialWindowSize: UInt32 = UInt32(SSHPacketParser.defaultMaximumPacketSize), maxPacketSize: UInt32 = UInt32(SSHPacketParser.defaultMaximumPacketSize)) -> SSHMessage { .channelOpenConfirmation(.init(recipientChannel: originalChannelID, senderChannel: peerChannelID, initialWindowSize: initialWindowSize, @@ -231,8 +231,8 @@ final class ChildChannelMultiplexerTests: XCTestCase { func assertChannelOpen(_ message: SSHMessage?) -> UInt32? { switch message { case .some(.channelOpen(let message)): - XCTAssertEqual(message.maximumPacketSize, 1 << 24) - XCTAssertEqual(message.initialWindowSize, 1 << 24) + XCTAssertEqual(message.maximumPacketSize, UInt32(SSHPacketParser.defaultMaximumPacketSize)) + XCTAssertEqual(message.initialWindowSize, UInt32(SSHPacketParser.defaultMaximumPacketSize)) return message.senderChannel case let fallback: @@ -246,8 +246,8 @@ final class ChildChannelMultiplexerTests: XCTestCase { switch message { case .some(.channelOpenConfirmation(let message)): XCTAssertEqual(message.recipientChannel, recipientChannel) - XCTAssertEqual(message.maximumPacketSize, 1 << 24) - XCTAssertEqual(message.initialWindowSize, 1 << 24) + XCTAssertEqual(message.maximumPacketSize, UInt32(SSHPacketParser.defaultMaximumPacketSize)) + XCTAssertEqual(message.initialWindowSize, UInt32(SSHPacketParser.defaultMaximumPacketSize)) return message.senderChannel case let fallback: @@ -1191,13 +1191,13 @@ final class ChildChannelMultiplexerTests: XCTestCase { let channelID = self.assertChannelOpen(harness.flushedMessages.first) XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.openConfirmation(originalChannelID: channelID!, peerChannelID: 1))) - // The default window size is 1<<24 bytes. Sadly, we need a buffer that size. + // Make a buffer the size of the default window size let buffer = ByteBuffer.bigBuffer // We're going to write one byte short. XCTAssertEqual(harness.flushedMessages.count, 1) XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.data(peerChannelID: channelID!, - data: buffer.getSlice(at: buffer.readerIndex, length: (1 << 23) - 1)!))) + data: buffer.getSlice(at: buffer.readerIndex, length: (SSHPacketParser.defaultMaximumPacketSize >> 1) - 1)!))) // Auto read is off, so nothing happens. XCTAssertEqual(harness.flushedMessages.count, 1) @@ -1214,17 +1214,17 @@ final class ChildChannelMultiplexerTests: XCTestCase { // Now issue a read. This triggers an outbound message. channel.read() XCTAssertEqual(harness.flushedMessages.count, 2) - self.assertWindowAdjust(harness.flushedMessages.last, recipientChannel: 1, delta: 1 << 23) + self.assertWindowAdjust(harness.flushedMessages.last, recipientChannel: 1, delta: UInt32(SSHPacketParser.defaultMaximumPacketSize >> 1)) // Now issue a really big read. Again, there's no autoread, so this does nothing. XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.data(peerChannelID: channelID!, - data: buffer.getSlice(at: buffer.readerIndex, length: 1 << 24)!))) + data: buffer.getSlice(at: buffer.readerIndex, length: SSHPacketParser.defaultMaximumPacketSize)!))) XCTAssertEqual(harness.flushedMessages.count, 2) // Issue the read. A new outbound message with a bigger window increment. channel.read() XCTAssertEqual(harness.flushedMessages.count, 3) - self.assertWindowAdjust(harness.flushedMessages.last, recipientChannel: 1, delta: 1 << 24) + self.assertWindowAdjust(harness.flushedMessages.last, recipientChannel: 1, delta: UInt32(SSHPacketParser.defaultMaximumPacketSize)) // Finally, issue an excessively large read. This should cause an error. XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.data(peerChannelID: channelID!, data: buffer))) @@ -1253,13 +1253,13 @@ final class ChildChannelMultiplexerTests: XCTestCase { let channelID = self.assertChannelOpen(harness.flushedMessages.first) XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.openConfirmation(originalChannelID: channelID!, peerChannelID: 1))) - // The default window size is 1<<24 bytes. Sadly, we need a buffer that size. + // Make a buffer the size of the default window size let buffer = ByteBuffer.bigBuffer // We're going to write the whole window. XCTAssertEqual(harness.flushedMessages.count, 1) XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.data(peerChannelID: channelID!, - data: buffer.getSlice(at: buffer.readerIndex, length: 1 << 24)!))) + data: buffer.getSlice(at: buffer.readerIndex, length: SSHPacketParser.defaultMaximumPacketSize)!))) // Auto read is off, so nothing happens. XCTAssertEqual(harness.flushedMessages.count, 1) @@ -1561,7 +1561,7 @@ final class ChildChannelMultiplexerTests: XCTestCase { ] for (channelID, channelType) in channelTypes.enumerated() { - let message = SSHMessage.channelOpen(.init(type: .init(channelType), senderChannel: UInt32(channelID), initialWindowSize: 1 << 24, maximumPacketSize: 1 << 24)) + let message = SSHMessage.channelOpen(.init(type: .init(channelType), senderChannel: UInt32(channelID), initialWindowSize: UInt32(SSHPacketParser.defaultMaximumPacketSize), maximumPacketSize: UInt32(SSHPacketParser.defaultMaximumPacketSize))) XCTAssertNoThrow(try harness.multiplexer.receiveMessage(message)) } @@ -1757,10 +1757,10 @@ final class ChildChannelMultiplexerTests: XCTestCase { } extension ByteBuffer { - /// A buffer `(1 << 24) + 1` bytes large. + /// A buffer `(SSHPacketParser.defaultMaximumPacketSize) + 1` bytes large. fileprivate static let bigBuffer: ByteBuffer = { - // The default window size is 1<<24 bytes. Sadly, we need a buffer that size. + // Make a buffer the size of the default window size // We store it in a static so that we don't have to re-create it for every test. - ByteBuffer(repeating: 0, count: (1 << 24) + 1) + ByteBuffer(repeating: 0, count: SSHPacketParser.defaultMaximumPacketSize + 1) }() } diff --git a/Tests/NIOSSHTests/EndToEndTests.swift b/Tests/NIOSSHTests/EndToEndTests.swift index e921eb8..8febdfa 100644 --- a/Tests/NIOSSHTests/EndToEndTests.swift +++ b/Tests/NIOSSHTests/EndToEndTests.swift @@ -79,11 +79,20 @@ class BackToBackEmbeddedChannel { try self.server.connect(to: .init(unixDomainSocketPath: "/fake")).wait() } - func configureWithHarness(_ harness: TestHarness) throws { - let clientHandler = NIOSSHHandler(role: .client(.init(userAuthDelegate: harness.clientAuthDelegate, serverAuthDelegate: harness.clientServerAuthDelegate, globalRequestDelegate: harness.clientGlobalRequestDelegate)), + func configureWithHarness(_ harness: TestHarness, maximumPacketSize: Int? = nil) throws { + var clientConfiguration = SSHClientConfiguration(userAuthDelegate: harness.clientAuthDelegate, serverAuthDelegate: harness.clientServerAuthDelegate, globalRequestDelegate: harness.clientGlobalRequestDelegate) + + var serverConfiguration = SSHServerConfiguration(hostKeys: harness.serverHostKeys, userAuthDelegate: harness.serverAuthDelegate, globalRequestDelegate: harness.serverGlobalRequestDelegate, banner: harness.serverAuthBanner) + + if let maximumPacketSize = maximumPacketSize { + clientConfiguration.maximumPacketSize = maximumPacketSize + serverConfiguration.maximumPacketSize = maximumPacketSize + } + + let clientHandler = NIOSSHHandler(role: .client(clientConfiguration), allocator: self.client.allocator, inboundChildChannelInitializer: nil) - let serverHandler = NIOSSHHandler(role: .server(.init(hostKeys: harness.serverHostKeys, userAuthDelegate: harness.serverAuthDelegate, globalRequestDelegate: harness.serverGlobalRequestDelegate, banner: harness.serverAuthBanner)), + let serverHandler = NIOSSHHandler(role: .server(serverConfiguration), allocator: self.server.allocator) { channel, _ in self.activeServerChannels.append(channel) channel.closeFuture.whenComplete { _ in self.activeServerChannels.removeAll(where: { $0 === channel }) } @@ -130,6 +139,8 @@ struct TestHarness { var serverHostKeys: [NIOSSHPrivateKey] = [.init(ed25519Key: .init())] + var maximumPacketSize: Int? + var serverAuthBanner: SSHServerConfiguration.UserAuthBanner? } @@ -243,6 +254,28 @@ class EndToEndTests: XCTestCase { helper(ChannelFailureEvent()) } + /// This test validates that all the channel requests roiund-trip appropriately. + func testChannelRejectsHugePacketsRequests() throws { + XCTAssertNoThrow(try self.channel.configureWithHarness(TestHarness(), maximumPacketSize: 32768)) + XCTAssertNoThrow(try self.channel.activate()) + XCTAssertNoThrow(try self.channel.interactInMemory()) + + // Create a channel. + let clientChannel = try self.channel.createNewChannel() + XCTAssertNoThrow(try self.channel.interactInMemory()) + guard let serverChannel = self.channel.activeServerChannels.first else { + XCTFail("Server channel not created") + return + } + + let userEventRecorder = UserEventExpecter() + XCTAssertNoThrow(try serverChannel.pipeline.addHandler(userEventRecorder).wait()) + + let hugeCommand = String(repeating: "a", count: 32768) + try clientChannel.triggerUserOutboundEvent(SSHChannelRequestEvent.ExecRequest(command: hugeCommand, wantReply: true)).wait() + XCTAssertThrowsError(try self.channel.interactInMemory()) + } + func testGlobalRequestWithDefaultDelegate() throws { XCTAssertNoThrow(try self.channel.configureWithHarness(TestHarness())) XCTAssertNoThrow(try self.channel.activate()) diff --git a/Tests/NIOSSHTests/SSHPacketParserTests.swift b/Tests/NIOSSHTests/SSHPacketParserTests.swift index 0a727dd..9e6db8e 100644 --- a/Tests/NIOSSHTests/SSHPacketParserTests.swift +++ b/Tests/NIOSSHTests/SSHPacketParserTests.swift @@ -142,6 +142,15 @@ final class SSHPacketParserTests: XCTestCase { // Now we should have cleared up. XCTAssertEqual(parser._discardableBytes, 0) } + + func testMaximumPacketSizeInVersion() throws { + var parser = SSHPacketParser(allocator: ByteBufferAllocator(), maximumPacketSize: 1 << 15) + let longVersionString = String(repeating: "z", count: SSHPacketParser.maximumAllowedVersionSize + 256) + var version = ByteBuffer.of(string: longVersionString + "\r\n") + parser.append(bytes: &version) + + XCTAssertThrowsError(try parser.nextPacket()) + } } extension ByteBuffer {