Skip to content
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

Kestrel: Ability to capture HTTP2 errors #53377

Open
1 task done
cyberfirst-developer opened this issue Jan 15, 2024 · 5 comments
Open
1 task done

Kestrel: Ability to capture HTTP2 errors #53377

cyberfirst-developer opened this issue Jan 15, 2024 · 5 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions question

Comments

@cyberfirst-developer
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

I am using Kestrel with YARP in place of nginx, i did implemented some basic ban with ipset for bad requests.
Once i found much of records in logs:
crit: Microsoft.AspNetCore.Server.Kestrel.Http2[60] Connection id "0HN0HCNL3FVF3" exceeded the output operations maximum queue size. Microsoft.AspNetCore.Connections.ConnectionAbortedException: HTTP/2 connection exceeded the output operations maximum queue size.
CPU load was 100% at that time, and proxy was nearly not avaliable.

i checked sources, and can't find the way to capture such errors(most of such errors are writen into log, and passed as argument to FrameWriter, but not exposed anywhere).
I want to capture such errors and ban addresses which generate too much errors.

Describe the solution you'd like

one of the option i can think of is make and expose IConnectionAbortReasonFeature, which will be added to connection. and can be examined with in connection builder middleware.(after call to next in chain of course), with some kind enum, like it is done in YARP with ForwarderError enum)

Additional context

Currently i found possible way to capture exception, by making wrapping Connectioncontext into own one and overriding Abort method(all other prop and methods just proxy toold context), but i can't see this as good solution.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jan 15, 2024
@amcasey
Copy link
Member

amcasey commented Jan 17, 2024

As you probably already know (but, for the sake of others reading this later), that message indicates that a client is behaving as if it's performing a Rapid Reset attack. One of the reasons we decided to close the connection but not flag the particular client was because the immediate client could be a proxy representing many different users. That reasoning may not apply to your particular server, but it's worth considering.

Another thing to note is that the limit is configurable. If you find that the default value is not providing adequate protection (i.e. your CPU is hitting 100% and your proxy is nearly unavailable), you can try reducing Microsoft.AspNetCore.Server.Kestrel.Http2.MaxConnectionFlowControlQueueSize . Of course, this will only affect individual connections - if you receive many of these apparent attacks simultaneously, the limit won't help.

@amcasey
Copy link
Member

amcasey commented Jan 17, 2024

When you see that log message, the corresponding connection will see a TCP RST frame send from kestrel. Is that a useful signal when determining whether to ban addresses?

@cyberfirst-developer
Copy link
Author

When you see that log message, the corresponding connection will see a TCP RST frame send from kestrel. Is that a useful signal when determining whether to ban addresses?

Error happens when connection still considered alive, and we have responses to write there.
i never saw kernel to send not existsing RST packet, is just closes socket AFAIK.

One of the reasons we decided to close the connection but not flag the particular client was because the immediate client could be a proxy representing many different users. That reasoning may not apply to your particular server, but it's worth considering.

Correct, and Kestrel actually can't do that itself(there only can be some interface to be implemented for marking clients. actually their src ip even, since connection already broken)
I don't plan to mark client for single error, this error can happen even as not part of attack, but usually we had same error once per hour at most. I plan to count such errors, if amount passes some threshold, then limit this particular src for some time.
i know that with NAT this can be many clients, but in most cases such ip's where from data centers, not from ISP. and safety of server comes first, better lose several clients, in comparison with being not responsive at all.

So, what i want is easier way to access such things, i currently found that with context replacement in ListenOption.Use i can capture error, but did not tested yet.

@amcasey
Copy link
Member

amcasey commented Jan 18, 2024

What about using something like a logger provider?

internal sealed class RapidResetLoggerProvider : ILoggerProvider 
{ 
    public static readonly ILoggerProvider Instance = new RapidResetLoggerProvider(); 
    private RapidResetLoggerProvider() { } 
    public ILogger CreateLogger(string _categoryName) => RapidResetLogger.Instance; 
    public void Dispose() { } 
 
    private class RapidResetLogger : ILogger 
    { 
        public static readonly ILogger Instance = new RapidResetLogger(); 
        private RapidResetLogger() { } 
        public IDisposable BeginScope<TState>(TState _state) => DummyDisposable.Instance; 
        public bool IsEnabled(LogLevel logLevel) => logLevel >= LogLevel.Debug; 
 
        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) 
        { 
            switch (eventId.Name) 
            { 
                case "Http2FlowControlQueueOperationsExceeded": 
                    // Handler goes here
                    break; 
            } 
        } 
 
        private sealed class DummyDisposable : IDisposable 
        { 
            public static readonly IDisposable Instance = new DummyDisposable(); 
            private DummyDisposable() { } 
            public void Dispose() { } 
        } 
    } 
} 

@cyberfirst-developer
Copy link
Author

What about using something like a logger provider?

I will prefer context replacement, since logger don't have access to context itself, so there i can't access src ip for example and so on.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions question
Projects
None yet
Development

No branches or pull requests

3 participants