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-staging] use also SslCertificateTrust when constructing CertificateContext #104541

Conversation

simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Jul 8, 2024

Backport of #103372 and #104016 to release/8.0-staging

Customer Impact

Customers developing Android apps are currently unable to use mutual TLS authentication in certain cases as the SslStreamCertificateContext.Create(...) method will fail to build an X509Chain instance if the certificate isn't trusted by the OS due to the limitations of the Android platform.

Regression

  • Yes
  • No

Testing

Unit tests and manual testing on Android emulator.

Risk

Low. The change is mostly limited to Android where this API doesn't currently work in many cases.

/cc @vitek-karas @wfurt

wfurt and others added 2 commits July 8, 2024 12:13
…tnet#103372)

* use also SslCertificateTrust when constructing CertificateContext

* 'build

* feedback
…ception (dotnet#104016)

* Check if certificate collections are not empty before changing trust mode to custom root trust

* Enable SslStream_ClientCertificateContext_SendsChain test on Android

* Apply suggestions from reviews

* Avoid unnecessary allocations
Copy link
Contributor

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

@carlossanlop
Copy link
Member

Friendly reminder that Monday July 15th is Code Complete day, that's the deadline to get this included in the August Release.

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

@matouskozak
Copy link
Member

The System.Net.Http.WinHttpHandlerFunctional.Tests.BidirectionStreamingTest.BackwardsCompatibility_DowngradeToHttp11 failures are unrelated as they are already present on release/8.0-staging rolling builds e.g. https://dev.azure.com/dnceng-public/public/_build/results?buildId=739868 and are tracked at #104390.

@matouskozak
Copy link
Member

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matouskozak
Copy link
Member

The runtime-extra-platforms failures are unrelated are can be seen failing on rolling builds as well. E.g. https://dev.azure.com/dnceng-public/public/_build/results?buildId=739937

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

@vitek-karas vitek-karas added Servicing-approved Approved for servicing release and removed api-approved API was approved in API review, it can be implemented labels Jul 15, 2024
@vitek-karas vitek-karas merged commit ce2fdb4 into dotnet:release/8.0-staging Jul 15, 2024
45 of 101 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants