Kestrel doesn't log now listening on correctly for https

[JunTaoLuo]

I configured the basic middleware RewriteSample with

var host = new WebHostBuilder()
                .UseKestrel(options =>
                {
                    options.Listen(IPAddress.Loopback, 5000);
                    options.Listen(IPAddress.Loopback, 5001, listenOptions =>
                    {
                        listenOptions.UseHttps("testCert.pfx", "testPassword");
                    });
                })

When I run the sample I see the output:

Hosting environment: Production
Content root path: C:\gh\BasicMiddleware\samples\RewriteSample
Now listening on: http://127.0.0.1:5000
Now listening on: http://127.0.0.1:5001
Application started. Press Ctrl+C to shut down.

the https url should be https://127.0.0.1:5001

[muratg]

@natemcmaster please ping @halter73 before you start looking into this.

[natemcmaster]

Related work:

aspnet/Hosting#968
#1519
#1495

[cesarblum]

Given #1519, I think you can close this. <- 💩

Problem is if you do

.UseKestrel(options =>
{
    options.Listen(IPAddress.Loopback, 5001, listenOptions =>
    {
        listenOptions.UseHttps("testCert.pfx", "testPassword");
        listenOptions.UseConnectionLogging();
    });
})

You still see this in the console:

Now listening on: http://127.0.0.1:5001

[natemcmaster]

I've already forgotten what we decided to do here. Was there going to be a new field on IServerAddressesFeature for scheme that Kestrel can set when appropriate? cc @JunTaoLuo @Tratcher

[Tratcher]

None of the design changes we discussed affect this issue, nor does #1519.

    options.Listen(IPAddress.Loopback, 5001, listenOptions =>
    {
        listenOptions.UseHttps("testCert.pfx", "testPassword");
        listenOptions.UseConnectionLogging();
    });

Should result in IServerAddressesFeature values being replaced with "https://127.0.0.1:5001/".
(unless IServerAddressesFeature has the new PreferHostingConfig option enabled, in which case you ignore Listen).

[natemcmaster]

Took a look at this again. Here are two options

The simple one
Make ListenOptions.Scheme public & settable. UseHttps overloads set options.Scheme = "https".

The make-an-abstraction one
Something like

public interface IServerAddressProvider
{
   // DisplayName = trying to communicate that the value is only used in console output or loggers
   string GetServerAddressDisplayName();
}

KestrelServer.Start would

1. check any ListenOptions.ConnectionAdapters that also implements IServerAddressProvider. Last one wins.
2. If none found, fallback to ListenOptions.ToString

cc @davidfowl

[cesarblum]

For the first option, internal would do it, right? Definitely don't want it to be public and settable by anything.

[cesarblum]

Second option is more flexible though. Someone could implement a Gopher adapter and set the scheme accordingly 😁 Doubt that would work 😛

[davidfowl]

Wait I forgot I had a design for this and it look nothing like the above. The idea was for connection filters to have a communication mechanism with kestrel for both scheme and protocol. When filters run and set the scheme and protocol (http, http2 when we do it).

[natemcmaster]

Attributes?

    // IHazAdapter = bag of properties about an an adapter

    [IHazAdapter(Scheme = HttpScheme.Https)]
    public class HttpsConnectionAdapter : IConnectionAdapter {}

    [IHazAdapter(Scheme = HttpScheme.Https, Protocol = Protocols.Http2)]
    public class TlsHttp2ConnectionAdapter : IConnectionAdapter {}

    [IHazAdapter(Protocol = Protocols.Http2)]
    public class RawHttp2ConnectionAdapter : IConnectionAdapter {}

    // no attribute b/c this provides neither scheme or protocol
    public class LoggingConnectionAdapater : IConnectionAdapter {}

Or, an attribute per aspect:

    [HttpScheme(HttpScheme.Https)]
    public class HttpsConnectionAdapter : IConnectionAdapter {}

    [HttpScheme(HttpScheme.Https)]
    [HttpProtocol(HttpProtocol.Http2)]
    public class TlsHttp2ConnectionAdapter : IConnectionAdapter {}

    [HttpProtcol(HttpProtocol.Http2)]
    public class RawHttp2ConnectionAdapter : IConnectionAdapter {}

[Tratcher]

A TLS connection can negotiate to use HTTP/1.1 or HTTP/2 via ALPN, it's not static.

[davidfowl]

No, not attributes. It needs to be programmatic. As @Tratcher eludes to. It's state that set during the execution of the connection filter that the ConnectionHandler reads

[natemcmaster]

Wait I forgot I had a design for this and it look nothing like the above.

@davidfowl Ok, well instead of making more blind guesses at what you are thinking, can you share your design?

[Drawaes]

I think there was a scheme and protocol setting because http1.1 can be over tcp or tls. Also there might be webrtc etc which can all be set by the tls layer. Definitely don't make it internal cause then we are "stuck" with SSLStream or what ever and I can't hack in kerb, ntlm, tls1.3 and all the cool stuff.

(Well I can but it will involve even more hacking : )