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

SslStream.TargetHostName is unavailable #57105

Closed
shirhatti opened this issue Aug 10, 2021 · 3 comments · Fixed by #60589
Closed

SslStream.TargetHostName is unavailable #57105

shirhatti opened this issue Aug 10, 2021 · 3 comments · Fixed by #60589

Comments

@shirhatti
Copy link
Contributor

Description

SslStream.TargetHostName is not set unless I've I'm using one of the callbacks on SslStream (LocalCertificateSelectionCallback or the ServerOptionsSelectionCallback). If I specify the certificate directly, you won't attempt to decode the SNI)

case TlsContentType.Handshake:
if (!_isRenego && _handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello &&
(_sslAuthenticationOptions!.ServerCertSelectionDelegate != null ||
_sslAuthenticationOptions!.ServerOptionDelegate != null))
{
TlsFrameHelper.ProcessingOptions options = NetEventSource.Log.IsEnabled() ?
TlsFrameHelper.ProcessingOptions.All :
TlsFrameHelper.ProcessingOptions.ServerName;
// Process SNI from Client Hello message
if (!TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame, options))
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"Failed to parse TLS hello.");
}

Repro

Server:

using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Security.Cryptography.X509Certificates;
using System.Text;
var port = 5001;
var ipEndPoint = new IPEndPoint(IPAddress.Loopback, port);
var listenSocket = new Socket(ipEndPoint.AddressFamily,
                        SocketType.Stream,
                        ProtocolType.Tcp);
listenSocket.Bind(ipEndPoint);
listenSocket.Listen();
using var acceptSocket = await listenSocket.AcceptAsync();
using var ns = new NetworkStream(acceptSocket);
using var sslStream = new SslStream(ns);
using var store = new X509Store("My", StoreLocation.CurrentUser, OpenFlags.ReadOnly);
using var certificate = store.Certificates.Find(X509FindType.FindBySubjectName, "localhost", false)
                  .Where(c => c.HasPrivateKey)
                  .FirstOrDefault();

// Specifying a delegate instead of directly providing the certificate works
var sslOptions = new SslServerAuthenticationOptions
{
    ServerCertificate = certificate,
//  ServerCertificateSelectionCallback = (_, name) => certificate,
    CertificateRevocationCheckMode = X509RevocationMode.NoCheck
};
sslStream.AuthenticateAsServer(sslOptions);
Debug.Assert(!string.IsNullOrEmpty(sslStream.TargetHostName));
await sslStream.WriteAsync(Encoding.UTF8.GetBytes("Hello, World!"));

Client: curl.exe -k https://localhost:5001

👀 @wfurt

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Security untriaged New issue has not been triaged by the area owner labels Aug 10, 2021
@ghost
Copy link

ghost commented Aug 10, 2021

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

Issue Details

Description

SslStream.TargetHostName is not set unless I've I'm using one of the callbacks on SslStream (LocalCertificateSelectionCallback or the ServerOptionsSelectionCallback). If I specify the certificate directly, you won't attempt to decode the SNI)

case TlsContentType.Handshake:
if (!_isRenego && _handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello &&
(_sslAuthenticationOptions!.ServerCertSelectionDelegate != null ||
_sslAuthenticationOptions!.ServerOptionDelegate != null))
{
TlsFrameHelper.ProcessingOptions options = NetEventSource.Log.IsEnabled() ?
TlsFrameHelper.ProcessingOptions.All :
TlsFrameHelper.ProcessingOptions.ServerName;
// Process SNI from Client Hello message
if (!TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame, options))
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"Failed to parse TLS hello.");
}

Repro

Server:

using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Security.Cryptography.X509Certificates;
using System.Text;
var port = 5001;
var ipEndPoint = new IPEndPoint(IPAddress.Loopback, port);
var listenSocket = new Socket(ipEndPoint.AddressFamily,
                        SocketType.Stream,
                        ProtocolType.Tcp);
listenSocket.Bind(ipEndPoint);
listenSocket.Listen();
using var acceptSocket = await listenSocket.AcceptAsync();
using var ns = new NetworkStream(acceptSocket);
using var sslStream = new SslStream(ns);
using var store = new X509Store("My", StoreLocation.CurrentUser, OpenFlags.ReadOnly);
using var certificate = store.Certificates.Find(X509FindType.FindBySubjectName, "localhost", false)
                  .Where(c => c.HasPrivateKey)
                  .FirstOrDefault();

// Specifying a delegate instead of directly providing the certificate works
var sslOptions = new SslServerAuthenticationOptions
{
    ServerCertificate = certificate,
//  ServerCertificateSelectionCallback = (_, name) => certificate,
    CertificateRevocationCheckMode = X509RevocationMode.NoCheck
};
sslStream.AuthenticateAsServer(sslOptions);
Debug.Assert(!string.IsNullOrEmpty(sslStream.TargetHostName));
await sslStream.WriteAsync(Encoding.UTF8.GetBytes("Hello, World!"));

Client: curl.exe -k https://localhost:5001

👀 @wfurt

Author: shirhatti
Assignees: -
Labels:

area-System.Net.Security, untriaged

Milestone: -

@ManickaP
Copy link
Member

Triage: we should either fix this or document the behavior.
@shirhatti how important is this for you? Is this something critical for 6.0?

@shirhatti
Copy link
Contributor Author

Not important.

@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Aug 10, 2021
@ManickaP ManickaP added this to the Future milestone Aug 10, 2021
@wfurt wfurt modified the milestones: Future, 7.0.0 Nov 3, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2021
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.

3 participants