Complaining about Trantor's SSL code and my plains to fix it

=> home

Just some quick rant about how Trantor does it's SSL stuff. For thoes who don't know. Trantor is a TCP library that I also maintain as a part of the Drogon web application framework. Personally, I think trantor is easier then boost::asio to use, and it's also more efficient. It also comes with integrated SSL support using OpenSSL or LibreSSL. However, the code for that is just.. yeiks.

I'm not just writing a blog to complain about stuff. I'm also thinking trough why the current design is bad and how I'm going to fix it. So, let's start with the current design.

=> Trantor: A non-blocking I/O tcp network lib based on c++14/17

What's wrong

Trantor is mostly a fork of Muduo. Which is only designed to handle plain TCP. Thus, the TCP server and client are abstracted and shares the same TcpConnectionImpl class that actually deals with the data stream. The relation looks more or less like the follwoing diagram.

=> Muduo - Event-driven network library for multi-threaded Linux server in C++11

    TcpServer       TcpClient
        |               |
        v               v
      (accept)       (connect)
        |               |
        -----------------
                |
                v
           TcpConnection
                |
                v
         TcpConnectionImpl
                |
                v
             Socket

This leaves no room for SSL. The solution currently is, to stick the SSL class inside a std::shared_ptr<SSLContext> inside the connection objecet. And perform different operations if that pointer is null or not. This is not only ugly, but also make code bloated, hard to read and feature check macros everywhere. I counted 7 of theses:

void TcpConnectionImpl::doSomething(...) {
#ifdef USE_OPENSSL // If SSL is enabled
    if (sslPtr_) {
    ...
    } else {
#endif

/* Plain TCP handling */


#ifdef USE_OPENSSL
    }
#endif

Hopefully I don't have to explain why this is bad. It also limits us to support one SSL library. For example, I can't transparently add Botan or GnuTLS support without adding another layer of feature checks.

And that's something I really want to do as OpenSSL doesn't have the best reputation right now.

Solution

I want to turn it into a modular design. Instead of having a SSL context pointer. SSL support shall be an abstraction. Making the instanctiation of SecureTcpConnection optional. All the wile exposing the same send() and onMessage() interface.

    TcpServer       TcpClient
        |               |
        v               v
      (accept)       (connect)
        |               |
        -----------------
                |
                v
           TcpConnection
                |-------------|
                v             |
        SecureTcpConnection   |
                |<------------|
                v
         RawTcpConnection
                |
                v
             Socket

Implementation wise, it's a daisy chain of calls. SSL encrypts the data and pass it TCP. This approach have one downside. There's no language level barrier to abuse this mechanism to wrap SSL in SSL as daisy chaning is opake to the compiler. As long as RawTcpConnection and SecureTcpConnection are derrived from the same class (they will since TcpConnection is their common interface), nothing stops you from doing this. However this is just somethig we ought to track internally. Users shouldn't see any of this.

void SecureTcpConnection::send(MsgBuffer* buf)
{
    encrypt_using_ssl(buf);
    // daisy chain to the acual TCP connection
    tcp->send(buf);
}

void TcpConnection::send(MsgBuffer* buf)
{
    // Actually send the data
    send(fd, ...);
}

One big issue, is trantor supports "delayed SSL". Any TCP client can start negoshating SSL after the connection is established. My new design doesn't support this. I hope no one uses this stupid feature.

That's the plan at least. Hopefully I can make it work without breaking public APIs.