Implement server name indication (SNI) for OpenSSL and SecureTransport backends (new property IceSSL.ServerNameIndication, default disabled)

[AndiDog]

Add property IceSSL.ServerNameIndication to send the SNI extension in TLS connections (currently only in OpenSSL/SecureTransport backends, defaults to 0). This is helpful, for example, if a web server in front of Ice secure WebSocket (WSS) endpoints needs to decide the certificate and backend based on requested hostname.

[zcabot]

Contributor requirements have been met.

[bentoi]

Thanks Andreas for this pull request.

When possible, we prefer to provide the same features with all SSL backends. It looks like it should be possible to support this with SChannel and Java. C# might also enable it by default (at least with .NET Core, https://github.com/dotnet/corefx/issues/17427... it's not clear for .NET Framework).

Does this actually need to be configurable? What are the backward compatibility concerns if we enable this all the time? I suppose a client could break if it gets a different certificate when SNI is enabled and if the verification of this certificate fails.

[AndiDog]

Makes sense! I am continuing with remaining backend implementations and have already got Java working.

Given that certain backends might not allow disabling SNI, and it's a backward-compatible TLS extension (only used if server supports it), I agree that we can go without the property-based configuration. Wondering if the openssl implementation should then throw or warn in case the SNI host could not be set?

[bentoi]

I'm on the fence as whether or not we should keep the property... even if SNI is a back compatible extension, it can change the server behaviour (return a different cert) and eventually break an existing application that upgrades to 3.7.3.

So for 3.7, it's probably best to keep the property and remove it in the next major Ice version...

If we have a property to enable this, best is to throw if the SNI name can't be set with OpenSSL.

[AndiDog]

Just tested on Windows (SChannel) and it sends the SNI extension by default, with no documented way to disable it (InitializeSecurityContext function). We should document in the CHANGELOG that IceSSL.ServerNameIndication=1 will become the default in Ice 3.8 for backends which didn't have it implicitly turned on before.

So far I have cpp (openssl, SecureTransport, SChannel) and java covered. I assume my remaining tasks will be cpp-uwp, csharp and java-compat. Any others to consider – don't know by heart how js/objectice-c/php/python/ruby/swift work (some probably use the C++ library)?

[bentoi]

I'm surprised SChannel sends it all the time as we don't set SCH_CRED_SNI_CREDENTIAL for the crendential object used with InitializeSecurityContext. According to this post, you have to set this flag to enable SNI.

I don't think we need to fix the UWP transport. For .NET, I didn't find a way to enable/disable SNI with the API. My understanding is that it's enabled with latest .NET Core versions but I don't know if it is with the .NET Framework. Java-compat should be fixed but other than that I don't think there are other SSL backends that need fixing.

Also, after discussing this with my colleagues, we were thinking it would be best to keep a property for 3.7 but change the default to 1 to enable it by default. This way if for some reasons you get a failure because of the activation of SNI, you can still disable it. This also allows to throw if there's a failure to the set SNI name (with OpenSSL for example). For the next major release, we'll probably deprecate or remove the property.

[AndiDog]

Latest state:

• Property default changed to 1 (I agree to that, especially given that some implementations always send the extension anyway)
• OpenSSL / macOS (SecureTransport) / Java: did not send extension before, now they do based on the property, tested and working
• Windows (SChannel) / C#: they always send the extension, added code comment, tested and working (specifically, I tested C# on macOS)
• Java-Compat: feature not supported in the SSLParameters class, therefore not implemented, added code comment
• UWP: not tested (see your note above), added code comment

[pepone]

Hi Andreas,

After review you pull requests I notice that enabling SNI on macOS implies the verification of the server name, this cause conflicts with IceSSL.CheckCertName=0, we decided to get rid of IceSSL.ServerNameIndication and add a new value for IceSSL.CheckCertName , when set to 2 it will enable TLS SNI extension, besides that I just rebase your pull request to 3.7 and make a few minor style fixes.

Thanks for your contribution.

[AndiDog]

That means CheckCertName will always need to be active, and the server certificate needs to match. My intention was that certificates like CN=my-server-application could still be used independent from SNI-based host selection – e.g. connect to server.example.com but don't check that name... instead rely on IceSSL.TrustOnly.Client=CN="my-server-application" rule which is applied regardless of IceSSL.CheckCertName. That one level of security should be enough. Any chance we can bring back the separate property? Otherwise such certificates would have to be re-issued to include the DNS name(s). Didn't test if something like that works (TrustOnly.Client to check cert's CN + CheckCertName to check DNS name).

Thanks for checking and merging, in any case! 🎉

[pepone]

These certificates will not work with macOS if SNI is enabled, this is the main reason we decided to go with a single property.

[AndiDog]

I think it makes sense in the long run, you're right.

Can you foresee if there will be a 3.7.3 release with this change in the next weeks?

[pepone]

Can you foresee if there will be a 3.7.3 release with this change in the next weeks?

Yes, that is our intention, shouldn't take too long now.

[AndiDog]

To anyone using this feature: mind that Ice resolves hostnames before checking for existing connections (see Connection establishment). This means if one proxy string uses -h a.example.com and another -h b.example.com, but *.example.com all have the same IP, you might get surprises by sending B requests to A or vice versa due to connection caching. Setting a connection ID is a workaround. Probably the caching algorithm should rather compare the original hostname, not the resolved IP?!