-
Notifications
You must be signed in to change notification settings - Fork 101
Add GlitchesMonitor #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Add GlitchesMonitor #518
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
58d08a4
Add GlitchesMonitor
gjcairo 2ad9fc0
Small PR changes
gjcairo 7eb85a5
PR changes
gjcairo dc5cbf5
Format
gjcairo 8e0c871
Fix test
gjcairo e8fdfea
Rename max to maximum
gjcairo 07f3182
PR changes
gjcairo 014ac64
Format
gjcairo 4dc06b3
Update default glitches threshold
gjcairo db92ada
Undo a change that shouldn't have been committed
gjcairo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the SwiftNIO open source project | ||
| // | ||
| // Copyright (c) 2025 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 | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| struct GlitchesMonitor { | ||
| static var defaultMaximumGlitches: Int { 200 } | ||
| private var stateMachine: GlitchesMonitorStateMachine | ||
|
|
||
| init(maximumGlitches: Int = GlitchesMonitor.defaultMaximumGlitches) { | ||
| self.stateMachine = GlitchesMonitorStateMachine(maxGlitches: maximumGlitches) | ||
| } | ||
|
|
||
| mutating func processStreamError() throws { | ||
| switch self.stateMachine.recordEvent() { | ||
| case .belowLimit: | ||
| () | ||
|
|
||
| case .exceededLimit: | ||
| throw NIOHTTP2Errors.excessiveNumberOfGlitches() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension GlitchesMonitor { | ||
| private struct GlitchesMonitorStateMachine { | ||
| enum State { | ||
| case monitoring(numberOfGlitches: Int) | ||
| case glitchesExceeded | ||
| } | ||
|
|
||
| private var state: State | ||
| private let maxGlitches: Int | ||
|
|
||
| init(maxGlitches: Int) { | ||
| precondition(maxGlitches >= 0) | ||
| self.state = .monitoring(numberOfGlitches: 0) | ||
| self.maxGlitches = maxGlitches | ||
| } | ||
|
|
||
| enum RecordEventAction { | ||
|
glbrntt marked this conversation as resolved.
|
||
| case belowLimit | ||
| case exceededLimit | ||
| } | ||
|
|
||
| mutating func recordEvent() -> RecordEventAction { | ||
| switch self.state { | ||
| case .monitoring(let numberOfGlitches): | ||
| if numberOfGlitches < self.maxGlitches { | ||
| self.state = .monitoring(numberOfGlitches: numberOfGlitches &+ 1) | ||
| return .belowLimit | ||
| } else { | ||
| self.state = .glitchesExceeded | ||
| return .exceededLimit | ||
| } | ||
|
|
||
| case .glitchesExceeded: | ||
| return .exceededLimit | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,8 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| /// This object deploys heuristics to attempt to detect denial of service attacks. | ||
| private var denialOfServiceValidator: DOSHeuristics<RealNIODeadlineClock> | ||
|
|
||
| private var glitchesMonitor: GlitchesMonitor | ||
|
|
||
| /// The mode this handler is operating in. | ||
| private let mode: ParserMode | ||
|
|
||
|
|
@@ -236,6 +238,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| maximumBufferedControlFrames: 10000, | ||
| maximumSequentialContinuationFrames: NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames, | ||
| maximumRecentlyResetStreams: Self.defaultMaximumRecentlyResetFrames, | ||
| maximumConnectionGlitches: GlitchesMonitor.defaultMaximumGlitches, | ||
| maximumResetFrameCount: 200, | ||
| resetFrameCounterWindow: .seconds(30), | ||
| maximumStreamErrorCount: 200, | ||
|
|
@@ -273,6 +276,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| maximumBufferedControlFrames: maximumBufferedControlFrames, | ||
| maximumSequentialContinuationFrames: NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames, | ||
| maximumRecentlyResetStreams: Self.defaultMaximumRecentlyResetFrames, | ||
| maximumConnectionGlitches: GlitchesMonitor.defaultMaximumGlitches, | ||
| maximumResetFrameCount: 200, | ||
| resetFrameCounterWindow: .seconds(30), | ||
| maximumStreamErrorCount: 200, | ||
|
|
@@ -302,6 +306,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, | ||
| maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames, | ||
| maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams, | ||
| maximumConnectionGlitches: connectionConfiguration.maximumConnectionGlitches, | ||
| maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, | ||
| resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength, | ||
| maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount, | ||
|
|
@@ -319,6 +324,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| maximumBufferedControlFrames: Int, | ||
| maximumSequentialContinuationFrames: Int, | ||
| maximumRecentlyResetStreams: Int, | ||
| maximumConnectionGlitches: Int, | ||
| maximumResetFrameCount: Int, | ||
| resetFrameCounterWindow: TimeAmount, | ||
| maximumStreamErrorCount: Int, | ||
|
|
@@ -348,6 +354,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| self.tolerateImpossibleStateTransitionsInDebugMode = false | ||
| self.inboundStreamMultiplexerState = .uninitializedLegacy | ||
| self.maximumSequentialContinuationFrames = maximumSequentialContinuationFrames | ||
| self.glitchesMonitor = GlitchesMonitor(maximumGlitches: maximumConnectionGlitches) | ||
| } | ||
|
|
||
| /// Constructs a ``NIOHTTP2Handler``. | ||
|
|
@@ -369,6 +376,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| /// 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. | ||
| /// - maximumConnectionGlitches: Controls the maximum number of stream errors that can happen on a connection before the connection is reset. Defaults to 200. | ||
| internal init( | ||
| mode: ParserMode, | ||
| initialSettings: HTTP2Settings = nioDefaultSettings, | ||
|
|
@@ -382,7 +390,8 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| maximumResetFrameCount: Int = 200, | ||
| resetFrameCounterWindow: TimeAmount = .seconds(30), | ||
| maximumStreamErrorCount: Int = 200, | ||
| streamErrorCounterWindow: TimeAmount = .seconds(30) | ||
| streamErrorCounterWindow: TimeAmount = .seconds(30), | ||
| maximumConnectionGlitches: Int = GlitchesMonitor.defaultMaximumGlitches | ||
| ) { | ||
| self.stateMachine = HTTP2ConnectionStateMachine( | ||
| role: .init(mode), | ||
|
|
@@ -408,6 +417,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler { | |
| self.tolerateImpossibleStateTransitionsInDebugMode = tolerateImpossibleStateTransitionsInDebugMode | ||
| self.inboundStreamMultiplexerState = .uninitializedLegacy | ||
| self.maximumSequentialContinuationFrames = maximumSequentialContinuationFrames | ||
| self.glitchesMonitor = GlitchesMonitor(maximumGlitches: maximumConnectionGlitches) | ||
| } | ||
|
|
||
| public func handlerAdded(context: ChannelHandlerContext) { | ||
|
|
@@ -600,32 +610,41 @@ extension NIOHTTP2Handler { | |
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: NIOHTTP2Errors.unableToParseFrame(), | ||
| reason: code | ||
| reason: code, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| return nil | ||
| } catch is NIOHTTP2Errors.BadClientMagic { | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: NIOHTTP2Errors.badClientMagic(), | ||
| reason: .protocolError | ||
| reason: .protocolError, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| return nil | ||
| } catch is NIOHTTP2Errors.ExcessivelyLargeHeaderBlock { | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: NIOHTTP2Errors.excessivelyLargeHeaderBlock(), | ||
| reason: .protocolError | ||
| reason: .protocolError, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| return nil | ||
| } catch is NIOHTTP2Errors.ExcessiveContinuationFrames { | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: NIOHTTP2Errors.excessiveContinuationFrames(), | ||
| reason: .enhanceYourCalm | ||
| reason: .enhanceYourCalm, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| return nil | ||
| } catch { | ||
| self.inboundConnectionErrorTriggered(context: context, underlyingError: error, reason: .internalError) | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: error, | ||
| reason: .internalError, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| return nil | ||
| } | ||
| } | ||
|
|
@@ -733,6 +752,7 @@ extension NIOHTTP2Handler { | |
| } | ||
|
|
||
| self.processDoSRisk(frame, result: &result) | ||
| self.processGlitches(result: &result) | ||
| self.processStateChange(result.effect) | ||
|
|
||
| let returnValue: FrameProcessResult | ||
|
|
@@ -744,9 +764,14 @@ extension NIOHTTP2Handler { | |
| case .ignoreFrame: | ||
| // Frame is good but no action needs to be taken. | ||
| returnValue = .continue | ||
| case .connectionError(let underlyingError, let errorCode): | ||
| case .connectionError(let underlyingError, let errorCode, let isMisbehavingPeer): | ||
| // We should stop parsing on received connection errors, the connection is going away anyway. | ||
| self.inboundConnectionErrorTriggered(context: context, underlyingError: underlyingError, reason: errorCode) | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: underlyingError, | ||
| reason: errorCode, | ||
| isMisbehavingPeer: isMisbehavingPeer | ||
| ) | ||
| returnValue = .stop | ||
| case .streamError(let streamID, let underlyingError, let errorCode): | ||
| // We can continue parsing on stream errors in most cases, the frame is just ignored. | ||
|
|
@@ -770,15 +795,20 @@ extension NIOHTTP2Handler { | |
| private func inboundConnectionErrorTriggered( | ||
| context: ChannelHandlerContext, | ||
| underlyingError: Error, | ||
| reason: HTTP2ErrorCode | ||
| reason: HTTP2ErrorCode, | ||
| isMisbehavingPeer: Bool | ||
| ) { | ||
| // A connection error brings the entire connection down. We attempt to write a GOAWAY frame, and then report this | ||
| // error. It's possible that we'll be unable to write the GOAWAY frame, but that also just logs the error. | ||
| // Because we don't know what data the user handled before we got this, we propose that they may have seen all of it. | ||
| // The user may choose to fire a more specific error if they wish. | ||
|
|
||
| // If the peer is misbehaving, set the stream ID to the minimum allowed value (0). | ||
| // This will cause all open streams for this connection to be immediately terminated. | ||
| let streamID = isMisbehavingPeer ? HTTP2StreamID(0) : .maxID | ||
| let goAwayFrame = HTTP2Frame( | ||
| streamID: .rootStream, | ||
| payload: .goAway(lastStreamID: .maxID, errorCode: reason, opaqueData: nil) | ||
| payload: .goAway(lastStreamID: streamID, errorCode: reason, opaqueData: nil) | ||
| ) | ||
| self.writeUnbufferedFrame(context: context, frame: goAwayFrame) | ||
| self.flushIfNecessary(context: context) | ||
|
|
@@ -818,7 +848,29 @@ extension NIOHTTP2Handler { | |
| () | ||
| } | ||
| } catch { | ||
| result.result = StateMachineResult.connectionError(underlyingError: error, type: .enhanceYourCalm) | ||
| result.result = StateMachineResult.connectionError( | ||
| underlyingError: error, | ||
| type: .enhanceYourCalm, | ||
| isMisbehavingPeer: true | ||
| ) | ||
| result.effect = nil | ||
| } | ||
| } | ||
|
|
||
| private func processGlitches(result: inout StateMachineResultWithEffect) { | ||
| do { | ||
| switch result.result { | ||
| case .streamError: | ||
| try self.glitchesMonitor.processStreamError() | ||
| case .succeed, .ignoreFrame, .connectionError: | ||
| () | ||
| } | ||
| } catch { | ||
| result.result = .connectionError( | ||
| underlyingError: error, | ||
| type: .enhanceYourCalm, | ||
| isMisbehavingPeer: true | ||
| ) | ||
| result.effect = nil | ||
| } | ||
| } | ||
|
|
@@ -894,7 +946,12 @@ extension NIOHTTP2Handler { | |
| } | ||
| } catch let error where error is NIOHTTP2Errors.ExcessiveOutboundFrameBuffering { | ||
| self.inboundStreamMultiplexer?.processedFrame(frame) | ||
| self.inboundConnectionErrorTriggered(context: context, underlyingError: error, reason: .enhanceYourCalm) | ||
| self.inboundConnectionErrorTriggered( | ||
| context: context, | ||
| underlyingError: error, | ||
| reason: .enhanceYourCalm, | ||
| isMisbehavingPeer: false | ||
| ) | ||
| } catch { | ||
| self.inboundStreamMultiplexer?.processedFrame(frame) | ||
| promise?.fail(error) | ||
|
|
@@ -968,7 +1025,7 @@ extension NIOHTTP2Handler { | |
| switch result.result { | ||
| case .ignoreFrame: | ||
| preconditionFailure("Cannot be asked to ignore outbound frames.") | ||
| case .connectionError(let underlyingError, _): | ||
| case .connectionError(let underlyingError, _, _): | ||
| self.outboundConnectionErrorTriggered(context: context, promise: promise, underlyingError: underlyingError) | ||
| return | ||
| case .streamError(let streamID, let underlyingError, _): | ||
|
|
@@ -1330,6 +1387,7 @@ extension NIOHTTP2Handler { | |
| maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, | ||
| maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames, | ||
| maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams, | ||
| maximumConnectionGlitches: connectionConfiguration.maximumConnectionGlitches, | ||
| maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, | ||
| resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength, | ||
| maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount, | ||
|
|
@@ -1362,6 +1420,7 @@ extension NIOHTTP2Handler { | |
| maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames, | ||
| maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames, | ||
| maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams, | ||
| maximumConnectionGlitches: connectionConfiguration.maximumConnectionGlitches, | ||
| maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount, | ||
| resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength, | ||
| maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount, | ||
|
|
@@ -1386,6 +1445,17 @@ extension NIOHTTP2Handler { | |
| public var maximumBufferedControlFrames: Int = 10000 | ||
| public var maximumSequentialContinuationFrames: Int = NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames | ||
| public var maximumRecentlyResetStreams: Int = NIOHTTP2Handler.defaultMaximumRecentlyResetFrames | ||
|
|
||
| /// The maximum number of glitches that are allowed on a connection before it's forcefully closed. | ||
| /// | ||
| /// A glitch is defined as some suspicious event on a connection, i.e., similar to a DoS attack. | ||
| /// A running count of the number of glitches occurring on each connection will be kept. | ||
| /// When the number of glitches reaches this threshold, the connection will be closed. | ||
| /// | ||
| /// For more information, see the relevant presentation of the 2024 HTTP Workshop: | ||
| /// https://github.com/HTTPWorkshop/workshop2024/blob/main/talks/1.%20Security/glitches.pdf | ||
| public var maximumConnectionGlitches: Int = GlitchesMonitor.defaultMaximumGlitches | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good, I think, to include narrative docs that explain this field and what it does. |
||
|
|
||
| public init() {} | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.