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

TLS/SSL support #46

Open
Lanayx opened this issue Oct 12, 2019 · 6 comments
Open

TLS/SSL support #46

Lanayx opened this issue Oct 12, 2019 · 6 comments

Comments

@Lanayx
Copy link

Lanayx commented Oct 12, 2019

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
Copy link
Owner

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:

  • requires zero API extension
  • works for any kind of stream
  • doesn't place any library-imposed limits on how you configure your SslStream

@Lanayx
Copy link
Author

Lanayx commented Oct 12, 2019

Yes, this makes sense, but isn't this wrapping a standard thing that will work for anyone? SocketConnection.ConnectAsync is 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
Copy link
Owner

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
Copy link
Author

Lanayx commented Oct 12, 2019

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 SslStream creation, 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
Copy link
Collaborator

Drawaes commented Oct 12, 2019

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
Copy link
Author

Lanayx commented Oct 12, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants