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

Support SNI via config #15144

Closed
Narnain opened this issue Oct 18, 2019 · 15 comments
Closed

Support SNI via config #15144

Narnain opened this issue Oct 18, 2019 · 15 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel feature-yarp This issue is related to work on yarp
Milestone

Comments

@Narnain
Copy link

Narnain commented Oct 18, 2019

I have configured SNI SSL by coding the certificate selector in the kestrel configuration. I would like to do this entirely through the config file.

It would be great if it was as simple as adding more endpoint configs each defining their url, host, port and scheme and the file or store cert definitions; so no changes would be required to the current config structure.

At the moment it appears that if two endpoints using the same port are defined in the config then only one is active.

@Tratcher Tratcher added enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel labels Oct 18, 2019
@Tratcher
Copy link
Member

At the moment it appears that if two endpoints using the same port are defined in the config then only one is active.

Kestrel isn't capable of sharing ports across endpoints, instances, apps, etc.. For that you need to use HttpSys.

It would be great if it was as simple as adding more endpoint configs each defining their url, host, port and scheme and the file or store cert definitions; so no changes would be required to the current config structure.

SNI config wouldn't be done by the aggregation of multiple endpoints, it would be adding additional certs to an existing one. This would be possible to build, but it'd be complicated.

Estimated cost/complexity: Medium to Large.

@analogrelay
Copy link
Contributor

SNI config wouldn't be done by the aggregation of multiple endpoints, it would be adding additional certs to an existing one.

@Tratcher is your suggestion here that we support a more first-class way to do this instead of the existing callback we have? My reading is that the callback would be sufficient to do this.

@Tratcher
Copy link
Member

The callback is OK, but you have to code up the config bindings yourself. The customer ask is to wire up the callback automatically based on config. That config would have to be pretty complex/flexible.

@analogrelay analogrelay added this to the Backlog milestone Oct 23, 2019
@halter73
Copy link
Member

halter73 commented Nov 6, 2019

This looks like a dupe of #4749 which is also backlogged pending more customer demand. We should probably close one of these.

@halter73 halter73 added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label Nov 6, 2019
@Tratcher Tratcher removed the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label Nov 6, 2019
@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2019

I closed the other one since this one is from a customer.

@Narnain
Copy link
Author

Narnain commented Nov 6, 2019

At the moment it is possible to configure SSL in kestrel via config.

