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

[Android] Investigate support for SSLStream below API Level 24 #78715

Closed
steveisok opened this issue Nov 22, 2022 · 9 comments · Fixed by #78918
Closed

[Android] Investigate support for SSLStream below API Level 24 #78715

steveisok opened this issue Nov 22, 2022 · 9 comments · Fixed by #78918

Comments

@steveisok
Copy link
Member

steveisok commented Nov 22, 2022

The block below will throw a PNSE because SNIHostName and SSLEngine.setServerNames are only supported on API level 24 and above.

Here is a sample that can throw the PNSE:

using var client = new TcpClient("login.sequrix.com", 443);
            using var stream = new SslStream(client.GetStream(), false);
            stream.AuthenticateAsClient(new SslClientAuthenticationOptions()
            {
                TargetHost = "login.sequrix.com"
            });
            stream.Read(new byte[100], 0, 100);

We should investigate if there are any alternative approaches.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 22, 2022
@steveisok steveisok added this to the 8.0.0 milestone Nov 22, 2022
@ghost
Copy link

ghost commented Nov 22, 2022

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

Issue Details

The block below will throw a PNSE because SNIHostName and setServerNames are only supported on API level 24 and above.

Here is a sample that can throw the PNSE:

using var client = new TcpClient("login.sequrix.com", 443);
            using var stream = new SslStream(client.GetStream(), false);
            stream.AuthenticateAsClient(new SslClientAuthenticationOptions()
            {
                TargetHost = "login.sequrix.com"
            });
            stream.Read(new byte[100], 0, 100);

We should investigate if there are any alternative approaches.

Author: steveisok
Assignees: -
Labels:

area-System.Net, untriaged

Milestone: 8.0.0

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 22, 2022
@steveisok steveisok added area-System.Net.Security untriaged New issue has not been triaged by the area owner and removed area-System.Net labels Nov 22, 2022
@ghost
Copy link

ghost commented Nov 22, 2022

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

Issue Details

The block below will throw a PNSE because SNIHostName and setServerNames are only supported on API level 24 and above.

Here is a sample that can throw the PNSE:

using var client = new TcpClient("login.sequrix.com", 443);
            using var stream = new SslStream(client.GetStream(), false);
            stream.AuthenticateAsClient(new SslClientAuthenticationOptions()
            {
                TargetHost = "login.sequrix.com"
            });
            stream.Read(new byte[100], 0, 100);

We should investigate if there are any alternative approaches.

Author: steveisok
Assignees: -
Labels:

area-System.Net, area-System.Net.Security, untriaged

Milestone: 8.0.0

@steveisok steveisok added os-android and removed untriaged New issue has not been triaged by the area owner labels Nov 22, 2022
@steveisok
Copy link
Member Author

After looking at this again, it does seem returning UNSUPPORTED_API_LEVEL is heavy handed.

if (g_SNIHostName == NULL || g_SSLParametersSetServerNames == NULL)
{
// SSL not supported below API Level 24
return UNSUPPORTED_API_LEVEL;
}

It does appear that the java SSLEngine can be created with a host name and a port. Not sure how much of the interop we would have to shuffle to accommodate.

@rgroenewoudt
Copy link

rgroenewoudt commented Nov 23, 2022

This also effects SocketsHttpHandler and ClientWebSocket usage with HTTPS.

For example:

var client = new HttpClient(new SocketsHttpHandler());
await client.GetStringAsync("https://google.com");

@simonrozsival
Copy link
Member

I digged into it and it seems that we can force APIs 21-23 to do what we want, although it's not very clean. I'll try to open a draft PR so that we can have a discussion if those code changes are acceptable or not.

@marek-safar
Copy link
Contributor

API23 is 5+ years out of supported version, do we need to support that in net8?

@simonrozsival
Copy link
Member

There are still quite a few devices running APIs 21-23 in the wild, around 6.1%. It would be easier to drop support for them, but it should be possible to keep supporting them with relatively little effort.

image

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 28, 2022
@steveisok
Copy link
Member Author

API23 is 5+ years out of supported version, do we need to support that in net8?

I'll raise that with the larger team and see if there's a desire to bump the min version. I think this definitely needs to be fixed for .NET 6/7.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 6, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 5, 2023
@karelz
Copy link
Member

karelz commented May 27, 2023

Fixed in main (8.0) in PR #78918 and in 7.0.3 in PR #79280 and in 6.0.14 in PR #79277.

@karelz karelz modified the milestones: 8.0.0, 6.0.x May 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants