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

Add TLS middleware with sample #6035

Merged
merged 1 commit into from
Oct 15, 2019
Merged

Conversation

ReubenBond
Copy link
Member

This PR adds support for securing client-to-silo and silo-to-silo connections using mutual-TLS.
Adds several UseTls methods to ISiloBuilder, IClientBuilder, and ISiloHostBuilder.

TLS can be configured on the silo like so:

var host = new HostBuilder()
    .UseOrleans((context, siloBuilder) =>
    {
        var isDevelopment = context.HostingEnvironment.IsDevelopment();
        siloBuilder
            .UseLocalhostClustering(serviceId: "HelloWorldApp", clusterId: "dev")
            .UseTls(StoreName.My, "fakedomain.faketld", allowInvalid: isDevelopment, StoreLocation.LocalMachine, options =>
            {
                if (isDevelopment)
                {
                    options.AllowAnyRemoteCertificate();
                }
            });
    })
    .ConfigureLogging(logging => logging.AddConsole())
    .Build();

By default, when the UseTls extension method is used, both sides of all connections are required to present a certificate.

This adds a new package: Microsoft.Orleans.Connections.Security which targets netcoreapp3.0 only and therefore .NET Core 3.0 is required to use this functionality.

A sample application is included under Samples\3.0\TransportLayerSecurity.

There are some rough edges here which I would especially like feedback on. For example, a TargetHost must be set whenever a connection is made. I would like to know how users would expect to set this value. In the provided sample the value is set to a fixed value:

.UseTls(StoreName.My, "fakedomain.faketld", allowInvalid: true, StoreLocation.LocalMachine, options =>
{
    options.OnAuthenticateAsClient = (connection, sslOptions) =>
    {
        sslOptions.TargetHost = "fakedomain.faketld";
    };
    // NOTE: Do not do this in a production environment
    options.AllowAnyRemoteCertificate();
})

I understand that this will not suffice for all cases. The connection parameter has a Features property which allows the users to determine which endpoint is being connected to (among other things). This could be used to determine the correct TargetHost value.

Fixes #828

xref dotnet/aspnetcore#12809

/// <summary>
/// Settings for how TLS connections are handled.
/// </summary>
public class TlsConnectionAdapterOptions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't replicate the Adapter naming mistake we made 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Https/Tls 😅
Perhaps TlsOptions is a nicer name for this, or TlsConnectionOptions

@ReubenBond ReubenBond force-pushed the feature/tls branch 4 times, most recently from a3969d1 to 4ffeb45 Compare October 10, 2019 22:50
@JanEggers
Copy link

@ReubenBond can you please let me know when there is a preview that includes this. I would like to use the TLS middleware for this: https://github.com/chkr1011/MQTTnet/tree/master/Source/MQTTnet.AspnetCore

@ReubenBond
Copy link
Member Author

@JanEggers you can take the current commit if you like - that MQTT project doesn't reference Orleans & the important bits are internal so I don't think a build of this would be useful

@JanEggers
Copy link

JanEggers commented Oct 14, 2019

@ReubenBond thx for letting me know, I didnt look at the code in detail yet. I was hoping I could just use the extension method on IConnectionBuilder and be done.

@ReubenBond
Copy link
Member Author

@JanEggers eventually yes, but those things are marked as internal in this PR because the user isn't supposed to interact with them. The intention it to upstream this to Bedrock eventually

@sergeybykov sergeybykov merged commit fd546a0 into dotnet:master Oct 15, 2019
@ReubenBond ReubenBond deleted the feature/tls branch October 15, 2019 20:56
@RehanSaeed
Copy link

Is it possible to configure TLS using IConfiguration similar to the way HTTPS can be configured for Kestrel?

{
  "Kestrel": {
    "Endpoints": {
      "Http": {
        "Url": "http://localhost:5000"
      },

      "HttpsInlineCertFile": {
        "Url": "https://localhost:5001",
        "Certificate": {
          "Path": "<path to .pfx file>",
          "Password": "<certificate password>"
        }
      },

      "HttpsInlineCertStore": {
        "Url": "https://localhost:5002",
        "Certificate": {
          "Subject": "<subject; required>",
          "Store": "<certificate store; required>",
          "Location": "<location; defaults to CurrentUser>",
          "AllowInvalid": "<true or false; defaults to false>"
        }
      },

      "HttpsDefaultCert": {
        "Url": "https://localhost:5003"
      },

      "Https": {
        "Url": "https://*:5004",
        "Certificate": {
          "Path": "<path to .pfx file>",
          "Password": "<certificate password>"
        }
      }
    },
    "Certificates": {
      "Default": {
        "Path": "<path to .pfx file>",
        "Password": "<certificate password>"
      }
    }
  }
}

@ReubenBond
Copy link
Member Author

The current implementation does not explicitly add support for that, but it doesn't stop you from using IConfiguration yourself. This could be a good area for contribution

@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding optional TLS/encryption to transport channel
5 participants