From 3798fe5f1564f27461390b4f6163f6ddfb21fd2d Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Tue, 10 Oct 2023 13:00:57 +0100 Subject: [PATCH] Merge pull request from GHSA-qppj-fm5r-hxr3 * Limit rate of permitted RST frames Motivation: Large number of stream reset frames may be used as a DoS (Denial of Service) vector in an attempt to overload the CPU of the handling server. Modifications: Introduce an additional DoS heuristic which evaluates the rate of incoming stream reset frames. If the rate exceeds that which is permitted then the connection is closed and a `GOAWAY` issued. The allowed rate is configurable but defaults to 200 resets within 30 seconds. This should be acceptable for most applications. Result: Excessive reset frames result in the connection being closed. * review comments * further review comments * add integration test --- Sources/NIOHTTP2/DOSHeuristics.swift | 86 +++++++++++++- Sources/NIOHTTP2/HTTP2ChannelHandler.swift | 65 ++++++++-- Sources/NIOHTTP2/HTTP2Error.swift | 26 ++++ Tests/NIOHTTP2Tests/DOSHeuristicsTests.swift | 111 ++++++++++++++++++ ...eClientServerFramePayloadStreamTests.swift | 34 ++++++ 5 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 Tests/NIOHTTP2Tests/DOSHeuristicsTests.swift diff --git a/Sources/NIOHTTP2/DOSHeuristics.swift b/Sources/NIOHTTP2/DOSHeuristics.swift index e083444e..7e5f3ae2 100644 --- a/Sources/NIOHTTP2/DOSHeuristics.swift +++ b/Sources/NIOHTTP2/DOSHeuristics.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2019 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2019-2023 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -12,9 +12,11 @@ // //===----------------------------------------------------------------------===// +import DequeModule +import NIOCore /// Implements some simple denial of service heuristics on inbound frames. -struct DOSHeuristics { +struct DOSHeuristics { /// The number of "empty" (zero bytes of useful payload) DATA frames we've received since the /// last useful frame. /// @@ -26,11 +28,14 @@ struct DOSHeuristics { /// The maximum number of "empty" data frames we're willing to tolerate. private let maximumSequentialEmptyDataFrames: Int - internal init(maximumSequentialEmptyDataFrames: Int) { + private var resetFrameRateControlStateMachine: HTTP2ResetFrameRateControlStateMachine + + internal init(maximumSequentialEmptyDataFrames: Int, maximumResetFrameCount: Int, resetFrameCounterWindow: TimeAmount, clock: DeadlineClock = RealNIODeadlineClock()) { precondition(maximumSequentialEmptyDataFrames >= 0, "maximum sequential empty data frames must be positive, got \(maximumSequentialEmptyDataFrames)") self.maximumSequentialEmptyDataFrames = maximumSequentialEmptyDataFrames self.receivedEmptyDataFrames = 0 + self.resetFrameRateControlStateMachine = .init(countThreshold: maximumResetFrameCount, timeWindow: resetFrameCounterWindow, clock: clock) } } @@ -48,7 +53,15 @@ extension DOSHeuristics { } case .headers: self.receivedEmptyDataFrames = 0 - case .alternativeService, .goAway, .origin, .ping, .priority, .pushPromise, .rstStream, .settings, .windowUpdate: + case .rstStream: + switch self.resetFrameRateControlStateMachine.resetReceived() { + case .rateTooHigh: + throw NIOHTTP2Errors.excessiveRSTFrames() + case .noneReceived, .ratePermitted: + // no risk + () + } + case .alternativeService, .goAway, .origin, .ping, .priority, .pushPromise, .settings, .windowUpdate: // Currently we don't assess these for DoS risk. () } @@ -58,3 +71,68 @@ extension DOSHeuristics { } } } + +extension DOSHeuristics { + // protect against excessive numbers of stream RST frames being issued + struct HTTP2ResetFrameRateControlStateMachine { + + enum ResetFrameRateControlState: Hashable { + case noneReceived + case ratePermitted + case rateTooHigh + } + + private let countThreshold: Int + private let timeWindow: TimeAmount + private let clock: DeadlineClock + + private var resetTimestamps: Deque + private var _state: ResetFrameRateControlState = .noneReceived + + init(countThreshold: Int, timeWindow: TimeAmount, clock: DeadlineClock = RealNIODeadlineClock()) { + self.countThreshold = countThreshold + self.timeWindow = timeWindow + self.clock = clock + + self.resetTimestamps = .init(minimumCapacity: self.countThreshold) + } + + mutating func resetReceived() -> ResetFrameRateControlState { + self.garbageCollect() + self.resetTimestamps.append(self.clock.now()) + self.evaluateState() + return self._state + } + + private mutating func garbageCollect() { + let now = self.clock.now() + while let first = self.resetTimestamps.first, now - first > self.timeWindow { + _ = self.resetTimestamps.popFirst() + } + } + + private mutating func evaluateState() { + switch self._state { + case .noneReceived: + self._state = .ratePermitted + case .ratePermitted: + if self.resetTimestamps.count > self.countThreshold { + self._state = .rateTooHigh + } + case .rateTooHigh: + break // no-op, there is no way to de-escalate from an excessive rate + } + } + } +} + +// Simple mockable clock protocol +protocol NIODeadlineClock { + func now() -> NIODeadline +} + +struct RealNIODeadlineClock: NIODeadlineClock { + func now() -> NIODeadline { + NIODeadline.now() + } +} diff --git a/Sources/NIOHTTP2/HTTP2ChannelHandler.swift b/Sources/NIOHTTP2/HTTP2ChannelHandler.swift index bf34f8cd..659f1e66 100644 --- a/Sources/NIOHTTP2/HTTP2ChannelHandler.swift +++ b/Sources/NIOHTTP2/HTTP2ChannelHandler.swift @@ -73,7 +73,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { private var wroteFrame: Bool = false /// This object deploys heuristics to attempt to detect denial of service attacks. - private var denialOfServiceValidator: DOSHeuristics + private var denialOfServiceValidator: DOSHeuristics /// The mode this handler is operating in. private let mode: ParserMode @@ -209,7 +209,9 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { headerBlockValidation: headerBlockValidation, contentLengthValidation: contentLengthValidation, maximumSequentialEmptyDataFrames: 1, - maximumBufferedControlFrames: 10000) + maximumBufferedControlFrames: 10000, + maximumResetFrameCount: 200, + resetFrameCounterWindow: .seconds(30)) } /// Constructs a ``NIOHTTP2Handler``. @@ -236,23 +238,47 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { headerBlockValidation: headerBlockValidation, contentLengthValidation: contentLengthValidation, maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames, - maximumBufferedControlFrames: maximumBufferedControlFrames) + maximumBufferedControlFrames: maximumBufferedControlFrames, + maximumResetFrameCount: 200, + resetFrameCounterWindow: .seconds(30)) } + /// Constructs a ``NIOHTTP2Handler``. + /// + /// - Parameters: + /// - mode: The mode for this handler, client or server. + /// - connectionConfiguration: The settings that will be used when establishing the connection. + /// - streamConfiguration: The settings that will be used when establishing new streams. + public convenience init(mode: ParserMode, + connectionConfiguration: ConnectionConfiguration = .init(), + streamConfiguration: StreamConfiguration = .init()) { + self.init(mode: mode, + eventLoop: nil, + initialSettings: connectionConfiguration.initialSettings, + headerBlockValidation: connectionConfiguration.headerBlockValidation, + contentLengthValidation: connectionConfiguration.contentLengthValidation, + maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames, + maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, + maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, + resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength) + } + private init(mode: ParserMode, eventLoop: EventLoop?, initialSettings: HTTP2Settings, headerBlockValidation: ValidationState, contentLengthValidation: ValidationState, maximumSequentialEmptyDataFrames: Int, - maximumBufferedControlFrames: Int) { + maximumBufferedControlFrames: Int, + maximumResetFrameCount: Int, + resetFrameCounterWindow: TimeAmount) { self.eventLoop = eventLoop self.stateMachine = HTTP2ConnectionStateMachine(role: .init(mode), headerBlockValidation: .init(headerBlockValidation), contentLengthValidation: .init(contentLengthValidation)) self.mode = mode self.initialSettings = initialSettings self.outboundBuffer = CompoundOutboundBuffer(mode: mode, initialMaxOutboundStreams: 100, maxBufferedControlFrames: maximumBufferedControlFrames) - self.denialOfServiceValidator = DOSHeuristics(maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames) + self.denialOfServiceValidator = DOSHeuristics(maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames, maximumResetFrameCount: maximumResetFrameCount, resetFrameCounterWindow: resetFrameCounterWindow) self.tolerateImpossibleStateTransitionsInDebugMode = false self.inboundStreamMultiplexerState = .uninitializedLegacy } @@ -271,19 +297,25 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { /// upper limit on the depth of this queue. Defaults to 10,000. /// - tolerateImpossibleStateTransitionsInDebugMode: Whether impossible state transitions should be tolerated /// in debug mode. + /// - maximumResetFrameCount: Controls the maximum permitted reset frames within a given time window. Too many may exhaust CPU resources. To protect + /// against this DoS vector we put an upper limit on this rate. Defaults to 200. + /// - resetFrameCounterWindow: Controls the sliding window used to enforce the maximum permitted reset frames rate. Too many may exhaust CPU resources. To protect + /// against this DoS vector we put an upper limit on this rate. 30 seconds. internal init(mode: ParserMode, initialSettings: HTTP2Settings = nioDefaultSettings, headerBlockValidation: ValidationState = .enabled, contentLengthValidation: ValidationState = .enabled, maximumSequentialEmptyDataFrames: Int = 1, maximumBufferedControlFrames: Int = 10000, - tolerateImpossibleStateTransitionsInDebugMode: Bool = false) { + tolerateImpossibleStateTransitionsInDebugMode: Bool = false, + maximumResetFrameCount: Int = 200, + resetFrameCounterWindow: TimeAmount = .seconds(30)) { self.stateMachine = HTTP2ConnectionStateMachine(role: .init(mode), headerBlockValidation: .init(headerBlockValidation), contentLengthValidation: .init(contentLengthValidation)) self.mode = mode self.eventLoop = nil self.initialSettings = initialSettings self.outboundBuffer = CompoundOutboundBuffer(mode: mode, initialMaxOutboundStreams: 100, maxBufferedControlFrames: maximumBufferedControlFrames) - self.denialOfServiceValidator = DOSHeuristics(maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames) + self.denialOfServiceValidator = DOSHeuristics(maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames, maximumResetFrameCount: maximumResetFrameCount, resetFrameCounterWindow: resetFrameCounterWindow) self.tolerateImpossibleStateTransitionsInDebugMode = tolerateImpossibleStateTransitionsInDebugMode self.inboundStreamMultiplexerState = .uninitializedLegacy } @@ -1040,7 +1072,9 @@ extension NIOHTTP2Handler { headerBlockValidation: connectionConfiguration.headerBlockValidation, contentLengthValidation: connectionConfiguration.contentLengthValidation, maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames, - maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames + maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, + maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, + resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength ) self.inboundStreamMultiplexerState = .uninitializedInline(streamConfiguration, inboundStreamInitializer, streamDelegate) @@ -1061,7 +1095,9 @@ extension NIOHTTP2Handler { headerBlockValidation: connectionConfiguration.headerBlockValidation, contentLengthValidation: connectionConfiguration.contentLengthValidation, maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames, - maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames + maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, + maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, + resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength ) self.inboundStreamMultiplexerState = .uninitializedAsync(streamConfiguration, inboundStreamInitializerWithAnyOutput, streamDelegate) } @@ -1086,6 +1122,17 @@ extension NIOHTTP2Handler { public var targetWindowSize: Int = 65535 public var outboundBufferSizeHighWatermark: Int = 8196 public var outboundBufferSizeLowWatermark: Int = 4092 + public var streamResetFrameRateLimit: StreamResetFrameRateLimitConfiguration = .init() + public init() {} + } + + /// Stream reset frame rate limit configuration. + /// + /// The settings that control the maximum permitted reset frames within a given time window. Too many may exhaust CPU resources. + /// To protect against this DoS vector we put an upper limit on this rate. + public struct StreamResetFrameRateLimitConfiguration: Hashable, Sendable { + public var maximumCount: Int = 200 + public var windowLength: TimeAmount = .seconds(30) public init() {} } diff --git a/Sources/NIOHTTP2/HTTP2Error.swift b/Sources/NIOHTTP2/HTTP2Error.swift index 6bbe0e73..ae5623b0 100644 --- a/Sources/NIOHTTP2/HTTP2Error.swift +++ b/Sources/NIOHTTP2/HTTP2Error.swift @@ -289,6 +289,11 @@ public enum NIOHTTP2Errors { return MissingMultiplexer(file: file, line: line) } + /// Creates a ``ExcessiveRSTFrames`` error with appropriate source context. + public static func excessiveRSTFrames(file: String = #fileID, line: UInt = #line) -> ExcessiveRSTFrames { + return ExcessiveRSTFrames(file: file, line: line) + } + /// Creates a ``StreamError`` error with appropriate source context. /// /// - Parameters: @@ -1666,6 +1671,27 @@ public enum NIOHTTP2Errors { return true } } + + + /// The client has issued RST frames at an excessive rate resulting in the connection being defensively closed. + public struct ExcessiveRSTFrames: NIOHTTP2Error { + private let file: String + private let line: UInt + + /// The location where the error was thrown. + public var location: String { + return _location(file: self.file, line: self.line) + } + + fileprivate init(file: String, line: UInt) { + self.file = file + self.line = line + } + + public static func ==(lhs: Self, rhs: Self) -> Bool { + return true + } + } } diff --git a/Tests/NIOHTTP2Tests/DOSHeuristicsTests.swift b/Tests/NIOHTTP2Tests/DOSHeuristicsTests.swift new file mode 100644 index 00000000..c98ff1dd --- /dev/null +++ b/Tests/NIOHTTP2Tests/DOSHeuristicsTests.swift @@ -0,0 +1,111 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of SwiftNIO project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import XCTest +import NIOCore +@testable import NIOHTTP2 + +final class DOSHeuristicsTests: XCTestCase { + func testRSTFramePermittedRate() throws { + let testClock = TestClock() + var dosHeuristics = DOSHeuristics(maximumSequentialEmptyDataFrames: 100, maximumResetFrameCount: 200, resetFrameCounterWindow: .seconds(30), clock: testClock) + + // more resets than allowed, but slow enough to be okay + for i in 0..<300 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .seconds(1)) + } + } + + func testRSTFrameExcessiveRate() throws { + let testClock = TestClock() + var dosHeuristics = DOSHeuristics(maximumSequentialEmptyDataFrames: 100, maximumResetFrameCount: 200, resetFrameCounterWindow: .seconds(30), clock: testClock) + + // up to the limit + for i in 0..<200 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .milliseconds(1)) + } + + // over the limit + XCTAssertThrowsError(try dosHeuristics.process(.init(streamID: HTTP2StreamID(201), payload: .rstStream(.cancel)))) + } + + func testRSTFrameGarbageCollects() throws { + let testClock = TestClock() + var dosHeuristics = DOSHeuristics(maximumSequentialEmptyDataFrames: 100, maximumResetFrameCount: 200, resetFrameCounterWindow: .seconds(30), clock: testClock) + + // up to the limit + for i in 0..<200 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .milliseconds(1)) + } + + // clear out counter + testClock.advance(by: .seconds(30)) + + // up to the limit + for i in 0..<200 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .milliseconds(1)) + } + + // over the limit + XCTAssertThrowsError(try dosHeuristics.process(.init(streamID: HTTP2StreamID(401), payload: .rstStream(.cancel)))) + } + + func testRSTFrameExcessiveRateConfigurableCount() throws { + let testClock = TestClock() + var dosHeuristics = DOSHeuristics(maximumSequentialEmptyDataFrames: 100, maximumResetFrameCount: 400, resetFrameCounterWindow: .seconds(30), clock: testClock) + + // up to the limit + for i in 0..<400 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .milliseconds(1)) + } + + // over the limit + XCTAssertThrowsError(try dosHeuristics.process(.init(streamID: HTTP2StreamID(401), payload: .rstStream(.cancel)))) + } + + func testRSTFrameExcessiveRateConfigurableWindow() throws { + let testClock = TestClock() + var dosHeuristics = DOSHeuristics(maximumSequentialEmptyDataFrames: 100, maximumResetFrameCount: 200, resetFrameCounterWindow: .seconds(3600), clock: testClock) + + // up to the limit, previously slow enough to be okay but not with this window + for i in 0..<200 { + try dosHeuristics.process(.init(streamID: HTTP2StreamID(i), payload: .rstStream(.cancel))) + testClock.advance(by: .seconds(1)) + } + + // over the limit + XCTAssertThrowsError(try dosHeuristics.process(.init(streamID: HTTP2StreamID(201), payload: .rstStream(.cancel)))) + } +} + +class TestClock: NIODeadlineClock { + private var time: NIODeadline + + func now() -> NIODeadline { + return self.time + } + + func advance(by delta: TimeAmount) { + self.time = self.time + delta + } + + init() { + self.time = NIODeadline.now() + } +} diff --git a/Tests/NIOHTTP2Tests/SimpleClientServerFramePayloadStreamTests.swift b/Tests/NIOHTTP2Tests/SimpleClientServerFramePayloadStreamTests.swift index f195da62..cb711701 100644 --- a/Tests/NIOHTTP2Tests/SimpleClientServerFramePayloadStreamTests.swift +++ b/Tests/NIOHTTP2Tests/SimpleClientServerFramePayloadStreamTests.swift @@ -2060,4 +2060,38 @@ class SimpleClientServerFramePayloadStreamTests: XCTestCase { promise.succeed(()) stream.writeAndFlush(headerPayload, promise: promise) } + + func testTooManyResets() throws { + // Begin by getting the connection up. + try self.basicHTTP2Connection() + + let headers = HPACKHeaders([(":path", "/"), (":method", "POST"), (":scheme", "https"), (":authority", "localhost")]) + + // We're now going to try to send lots of resets from the client to the server. + for i in stride(from: 1, to: 402, by: 2) { + let clientStreamID = HTTP2StreamID(i) + let reqFrame = HTTP2Frame(streamID: clientStreamID, payload: .headers(.init(headers: headers))) + let reqBodyFrame = HTTP2Frame(streamID: clientStreamID, payload: .rstStream(.cancel)) + + try self.assertFramesRoundTrip(frames: [reqFrame], sender: self.clientChannel, receiver: self.serverChannel) + if i < 401 { + try self.assertFramesRoundTrip(frames: [reqBodyFrame], sender: self.clientChannel, receiver: self.serverChannel) + } else { + // this one should trip the DoS threshold and cause EmbeddedChannel to throw, we have to do more manually + self.clientChannel.write(reqBodyFrame, promise: nil) + self.clientChannel.flush() + switch try self.clientChannel.readOutbound(as: IOData.self) { + case .byteBuffer(let b): + XCTAssertThrowsError(try self.serverChannel.writeInbound(b)) { error in + XCTAssert(error is NIOHTTP2Errors.ExcessiveRSTFrames) + } + case .fileRegion, .none: + XCTFail() + } + } + } + + XCTAssertNoThrow(try self.clientChannel.finish()) + XCTAssertNoThrow(try self.serverChannel.finish()) + } }