"Kestrel": {
    "Limits": {
      "MaxConcurrentConnections": 100,
      "MaxConcurrentUpgradedConnections": 100
    },
    "DisableStringReuse": true,
    "Endpoints": {
      "Http": {
        "Url": "http://localhost:5000"
      }
      "HttpsInlineCertStoreLocaltest": {
        "Url": "https://*.localtest.me:5001",
        "Certificate": {
          "Subject": "*.localtest.me",
          "Store": "My",
          "Location": "LocalMachine",
          "AllowInvalid": "true"
        }
      }
    }

The config I would like to use is:

"Kestrel": {
    "Limits": {
      "MaxConcurrentConnections": 100,
      "MaxConcurrentUpgradedConnections": 100
    },
    "DisableStringReuse": true,
    "Endpoints": {
      "Http": {
        "Url": "http://localhost:5000"
      }
      "HttpsInlineCertStoreLocaltest": {
        "Url": "https://*.localtest.me:5001",
        "Certificate": {
          "Subject": "*.localtest.me",
          "Store": "My",
          "Location": "LocalMachine",
          "AllowInvalid": "true"
        },
        "HttpsInlineCertStoreLocalhost": {
        "Url": "https://localhost:5001",
        "Certificate": {
          "Subject": "localhost",
          "Store": "My",
          "Location": "LocalMachine",
          "AllowInvalid": "true"
        }
      }
    }

which would allow both certs to be used on the same port.

This is currently achieved by the following code added to the ConfigureKestrel method:

serverOptions.Listen(IPAddress.Loopback, 5001, listenOptions =>
                            {
                                listenOptions.UseHttps(httpsOptions =>
                                {
                                    var certs = new Dictionary<string, X509Certificate2>(StringComparer.OrdinalIgnoreCase)
                                    {
                                        ["localhost"] = CertificateLoader.LoadFromStoreCert("localhost", "My", StoreLocation.CurrentUser, allowInvalid: true),
                                        ["localtest.me"] = CertificateLoader.LoadFromStoreCert("localtest.me", "My", StoreLocation.LocalMachine, allowInvalid: true)
                                    };

                                    httpsOptions.ServerCertificateSelector = (connectionContext, name) =>
                                    {
                                        name = certs.FirstOrDefault(c => name.EndsWith(c.Key)).Key;
                                        if (name != null && certs.TryGetValue(name, out var cert))
                                        {
                                            return cert;
                                        }
                                        return null;
                                    };
                                });
                            });

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2019

The config I would like to use is:

That doesn't look quite right, did you copy too many lines?

@Narnain
Copy link
Author

Narnain commented Nov 6, 2019

The desired config should read as

  • Allow Http for localhost on port 5000
  • Allow Https for localhost with cert in My/LocalMachine on port 5001
  • Allow Https for *.localtest.me with cert in My/LocalMachine on port 5001

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2019

Ah, I didn't notice the difference between the urls on those two examples.

That url field is used to create the socket, not as part of certificate negotiation. I picture something more like this:

"Kestrel": {
    "Limits": {
      "MaxConcurrentConnections": 100,
      "MaxConcurrentUpgradedConnections": 100
    },
    "DisableStringReuse": true,
    "Endpoints": {
      "Http": {
        "Url": "http://localhost:5000"
      }
      "Https": {
        "Url": "https://*:5001",
        "Certificates": {
          "localhost": {
            "Subject": "localtest",
            "Store": "My",
            "Location": "LocalMachine",
            "AllowInvalid": "true"
          }
          "*.localhost.me": {
            "Subject": "*.localtest.me",
            "Store": "My",
            "Location": "LocalMachine",
            "AllowInvalid": "true"
          }
        }
      }
    }

@Narnain
Copy link
Author

Narnain commented Nov 8, 2019

That looks easier to read and for me solves the same problem.

@analogrelay analogrelay added good first issue Good for newcomers. and removed good first issue Good for newcomers. labels Nov 8, 2019
@analogrelay
Copy link
Contributor

@Tratcher this design seems good to me and is relatively clear. The only change I'd make is that we probably don't need the Certificates value to be a dictionary since the Subject is the "key" already. Perhaps just:

"Kestrel": {
    "Limits": {
      "MaxConcurrentConnections": 100,
      "MaxConcurrentUpgradedConnections": 100
    },
    "DisableStringReuse": true,
    "Endpoints": {
      "Http": {
        "Url": "http://localhost:5000"
      }
      "Https": {
        "Url": "https://*:5001",
        "Certificates": [
          {
            "Subject": "localtest",
            "Store": "My",
            "Location": "LocalMachine",
            "AllowInvalid": "true"
          },
          {
            "Subject": "*.localtest.me",
            "Store": "My",
            "Location": "LocalMachine",
            "AllowInvalid": "true"
          }
        }
      }
    }

So the work here is basically to add a "Certificates" property to the [EndpointConfig]9https://github.com/aspnet/AspNetCore/blob/845e6d5512c3521a07d7fa93737b0da041a8a3c6/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs#L153) and then flow all that forward and (if set) use it to build a callback that selects a cert based on the subject name

We'll have to process wildcards properly (i.e. match *.example.com when subdomain.example.com is requested).

@analogrelay analogrelay added help wanted Up for grabs. We would accept a PR to help resolve this issue and removed help wanted Up for grabs. We would accept a PR to help resolve this issue labels Nov 8, 2019
@Tratcher Tratcher removed this from the Backlog milestone Apr 23, 2020
@Tratcher
Copy link
Member

Bumping for re-triage. Proxy will want some form of this.

@analogrelay analogrelay added the feature-yarp This issue is related to work on yarp label Apr 24, 2020
@analogrelay analogrelay added this to the Next sprint planning milestone Apr 24, 2020
@analogrelay
Copy link
Contributor

As per some discussions in YARP, it might be a good idea to have an option to scan the Windows cert store for candidate certs an automatically select one. That's not suitable to completely solve this issue since it only applies on Windows, but it could be a convenient approach since it means the only thing you need to do to install/rotate/etc. a cert is make sure it's in the store and valid.

This was discussed in microsoft/reverse-proxy#86

A key aspect is that the system would load all reasonable certificates from the Windows store and then select the best certificate. Where "reasonable" and "best" are defined by:

  • Definition of reasonable cert: Similar to Kestrel's existing logic:

    • 1.3.6.1.5.5.7.3.1 Enhanced Key Usage oid when the extension is present
    • Private key is available
    • Additional validity + revocation checks. we call X509Certificate2.Verify() to also check for revocation, whereas X509CertificateStore.Find(... validOnly: true) (used in Kestrel's defaults) does not check revocation.
  • Definition of best cert: The most recently-issued certificate that is reasonable.

@Tratcher
Copy link
Member

@anurse that seems like a pretty different feature from this one, we should track it separately.

@BrennanConroy
Copy link
Member

Fixed via #24286

@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2020
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel feature-yarp This issue is related to work on yarp
Projects
None yet
Development

No branches or pull requests

7 participants