search

mgravell / Pipelines.Sockets.Unofficial

.NET managed sockets wrapper using the new "Pipelines" API
Other
400 stars 51 forks source link

TLS/SSL support #46

Open Lanayx opened 4 years ago

Lanayx commented 4 years ago

Hi! Can we expect SSL/TLS support? I've googled this SO question, and looks like the proposed workaround can be generic and included in this library.

mgravell commented 4 years ago

The library includes bridges between pipelines and arbitrary streams, so my recommendation would be : just wrap SslStream in a pipelines bridge. This is exactly what we do in SE.Redis. This approach:

Lanayx commented 4 years ago

Yes, this makes sense, but isn't this wrapping a standard thing that will work for anyone? SocketConnection.ConnectAsyncis a convenient method that hides Socket creation, so SslStream wrapping could be hidden as well. SslStream configuration options could be passed through parameters. Of course I can copy the SE.Redis implementation as well as any other developer that needs it, but wouldn't it be better to keep it in common place?

mgravell commented 4 years ago

TLS config is so complex that frankly I don't see the advantage of trying to describe it as an API: it would be re-exposing every conceivable option that already exists on SslStream. Plus: it probably makes sense to go Socket=>NetworkStream=>SslStream=>(bridge), so pipelines isn't really involved until after you have done that.

There might be some potential as an ASP.NET config style, perhaps? But...

Tell you what: can you tell me what you think this API would look like to be useful, so we can picture it?

Lanayx commented 4 years ago

First, I want it to be transparent, just like in SE.Redis example, where we get IDuplexPipe no matter is it secure connection or normal. Second, as for api we need 3 user parameters for SslStreamcreation, and additional 3 parameters for AuthenticateAsClient (btw why is it not AuthenticateAsClientAsync?). All of them could be joined in a class (like SslOptions with fields SslStreamOptions and AuthenticateOptions) and passed as an additional optional parameter to SocketConnection.ConnectAsync.

Drawaes commented 4 years ago

I think that is fine if you only ever want an SslStream shim, but I would say that way lies madness to just grab their API it's a mess of legacy and crazy.

Lanayx commented 4 years ago

Ok, let the parameter be called SslShimOptions =) But looks like using this shim is the only way to get the job done as for today.