[release/7.0] Ensure free buffer space when reading TLS messages #83574
Conversation
![github-actions[bot]](https://github.com/avatars/in/15368?s=60&v=4){kind=small}
The initial size is not enough to cover later TLS frames
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsBackport of #83480 to release/7.0 /cc @rzikm Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
| @@ -684,10 +684,18 @@ private async ValueTask<int> EnsureFullTlsFrameAsync<TIOAdapter>(CancellationTok | |||
| return frameSize; | |||
| } | |||
|
|
|||
| if (frameSize < int.MaxValue) | |||
| if (frameSize != int.MaxValue) | |||
There was a problem hiding this comment.
Why is it needed? Looks like semantically it is the same. Only MaxValue can be "not <".
There was a problem hiding this comment.
Not needed, it was part of stylistic change to make it evident that int.MaxValue is a special value ("unknown size"). I can remove it from the backport.
|
Impact on customers hitting real servers (see top post), worth servicing. |
rbhanda
added
Servicing-approved
|
I'm retargeting this PR to the new Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
The new process is described here: runtime/docs/project/library-servicing.md. The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues. Let me know if you have any questions. |
|
Test failures are known problems - tracked in #83901 |
|
@carlossanlop The |
carlossanlop
added
Servicing-approved
|
Test failures are #83901 (known and closed already) |
ghost
locked as resolved and limited conversation to collaborators
Backport of #83480 to release/7.0
Fixes #83455.
/cc @rzikm
Customer Impact
7.0 Introduced regression where SslStream would fail to establish TLS connection (and, by extension, HTTPS) to some servers in very specific circumstances (depending on the sizing of TLS records exchanged during the handshake). One such server is
lh3.googleusercontent.com.For affected servers it happens all the time - i.e. .NET is unable to establish connection to them.
Testing
Manual test against the affected server
lh3.googleusercontent.com.CI test suite passes.
Risk
Low, we didn't change code path when the frame size is known. The code change affects only code path when the frame size is unknown, which happens only when the server sends TLS frames slightly shorter than 16K/8K/4K (see example of the google server above).