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

fix failing ConnectWithRevocation_ServerCertWithoutContext_NoStapleOcsp failures #83013

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Mar 6, 2023

There was mismatch between the test and product. When context is created from certificate internally, the code defaults to OCSP processing

public static SslStreamCertificateContext Create(X509Certificate2 target, X509Certificate2Collection? additionalCertificates, bool offline = false, SslCertificateTrust? trust = null)
{
return Create(target, additionalCertificates, offline, trust, noOcspFetch: false);
}

(negative false -> oof ;( )

So the server would provide OCSP response and based on the timing the test would succeed or fail.

I was thinking about deleting the test but I decided to keep it and modify the behavior instead.
This would IMHO preserve legacy behavior. When single certificate is used it would act as it always did - e.g. no OCSP.

When SslStreamCertificateContext is created and provided by user we would take is as intention to reuse it over time and we would handle TLS resume & OCSP. This is currently recommended use as it also offers other performance benefits.

fixes #70981

@wfurt wfurt added area-System.Net.Security os-linux Linux OS (any supported distro) labels Mar 6, 2023
@wfurt wfurt requested review from rzikm and liveans March 6, 2023 01:56
@wfurt wfurt self-assigned this Mar 6, 2023
@ghost
Copy link

ghost commented Mar 6, 2023

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

Issue Details

There was mismatch between the test and product. When context is created from certificate internally, the code defaults to OCSP processing

public static SslStreamCertificateContext Create(X509Certificate2 target, X509Certificate2Collection? additionalCertificates, bool offline = false, SslCertificateTrust? trust = null)
{
return Create(target, additionalCertificates, offline, trust, noOcspFetch: false);
}

(negative false -> oof ;( )

So the server would provide OCSP response and based on the timing the test would succeed or fail.

I was thinking about deleting the test but I decided to keep it and modify the behavior instead.
This would IMHO preserve legacy behavior. When single certificate is used it would act as it always did - e.g. no OCSP.

When SslStreamCertificateContext is created and provided by user we would take is as intention to reuse it over time and we would handle TLS resume & OCSP. This is currently recommended use as it also offers other performance benefits.

fixes #70981

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Security, os-linux

Milestone: -

@wfurt
Copy link
Member Author

wfurt commented Mar 7, 2023

windows test failure is #83110

@wfurt wfurt merged commit 4354297 into dotnet:main Mar 7, 2023
@wfurt wfurt deleted the ocsp branch March 7, 2023 22:25
@karelz karelz added this to the 8.0.0 milestone Mar 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assert in System.Net.Security.Tests.CertificateValidationRemoteServer.ConnectWithRevocation_*
3 participants