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

[release/8.0] fix ReceiveFrom with dual mode socket #92103

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 15, 2023

Backport of #92086 to release/8.0

Fixes #92006

/cc @karelz @wfurt

Customer Impact

Regression against .NET 7.0

When UDP user uses IPv4 address (on machine with IPv6 enabled as well - which is typical scenario), they may get wrong peer address from Socket.ReceiveFrom (the main API for receiving data for UDP). As a result, they may start sending UDP packets to the wrong endpoint - potentially leading to privacy problems.
There is a workaround to pass the address with correct AddressFamily, but that requires the developer to anticipate the problem.

The regression was introduced by PR #89841 (UDP perf improvements).

Testing

Targeted tests are added in the PR
Verified on E2E original repro from #92006 (using managed QUIC implementation side project)

Risk

Medium. UDP is used much less than TCP (also partially due to its suboptimal performance prior to .NET 8.0).

@ghost
Copy link

ghost commented Sep 15, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #92086 to release/8.0

/cc @karelz @wfurt

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net.Sockets

Milestone: -

Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I also verified the change on the original repro and it fixes the issue.

@karelz karelz added this to the 8.0.0 milestone Sep 15, 2023
@karelz karelz added the Servicing-consider Issue for next servicing release review label Sep 15, 2023
@karelz
Copy link
Member

karelz commented Sep 15, 2023

I approve - it is a regression and the fix is not that complicated. Given the privacy concerns, I would take it.

@artl93 ready for your approval

Copy link
Member

@artl93 artl93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M2 Approved.

@artl93 artl93 added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 15, 2023
@carlossanlop
Copy link
Member

The tvos-arm64 legs might be stuck.

@wfurt
Copy link
Member

wfurt commented Sep 15, 2023

The tvos-arm64 legs might be stuck.

it is finished now, all check are green.

@carlossanlop carlossanlop merged commit bb44ca5 into release/8.0 Sep 15, 2023
107 of 108 checks passed
@carlossanlop carlossanlop deleted the backport/pr-92086-to-release/8.0 branch September 15, 2023 18:19
@radical radical mentioned this pull request Sep 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Sockets Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants