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/7.0] Prevent use of null when processing ALPN list #81797

Merged
merged 3 commits into from
Mar 10, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Feb 7, 2023

Backport of #81629 to release/7.0

Fixes #81588

/cc @wfurt

Customer Impact

We can throw exception in our native callback we register in OpenSSL. OpenSSL cannot handle it, which leads to process crash.
It seems to be a rare condition - so far 2 customers hit it, (for one, happening once per 2 days). The impact is bad (a process crash).

The root cause is not yet fully understood. We need to look deeper into the dumps.
This is a band-aid fix ('null' check) to change crash into OpenSSL error code, which customer code can handle.

Testing

2 customers verified the fix on private 7.0 servicing bits in their environments - see confirmation and confirmation.

All functional tests are passing

Regression

Yes, this is a new code path in 7.0 which app written against 6.0 can hit.

Risk

Small

@ghost
Copy link

ghost commented Feb 7, 2023

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

Issue Details

Backport of #81629 to release/7.0

/cc @wfurt

Customer Impact

Testing

Risk

IMPORTANT: 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.

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

area-System.Security

Milestone: -

@ghost
Copy link

ghost commented Feb 8, 2023

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

Issue Details

Backport of #81629 to release/7.0

/cc @wfurt

Customer Impact

Testing

Risk

IMPORTANT: 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.

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

area-System.Net.Security

Milestone: -

@wfurt wfurt added Servicing-consider Issue for next servicing release review os-linux Linux OS (any supported distro) labels Feb 8, 2023
@carlossanlop carlossanlop force-pushed the backport/pr-81629-to-release/7.0 branch from 512159f to b414e32 Compare February 10, 2023 22:27
@carlossanlop
Copy link
Member

carlossanlop commented Feb 13, 2023

@rzikm @wfurt @karelz I see the servicing-consider label was added, but I can't find a Tactics email approving it. Friendly reminder that today's Code Complete day for the March Servicing Release.

@karelz
Copy link
Member

karelz commented Feb 14, 2023

@carlossanlop we should NOT push this into March release, let's do it next month. Thank you!

@ericstj ericstj removed the Servicing-consider Issue for next servicing release review label Feb 14, 2023
@karelz karelz added the blocked Issue/PR is blocked on something - see comments label Feb 21, 2023
@karelz
Copy link
Member

karelz commented Feb 21, 2023

Marking as 'blocked' to first gather more info about impact, workaround feasibility and finding root-cause -- see comments in the template above.

@karelz karelz changed the title [release/7.0] prevent use of null when processing alpn list [release/7.0] Prevent use of null when processing ALPN list Feb 21, 2023
@karelz karelz added this to the 7.0.x milestone Feb 21, 2023
@carlossanlop
Copy link
Member

@karelz @wfurt the branch is now open for merging into the April release. Is this still blocked?

@wfurt wfurt added Servicing-consider Issue for next servicing release review and removed blocked Issue/PR is blocked on something - see comments labels Mar 9, 2023
@wfurt
Copy link
Member

wfurt commented Mar 9, 2023

two customers verified the mainline fix.

@karelz
Copy link
Member

karelz commented Mar 9, 2023

@carlossanlop it is ready for Tactics consideration -- the fix has been verified by 2 customers (see updated template).

@carlossanlop carlossanlop removed this from the 7.0.x milestone Mar 10, 2023
@carlossanlop carlossanlop added this to the 7.0.5 milestone Mar 10, 2023
@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 10, 2023
@carlossanlop
Copy link
Member

Approved by Tactics via email.
Signed-off by area owners.
No OOB changes needed (Interop/Common code).
CI is green.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 2448ea9 into release/7.0 Mar 10, 2023
@carlossanlop carlossanlop deleted the backport/pr-81629-to-release/7.0 branch March 10, 2023 23:37
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security os-linux Linux OS (any supported distro) Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants