WebSockets - custom validation of server certificate

[tintoy]

Hey, @brendanburns, as previously discussed in tintoy/dotnet-kube-client#6 I'm happy to contribute my WebSocket functionality (which includes support for custom verification of the server certificate).

But I figured it'd be worth discussing here first :)

The big gotcha (currently) is that it has a dependency on netcoreapp2.1 because in previous versions of corefx WebSockets used WinHTTP on Windows (which didn't honour a subset of the certificate options on ClientWebSocketOptions) and something else on OSX / Linux (which didn't honour a different subset of the certificate options). In 2.1, the WebSockets implementation is entirely implemented in managed code and fully supports customisation of SSL certificate usage / validation.

As it stands, here's how I implemented it:

https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient.Extensions.WebSockets/K8sWebSocketOptions.cs#L13
https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient.Extensions.WebSockets/K8sWebSocket.cs#L31

I borrowed some of the original ClientWebSocket code from .NET Core 2.0 and modified it to work with 2.1-preview1 (but from 2.1-preview2 onwards, the ManagedWebSocket types will be publicly available and much of this code can be eliminated).

The latest code from corefx is a fair bit simpler because the required managed types are available internally:

https://github.com/dotnet/corefx/blob/f1f3b3f8af21c0f782f7f0699d505a0cdb9026ba/src/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs#L71

[tintoy]

So it might be worth placing in a separate package (as I did in my client) that extends the client to add operations that are WebSocket-dependent.

[tintoy]

CC: @qmfrederik

[qmfrederik]

It's a pitty the WebSockets implementation was not complete in .NET Core 2.0 but the good news is it is being fixed in 2.1.

My 2 cents would be that it's:

• Not a problem to depend on netcoreapp2.1; I think people will move to netcoreapp2.1 fairly fast. The library can multitarget 2.0 and 2.1 with a warning that if you want to use WebSockets, you need 2.1.
• Worth getting the WebSocket fixes in CoreFX rather than copy/pasting code, though I know it can be a lengthy process (especially the certificate validation callback seems to be low prio over there).

Not sure whether there are any "One Microsoft" strings @brendanburns can pull that would bump the prio of WebSockets in CoreFX 😄 .

[tintoy]

Heh, yes - I've been following an issue in the corefx repo (dotnet/corefx#12038 is how @brendanburns found my project in the first place) where they've been looking at implementing the validation callback. I I think they'd like to get it in for 2.1 (so here's hoping).

Yeah that's a good point, I hadn't thought of that - can always use #if / #endif to keep the WebSockets code available in 2.1 only.

Only other wrinkle is that, by targeting netcoreapp rather than netstandard (netstandard2.1 seems to still be further in the future), the library can't be used on the full .NET Framework / Mono (but again, multi-targeting and conditionals can probably help there).

[qmfrederik]

I think it's dotnet/corefx#12038 you're referring to.

[tintoy]

Yep, thanks! Edited :)

[tintoy]

https://github.com/dotnet/corefx/issues/12038#issuecomment-370796142

[qmfrederik]

... that issue is marked as "api-ready-for-review"; so the next step would be for the API review team to give a formal go for the API. You may want to ping @karelz to figure out what the next steps are (i.e. when a review is planned). Once the API is greenlighted you can file a PR.

[karelz]

We meet every & steam API reviews every Tuesday (see list). However, technically speaking, we are API frozen for .NET Core 2.1 for last week. If you can chime in on importance of the scenarios (and how mainline they are), we might try to get it in ... otherwise post-2.1 if that works.

[tintoy]

Hi - obviously I'm biased, but... 😉

Being able to control validation of the server certificate is pretty important for Websockets (at least the way we use them). Otherwise they're unusable with servers that have self-signed certificates unless the user goes through the rigmarole of trusting the CA certificate (needs admin privileges on Windows from memory and probably on Linux too).

Having said that, we at least have a workaround in the worst case (temporarily duplicate the corefx code for initiating the connection and add support there until support is added to corefx).

[tintoy]

Is there someplace in particular I should be chiming in? 😊

[tintoy]

@karelz oh, and I suspect the same issue will bite anyone using SignalR (over WebSockets) for inter-server communication (i.e. with the managed HubConnection client) with self-signed certificates. As mentioned above, there are workarounds available but they're pretty ugly.

[tintoy]

Relevant issue for SignalR: aspnet/SignalR#1389 (comment)

[brendandburns]

Personally, I'd love to see this in 2.1, but regarding the general issue, I think that supporting DotNet standard framework is important, so let's see what we can do with conditionals/extensions.

My proposal would be:

1. Requiring net-core 2.1 seems fine. There's no reason for someone not to upgrade

2. Enabling net-standard (at the expense of this funcitonality) is also desirable.

And of course the simplest solution (conditionals vs alternate extension package) is the right choice assuming we satisfy those goals.

Thanks!
--brendan

[tintoy]

Ok, I'll work up a PR then :)