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

Author: rnro

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<DeadlineClock: NIODeadlineClock> {
     /// 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<NIODeadline>
+        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()
+    }
+}

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<RealNIODeadlineClock>
 
     /// 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() {}
     }
 

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
+        }
+    }
 }