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

Disable MsQuicTests.CertificateCallbackThrowPropagates test #99108

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Feb 29, 2024

Contributes to #99074.

Seems like microsoft/msquic#4145 was not enough to fix async certificate rejection.

The test spuriously fails on platforms which have MsQuic built from main, which seems to be the Alpine queues where we don't have MsQuic release feeds for.

@ghost
Copy link

ghost commented Feb 29, 2024

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

Issue Details

Contributes to 99074.

Seems like microsoft/msquic#4145 was not enough to fix async certificate rejection.

The test is spuriously fails on platforms which have MsQuic built from main, which seems to be the Alpine queues where we don't have MsQuic release feeds for.

Author: rzikm
Assignees: rzikm
Labels:

area-System.Net.Quic

Milestone: -

@CarnaViire
Copy link
Member

MsQuic built from main

IIRC it would have the version number of the last release +1, would the version comparison actually work? Should we just disable it for Alpine?

@rzikm
Copy link
Member Author

rzikm commented Feb 29, 2024

IIRC it would have the version number of the last release +1, would the version comparison actually work? Should we just disable it for Alpine?

That's true, but the issue is not Alpine specific, it happens when we try to do the certificate validation asynchronously. There were actually two bugs in MsQuic:

  • It would attempt to send Transport Close on 1RTT, this I have fixed in main, and I have added a version check to .NET code so that we would do the validation asynchronously only if we run against MsQuic with the fix, which is currently in only main, so the check is >= 2.4.
  • There is a race between ConnectionCertificateValidationComplete and ConnectionShutdown which is more rare and I did not hit it during testing, but it semi-reliably hits on Alpine containers, more info on the original issue.

So, on versions < 2.4 we will do the validation inline and will avoid both of the above mentioned issues.

@rzikm
Copy link
Member Author

rzikm commented Feb 29, 2024

CI failures are unrelated

@rzikm rzikm merged commit 028eda8 into dotnet:main Feb 29, 2024
107 of 111 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2024
@karelz karelz added this to the 9.0.0 milestone May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants