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

Move the SSL logic out of Gambit, as to keep Gambit lean and secure. (Prerequisite: Userland ports) #285

Open
adam-7 opened this issue Jul 25, 2017 · 9 comments

Comments

@adam-7
Copy link

adam-7 commented Jul 25, 2017

OpenSSL is a compromised, broken, filthy API with an even worse code base and track record of security vulnerabilities, and issues like #281 will go on and on.

Additionally also because of its wealth of possible failure conditions, I suggest that having OpenSSL logics inside Gambit (both its C and Scheme code) is utterly inappropriate.

It is by no means surprising if the SSL logics would imply code execution vulnerabilities, and any default configuration option for Gambit compilation must hence have the SSL disabled.

The proper thing to do is to move SSL logics out of the Gambit codebase as we're talking about thousands of lines of code and maintenance complexity to keep it going.

This can be done conveniently as soon as there are userland ports - for these refer to this issue: #284 .

This issue can be used to track the progress of moving the SSL logics out.

@vyzo
Copy link
Contributor

vyzo commented Jul 25, 2017

I think you exaggerate the effect of OpenSSL in gambit's security disposition. If you don't open tls sockets, then you don't actually use openssl anywhere.
Plus it's very nice to have a portable builtin implementation of SSL sockets, even if there are bugs.

The issue you quote (#281) was caused by some left-over debugging code wreaking havoc, PR already open that fixes it (#282).

@adam-7
Copy link
Author

adam-7 commented Jul 25, 2017

@vyzo well yes, you are right, merely linking to OpenSSL does not imply remote code execution holes.

But, any use of OpenSSL may, and Gambit does not contain any warnings for known exploited versions of OpenSSL, and future versions of OpenSSL are likely to be full of holes, also.

Next, using OpenSSL in a way that's cryptographically safe, also requires lots of considerations, and also per-OpenSSL version considerations, which are all way too much for Gambit to track.

Gambit's general policy is to not have external dependencies, so I suggest also not having built-in code that depends on OpenSSL would be the most consistent approach as it's one of the hairier libraries there are.

When Gambit has userland ports, then that system could include facilities for failure states and callbacks, that would help users operate SSL connections safely. I think that is totally out of scope presently, and hence the current SSL implementation has a too high "quick and dirty" factor to it. Better just remove it and run "stunnel" pipes or something.

@vyzo
Copy link
Contributor

vyzo commented Jul 25, 2017

I am happy Gambit contains the functionality for SSL sockets now; that means I can easily do https/wss requests.

@adam-7
Copy link
Author

adam-7 commented Jul 25, 2017

Let's follow up on this one when we have good userland ports (#284) in place.

Until then last comments from my side on this one:

A separate Scheme+C SSL channels module exists, also usable now, it works really well.

"Now" support doesn't justify it being in Gambit.

XMPP and, I think, WSS - that is SSL-secured WebSockets -, provide excellent examples of why SSL must be implemened as a userland port: XMPP (and I think WSS) has some separate handshake logics going on before you move the TCP connection to be deployed as an SSL channel.

The proper way to do this is to do the XMPP(/WS) handshake using normal userland code, for instance as an XMPP(/WS) userland port, which does the handshake and then sets up an SSL userland port and itself goes into passthrough mode with respect to the underlying port, and then adding a layer on top of that SSL userland port, being user-facing IO to deliver the XMPP data and post-encoding/decoding channel content for the WS.

The current absence of userland ports leave all this in a total mess and you're lucky if you can hack something together enough even for your own usecase, and you're also lucky if you have any use of Gambit's built-in SSL logics as their scope of use is incredibly limited.

So in either case you are forced into improvising userland ports and suffering the consequence of different improvised userland ports not interoperating.

Using OpenSSL is really complex.

There are out of band aspects like domain validation of HTTPS certificates, and how to use OpenSSL in a way that is cryptographically safe, is a moving target.

Gambit having built-in OpenSSL integration be fine for quick and dirty use may, but that's all.

What I see is that OpenSSL is waaay to hairy to justify deep Gambit code giving any consideration to it.

Right now, the SSL reading and writing logics are just an overlay piece that's inside Gambit's own TCP read/write (

if (d->tls)
,
if (d->tls)
). That that is possible and hence works with a small code footprint, is only coindicental and cannot be expected. These SSL logics only support usage directly atop a TCP connection, for instance. Any other use needs a separate SSL implementation. A single user port implementation could be optimized for both usecases.

A solution to #286 is needed for Gambit's IO subsystem to deliver HTTP(s) and WS(S) well as they involve both text parts (HTTP headers etc.) and binary parts (binary content passed over the protocol).

@alvatar
Copy link
Contributor

alvatar commented Jul 26, 2017

@adam-7 I agree that a separate implementation on to of userland ports is more flexible. It's obvious that anything that can be implemented as a library can bring more control to the user. I think Gambit should aim in that direction.

But also, I disagree that having the current implementation is not useful. Not just for "quick and dirty", but actually supporting 99% of TLS uses. If you want to do some advanced tunneling or tweak some things, you might need to change that code, but if you need to do what a regular client or server does, you don't need that.
However, this code is not even enabled by default, and is clearly isolated, so having this meanwhile is not going to hurt Gambit, but rather help users that want to do a regular usage of TLS on Gambit.

OpenSSL "moving target" as you mention basically boils down to adding patches to new vulnerabilities, most of the time consisting in just updating the version. Sometimes, that requires setting a specific flag in the code. The API is not a moving target.

@adam-7
Copy link
Author

adam-7 commented Jul 27, 2017

Yes sure it's nice.

Re the API not being a moving target, right, https://abi-laboratory.pro/tracker/timeline/openssl/ , there were some changes but not so many. I wonder when the DHparams was introduced, I can't remember seeing it mentioned anywhere a long time ago.

Complexity like the need to configure which protocol versions and algos to accept, callbacks for domain validation, session cache management, and other configuration and failure handling that is specified as values or via callbacks, such as manual provision of certificates, make a serious implementation quite complex anyhow.

@feeley
Copy link
Member

feeley commented Jul 27, 2017

I agree with alvatar and vyzo that it is good for Gambit to have an SSL implementation that can easily be enabled, and moreover with a lib that has widespread acceptance and maintenance. I'm grateful alvatar put the effort into this, as it was sorely lacking from Gambit.

How would you "move the SSL implementation out of Gambit" with userland ports? And how would this avoid OpenSSL?

@adam-7
Copy link
Author

adam-7 commented Jul 27, 2017

Yes the in-Gambit SSL is as such well-implemented and good for convenience.

Most of my comments are from in-depth tinkering with implementing SSL channels and bringing an SSL server to "A+" security test grade. It was a major facepalm experience that took in the range 7 weeks.

How would this avoid OpenSSL?
With a userland SSL port, OpenSSL would not by necessity be avoided, but, it would be worthwhile to track unsecure versions (of which there are many e.g. https://en.wikipedia.org/wiki/OpenSSL#Notable_vulnerabilities ).

LibreSSL is a secure/security-enhanced fork of OpenSSL, which is distributed under OpenSSL's default library name on OpenBSD, and comes under alternative paths or library names on other platforms.

How would you "move the SSL implementation out of Gambit" with userland ports?
The userland SSL port would deliver a port (for communicating with the user/decrypted data), and abstract on an underlying port (for the encrypted communication).

It would detect whether the underlying port is actually a TCP connection. If not, the OpenSSL API's BIO (binary-IO) mode would be used. If so, possibly the underlying port's OS socket could be borrowed, and the OpenSSL API's normal socket-based IO would be used, saving us some data copying.

The major feature with an OpenSSL userland port would be that it abstracts well as in, it works well in a sandwhiched configuration. The two primary usecases for that would be

  1. when you want to run your SSL channel over something else than a TCP connection (such as, within a SOCKS5- or SSH-multiplexed connection or a serial port), and
  2. when you want to run a handshake in plaintext over a data channel (be it a TCP connection or not) before going into running the SSL channel on it (depends on Make the IO subsystem allow arbitrary live switches between binary and various text encoding modes, and between line endings #286 ).

The secondary utility of SSL logics being a separate module, is size and complexity.

Running an SSL server competently is much more complex than running an SSL client.

Complexity involves:

  • Specifying which SSL/TLS versions you are OK with
  • Specifying which crypto algorithms you are OK with
  • Specifying manual SSL certificates, sourced from file or object
  • Specifying domain name validation logics - are you OK with an invalid SSL certificate, if so which/how
  • Server only or mostly: Implement/specify a SSL renegotiations policy
  • Server only: Providing the logics to deliver multiple SSL certificates for multiple domains, over one single SSL server, that's called SNI
  • Server only: Specifying your DHparms
  • Server only: Generate a SSL certificate

Last, I see two potential problems about how the SSL data channel is run:

  1. SSL_write/SSL_read have weird semantics. For instance, SSL_write/read can tell you to run a zero-byte SSL_read/write operation, to drive the library's handshake. The actual meaning of SSL_write/read combined with SSL error codes is fairly intricate. This could be contradictory to an implementation model where you use SSL_read/write in a place where you otherwise are recycling the code for OS read/write().

  2. This one is a bit beyond my knowledge, but, my best understanding is that an SSL channel can fail in different ways, and that certain out-of-band callbacks are possible.

These two add up to a place where you may need complex logics both to drive the SSL data channel when it works, and also to a place where you need to have a well-defined convention for OOB events.

@alvatar
Copy link
Contributor

alvatar commented Jul 27, 2017

@adam-7 I also work with servers and TLS and while I agree that at some point you might need to tune some of that stuff for solid network programming, if you look out there most servers supporting TLS don't provide such configurability and are still very useful. Morover, this implementation is extendable, as many of those are easily built onto current design, as they are just C flags that can be exposed as keywords, may you really need to touch.

The weird semantics of SSL_write/read are taken into account, and one-line tweak in Gambit was required to support it (that isn't activated unless SSL is compiled IRCC). I don't remember the details about the second point.

I think in a nutshell, the implementation is by no means comprehensive, but my general opinion on the topic is:

  1. The implementation is already useful for many cases. Most servers and libraries out there don't expose the full API OpenSSL either.
  2. The support is optional, may you prefer to leave it out and keep Gambit lean or prefer to use your own layer now (IRCC you'd need to copy the data when crossing the library boundary in this case, and server CPU and memory usage tends to suffer from TLS, with this making it worse).
  3. This doesn't prevent the option to work on userland ports and bring the solution you propose, as it is indeed more flexible (but is currently not implemented, and there are more lines or work in Gambit that are also important, like SMP). In an ideal future, a userland implementation exists that is as performant or nearly as performant as the embedded one, with multiple implementations on top LibreSSL, BoringSSL, NSS, GnuTLS or a library in Rust.

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

4 participants