Skip to content

[release/8.0-staging] use also SslCertificateTrust when constructing CertificateContext #104541

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.

Sign up for GitHub

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

Jump to bottom
Merged
vitek-karas merged 3 commits into from
Jul 15, 2024
Merged

[release/8.0-staging] use also SslCertificateTrust when constructing CertificateContext #104541

vitek-karas merged 3 commits into from
Jul 15, 2024

Conversation

@simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Jul 8, 2024 •
edited
Loading

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
@wfurt @simonrozsival
use also SslCertificateTrust when constructing CertificateContext (do…
9f1b0c4 
…tnet#103372)

* use also SslCertificateTrust when constructing CertificateContext

* 'build

* feedback
@simonrozsival
[Android] Fix SslStreamCertificateContext empty custom trust store ex…
bd75077 
…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
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 8, 2024

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

@simonrozsival simonrozsival mentioned this pull request Jul 8, 2024
Closed
@build-analysis build-analysis bot mentioned this pull request Jul 8, 2024
Closed
@carlossanlop
Copy link
Contributor

carlossanlop commented Jul 12, 2024

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

rzikm
rzikm approved these changes Jul 13, 2024
View reviewed changes
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

matouskozak commented Jul 15, 2024

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

matouskozak commented Jul 15, 2024

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 15, 2024

Azure Pipelines successfully started running 1 pipeline(s).

@matouskozak
Copy link
Member

matouskozak commented Jul 15, 2024

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

@dotnet dotnet deleted a comment from azure-pipelines bot Jul 15, 2024
@dotnet dotnet deleted a comment from azure-pipelines bot Jul 15, 2024
@dotnet dotnet deleted a comment from azure-pipelines bot Jul 15, 2024
@dotnet dotnet deleted a comment from azure-pipelines bot Jul 15, 2024
@dotnet dotnet deleted a comment from azure-pipelines bot Jul 15, 2024
@vitek-karas vitek-karas added the Servicing-consider Issue for next servicing release review label Jul 15, 2024
@vitek-karas vitek-karas added this to the 8.0.x milestone Jul 15, 2024
This was referenced Jul 15, 2024
Closed
Closed
Open
Closed
Closed
Open
Open
@vitek-karas vitek-karas added api-approved API was approved in API review, it can be implemented and removed Servicing-consider Issue for next servicing release review labels Jul 15, 2024
wfurt
wfurt approved these changes Jul 15, 2024
View reviewed changes
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
@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.

Reviewers

@wfurt wfurt wfurt approved these changes

@rzikm rzikm rzikm approved these changes

Assignees

@simonrozsival simonrozsival

Labels

area-System.Net.Security Servicing-approved Approved for servicing release

Projects

None yet

Milestone

8.0.x

Development

Successfully merging this pull request may close these issues.

6 participants

@simonrozsival @carlossanlop @matouskozak @wfurt @rzikm @vitek-karas