Repository navigation
[release/9.0] Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table - #69249
Conversation
…HPACK table (#68743) * Hardening trailers to refuse new line chars * Hardening HPACK dynamic header to refuse new line chars * Http3 request trailers with newline chars test * Added test to exercise newline chars on indexed headers
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Hi @BrennanConroy. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
|
Hi @BrennanConroy. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge. To learn more about how to prepare a servicing PR click here. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The only noted issue is a non-blocking test assertion nit; no approval-blocking issues remain.
Review tier: Lite
Findings: None
What changed in this PR
This backport hardens Kestrel HTTP/2 and HTTP/3 handling by rejecting CR/LF characters in trailers and dynamic HPACK values.
Changes:
- Enables newline validation.
- Adds HTTP/2 and HTTP/3 regression tests.
| File | Summary |
|---|---|
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs |
Adds HTTP/3 trailer rejection tests; includes one test assertion nit. |
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs |
Tests dynamic HPACK newline rejection. |
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs |
Adds malformed trailer cases. |
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs |
Validates dynamic HPACK header values. |
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs |
Validates trailer values. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Hi @BrennanConroy. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed. |
|
/ba-g Known unrelated test issue |
) Updated [Microsoft.AspNetCore.Mvc.Testing](https://github.com/dotnet/aspnetcore) from 9.0.20 to 9.0.22. <details> <summary>Release notes</summary> _Sourced from [Microsoft.AspNetCore.Mvc.Testing's releases](https://github.com/dotnet/aspnetcore/releases)._ ## 9.0.21 [Release](https://github.com/dotnet/core/releases/tag/v9.0.21) ## What's Changed * [release/9.0] Update branding to 9.0.21 by @vseanreesermsft in dotnet/aspnetcore#68962 * [release/9.0] Update dependencies from dotnet/source-build-assets by @dotnet-maestro[bot] in dotnet/aspnetcore#68967 * [release/9.0] (deps): Bump src/submodules/googletest from `3940de9` to `36ba75f` by @dependabot[bot] in dotnet/aspnetcore#68919 * [release/9.0] Update OpenApi to 1.6.31 by @Youssef1313 in dotnet/aspnetcore#68837 * [release/9.0] Extract IsAuthenticated helper method by @Youssef1313 with @Copilot in dotnet/aspnetcore#68659 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#68570 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#69085 * [release/9.0] Fix h3 connection-level and stream-level abort locking by @BrennanConroy in dotnet/aspnetcore#69151 * Merging internal commits for release/9.0 by @dotnet-bot in dotnet/aspnetcore#69179 * [release/9.0] [SignalR] [Java] Fix hang when reading an HTTP response body fails by @github-actions[bot] in dotnet/aspnetcore#69208 * [release/9.0] Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table by @BrennanConroy in dotnet/aspnetcore#69249 * [release/9.0] Scope sections E2E assertions to mounted app by @wtgodbe in dotnet/aspnetcore#69252 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#69268 * Update MessagePack to 2.5.303 by @BrennanConroy in dotnet/aspnetcore#69369 **Full Changelog**: dotnet/aspnetcore@v9.0.20...v9.0.21 Commits viewable in [compare view](https://github.com/dotnet/aspnetcore/commits). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
) Updated [Microsoft.AspNetCore.Mvc.Testing](https://github.com/dotnet/aspnetcore) from 9.0.20 to 9.0.22. <details> <summary>Release notes</summary> _Sourced from [Microsoft.AspNetCore.Mvc.Testing's releases](https://github.com/dotnet/aspnetcore/releases)._ ## 9.0.21 [Release](https://github.com/dotnet/core/releases/tag/v9.0.21) ## What's Changed * [release/9.0] Update branding to 9.0.21 by @vseanreesermsft in dotnet/aspnetcore#68962 * [release/9.0] Update dependencies from dotnet/source-build-assets by @dotnet-maestro[bot] in dotnet/aspnetcore#68967 * [release/9.0] (deps): Bump src/submodules/googletest from `3940de9` to `36ba75f` by @dependabot[bot] in dotnet/aspnetcore#68919 * [release/9.0] Update OpenApi to 1.6.31 by @Youssef1313 in dotnet/aspnetcore#68837 * [release/9.0] Extract IsAuthenticated helper method by @Youssef1313 with @Copilot in dotnet/aspnetcore#68659 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#68570 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#69085 * [release/9.0] Fix h3 connection-level and stream-level abort locking by @BrennanConroy in dotnet/aspnetcore#69151 * Merging internal commits for release/9.0 by @dotnet-bot in dotnet/aspnetcore#69179 * [release/9.0] [SignalR] [Java] Fix hang when reading an HTTP response body fails by @github-actions[bot] in dotnet/aspnetcore#69208 * [release/9.0] Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table by @BrennanConroy in dotnet/aspnetcore#69249 * [release/9.0] Scope sections E2E assertions to mounted app by @wtgodbe in dotnet/aspnetcore#69252 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#69268 * Update MessagePack to 2.5.303 by @BrennanConroy in dotnet/aspnetcore#69369 **Full Changelog**: dotnet/aspnetcore@v9.0.20...v9.0.21 Commits viewable in [compare view](https://github.com/dotnet/aspnetcore/commits). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Backport of #68743 to release/9.0
Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table
Description
Kestrel previously skipped validation for CR/LF in two places: request trailers and HPACK dynamic table retrieval. Based on RFC 7540 section 10.3, these values must be treated as malformed. The validation for newline characters has now been enabled along with tests to ensure compliance.
Customer Impact
Invalid headers could show up in logs or application logic processing.
Regression?
Risk
Good test coverage, easy to reason about change
Verification
Packaging changes reviewed?