ssl::stream: too big memory usage

[reddwarf69]

I can get my server to handle more SSL connections before using all the RAM of my machine if I instead of using ssl::stream I use ip::tcp::socket and let a nginx reverse proxy handle the SSL.

As far as I can see the problem is the use of a BIO pair (each with its own buffer), and the lack of access to the internal buffers of these BIOs, which creates the need for yet another 17 KiB ASIO buffer pair. #440 suggests using BIO_nread to be able to access the internal buffers. But that's an undocumented function and https://github.com/openssl/openssl/blob/master/crypto/bio/bss_bio.c#L199 says WARNING: The non-copying interface is largely untested as of yet and may contain bugs.

The use of SSL_MODE_RELEASE_BUFFERS is not helping (enough). In part because of the ssl::detail::stream_core::max_tls_record_size value.
Notice that an user worried about memory usage may be using https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_tlsext_max_fragment_length.html to reduce the maximum fragment size to 512 bytes, but ASIO would still be allocating those two 17 KiB buffers.

From my understanding the best solution would be creating a custom BIO_s_asio (e.g. https://github.com/liuqun/openssl-dtls-custom-bio/blob/majipeng-20190816/cbio.c). Since ASIO would be the BIO it would have direct access to everything, with no need for extra buffers and being able to respect SSL_CTX_set_tlsext_max_fragment_length()/SSL_CTX_set_max_send_fragment. Instead of restricting itself to the BIO external interface, ASIO could be a BIO itself.

@chriskohlhoff What do you think? Should we go for custom BIO?

[reddwarf69]

I have actually been looking at this recently.

This is how openSSL uses a file descriptor usually: https://github.com/openssl/openssl/blob/33b4f731451bcd7321ddae002c7945cd83d52f78/crypto/bio/bss_fd.c#L115 (there is bss_sock.c, but bss_fd.c is basically the same and simpler to read). There is nothing stopping us from creating a custom BIO using the ASIO wrappers over such methods, we have https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/reference/SyncReadStream.html and https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/reference/basic_socket/non_blocking/overload3.html. But this is not ideal because it requires the next_layer_type to be a basic_socket (or at least to have the non_blocking() method), when for maximum generality we want to depend only on the next_layer_type being a https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/reference/AsyncReadStream.html and https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/reference/AsyncWriteStream.html. Depending only on AsyncReadStream/AsyncWriteStream is what the current code does, so changing that could break existing code.

What the existing code does is what the openSSL documentation says to do. The documentation of the BIO pairs says: "One typical use of BIO pairs is to place TLS/SSL I/O under application control, this can be used when the application wishes to use a non standard transport for TLS/SSL or the normal socket routines are inappropriate." But it has the extra buffers issue.

There is the BIO_nread/BIO_nwrite interfaces to avoid copies. But this is only giving zero-copy/raw access to the BIO buffer. There is a buffer in the SSL object itself (https://github.com/openssl/openssl/blob/33b4f731451bcd7321ddae002c7945cd83d52f78/ssl/record/ssl3_buffer.c#L121), this is the main one that can't be avoided, and then we can have buffers in BIOs (https://github.com/openssl/openssl/blob/33b4f731451bcd7321ddae002c7945cd83d52f78/crypto/bio/bss_bio.c#L624), but any buffer in a BIO is extra memory. Even in the best scenario, when using BIO_nread/BIO_nwrite the memory usage doubles.

Notice that the buffer in the SSL object itself doesn't even need to be specially big. If the user uses SSL_CTX_set_max_send_fragment() to say "look, I know it's extra CPU, but I don't have more RAM" to say it doesn't want to send SSL records containing more than 512 bytes of plain data (instead of the default of 16 KiB) the internal SSL buffer will take this into account (https://github.com/openssl/openssl/blob/33b4f731451bcd7321ddae002c7945cd83d52f78/ssl/record/ssl3_buffer.c#L100). But the BIO pair buffer size is under the user control, and unless the user (ASIO) adds the logic it defaults to a 17 KiB buffer always (https://github.com/openssl/openssl/blob/33b4f731451bcd7321ddae002c7945cd83d52f78/crypto/bio/bss_bio.c#L86). So the current BIO buffer can increase the memory usage a LOT, from ~512 bytes to ~17.5 KiB, 35 times more memory.

So this doesn't leaves us a lot of options. Either use a BIO, with its extra memory/buffers, or change the requirements of the next_layer_type in a strange way. We can't really use async_read_some/async_write_some to implement the read/write interfaces of a BIO because those interfaces expect a result immediately and async_read_some/async_write_some promise not to call the completion handler until after they return. What we would need is for openSSL to offer an interface more similar to the one from ASIO, one with a function simply "requesting" the I/O and then a "completion notification" callback to tell openSSL when the read/write has been done. Adding such an interface to openSSL would probably be quite some work.

Now... I have an idea. It's a hack, it requires openSSL to make a promise that I think it currently fullfills but is not written down anywhere. If we were to do this we would need to ask openSSL to write it down, to actually make the promise. This is the idea: let's make the BIO_read() (and BIO_write()) both the "requesting" function and the "completion notification" function.

1. When you call SSL_read_ex() it's going to call BIO_read(), I propose to make this first call always return SSL_ERROR_WANT_READ. It should also store the received buffer pointer and size parameters it received somewhere. This would be the "requesting" part.
2. After this I propose calling async_read_some() with those buffer pointer/size values we received before.
3. When async_read_some() completes let's call SSL_read_ex() again, which will call BIO_read() again. The read has actually already been done, so this call will return "success" with the number of bytes async_read_some() did read before. This would be the "completion notification" part.

There would be a new BIO involved here. But the BIO would not have any ASIO logic. Its only job would be to facilite the communication between openSSL and some external entity doing the I/O outside of openSSL control.

The promise we need openSSL to make here is make those buffer pointer/size arguments passed to BIO_read (and BIO_write()) outlive the function calls. That buffer information that exists in 1 needs to still be valid in 3.

Here I attach sample code of the idea. It's ugly, broken and only implements a SSL_connect() (which does both BIO_write and BIO_read), but it actually works and avoid the extra buffers:

#include <stdio.h>
#include <memory.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <openssl/crypto.h>
#include <openssl/x509.h>
#include <openssl/pem.h>
#include <openssl/ssl.h>
#include <openssl/err.h>
#include <boost/asio.hpp>
#include <openssl/bio.h>
#include <vector>
#include <boost/asio/yield.hpp>


struct BIO_asio_st
{
    bool read_done;
    bool write_done;
    char *read_data;
    const char *write_data;
    size_t read_dlen;
    size_t write_dlen;
};

static int BIO_s_asio_read_ex(BIO *b, char *data, size_t dlen, size_t *readbytes)
{
    struct BIO_asio_st* ba = (struct BIO_asio_st*)BIO_get_data(b);
    if(ba->read_done) {
        assert(data == ba->read_data); // THE GUARANTEE I WANT <--------------------------------------------------------
        *readbytes = ba->read_dlen;
        ba->read_done = false;
        printf("Reported the completion of a read in %p of %zu bytes\n", data, *readbytes);
        return 1;
    }
    
    ba->read_data = data;
    ba->read_dlen = dlen;
    BIO_clear_retry_flags(b);
    BIO_set_retry_read(b);
    printf("Requested read in %p of a maximum of %zu bytes\n", data, dlen);
    
    return 0;
}

static int BIO_s_asio_write_ex(BIO *b, const char *data, size_t dlen, size_t *written)
{
    struct BIO_asio_st* ba = (struct BIO_asio_st*)BIO_get_data(b);
    if(ba->write_done) {
        assert(data == ba->write_data); // THE GUARANTEE I WANT <--------------------------------------------------------
        *written = ba->write_dlen;
        ba->write_done = false;
        printf("Reported the completion of a write in %p of %zu bytes\n", data, *written);
        return 1;
    }
    
    ba->write_data = data;
    ba->write_dlen = dlen;
    BIO_clear_retry_flags(b);
    BIO_set_retry_write(b);
    printf("Requested write in %p of %zu bytes\n", data, dlen);
    
    return 0;
}

static long BIO_s_asio_ctrl(BIO *b, int cmd, long larg, void *parg) {
    long ret = 0;

    switch (cmd) {
        case BIO_CTRL_FLUSH:
            ret = 1;
            break;
    }

    return ret;
}

static int bio_new(BIO *bio)
{
    struct BIO_asio_st *b = (struct BIO_asio_st *)OPENSSL_zalloc(sizeof(*b));
    if (b == NULL)
        return 0;
    
    b->read_done = false;
    b->write_done = false;
    
    BIO_set_data(bio, b);
    
    BIO_set_init(bio, 1);
    return 1;
}

static int bio_free(BIO *bio)
{
    struct BIO_asio_st *b = (struct BIO_asio_st *)BIO_get_data(bio);

    OPENSSL_free(b);

    return 1;
}
    

BIO_METHOD *BIO_s_asio_meth_init(void)
{
    BIO_METHOD *BIO_s_asio_ = BIO_meth_new(BIO_get_new_index()|BIO_TYPE_SOURCE_SINK, "BIO_s_asio");

    BIO_meth_set_write_ex(BIO_s_asio_, BIO_s_asio_write_ex);
    BIO_meth_set_read_ex(BIO_s_asio_, BIO_s_asio_read_ex);
    BIO_meth_set_ctrl(BIO_s_asio_, BIO_s_asio_ctrl);
    BIO_meth_set_create(BIO_s_asio_, bio_new);
    BIO_meth_set_destroy(BIO_s_asio_, bio_free);
    
    return BIO_s_asio_;
}

static BIO_METHOD *BIO_s_asio_ = nullptr;
BIO_METHOD *BIO_s_asio(void)
{
    if (!BIO_s_asio_)
    {
        BIO_s_asio_ = BIO_s_asio_meth_init();
    }

    return BIO_s_asio_;
}

class ssl_socket {
public:
    ssl_socket(SSL_CTX *ctx, boost::asio::ip::tcp::socket socket)
        : bio_{BIO_new(BIO_s_asio())}
        , ssl_{SSL_new (ctx)}
        , socket_{std::move(socket)}
        , bio_data_{(struct BIO_asio_st*)BIO_get_data(bio_)}
    {
        if(ssl_ == nullptr) {
            throw std::runtime_error{"Ups"};
        }
        
        if(bio_ == nullptr) {
            throw std::runtime_error{"Ups2"};
        }
        
        if(bio_data_ == nullptr) {
            throw std::runtime_error{"Ups3"};
        }
        
        SSL_set_bio(ssl_,bio_,bio_);
        SSL_set_read_ahead(ssl_, 1);
    }
    
    template <typename CompletionToken>
    void async_handshake(CompletionToken&& token) {
        return boost::asio::async_compose<
            CompletionToken, void(boost::system::error_code)>(
            [
                this,
                coro = boost::asio::coroutine()
            ]
            (
                auto& self,
                const boost::system::error_code& error = {},
                std::size_t len = 0
            ) mutable
            {
                int err;
                int get_error;
                reenter (coro)
                {
                    while(true) {
                        err = SSL_connect(ssl_);
                        if(err >= 0) {
                            break;
                        }
                        get_error = SSL_get_error(ssl_, err);
                        assert(get_error != 1);
                        if(get_error == SSL_ERROR_WANT_READ) {
                                yield socket_.async_read_some(boost::asio::buffer(bio_data_->read_data, bio_data_->read_dlen), std::move(self));
                                bio_data_->read_dlen = len;
                                bio_data_->read_done = true;
                        }else if(get_error == SSL_ERROR_WANT_WRITE) {
                                yield socket_.async_write_some(boost::asio::buffer(bio_data_->write_data, bio_data_->write_dlen), std::move(self));
                                bio_data_->write_dlen = len;
                                bio_data_->write_done = true;
                        }
                    }
                    self.complete(error);
                }
            },
            token, socket_);
    }
    
    using executor_type = boost::asio::ip::tcp::socket::executor_type;
    
    executor_type get_executor() {
        return socket_.get_executor();
    }
    
    BIO *bio_;
    struct BIO_asio_st* bio_data_;
    SSL* ssl_;
    boost::asio::ip::tcp::socket socket_;
};


#define CHK_NULL(x) if ((x)==NULL) exit (1)
#define CHK_ERR(err,s) if ((err)==-1) { perror(s); exit(3); }
#define CHK_SSL(err) if ((err)==-1) { ERR_print_errors_fp(stderr); exit(3); }
#define CHK_SSL2(err) if ((err)==0) { ERR_print_errors_fp(stderr); exit(4); }

int main(int argc, char*argv[]) {
    SSL_library_init();
    SSL_load_error_strings();

    boost::asio::io_context io_context;
    
    boost::asio::ip::tcp::socket s(io_context);
    boost::asio::ip::tcp::resolver resolver(io_context);
    boost::asio::connect(s, resolver.resolve(argv[1], argv[2]));

    SSL_CTX *ctx = SSL_CTX_new(TLS_client_method()); CHK_NULL(ctx);
    
    ssl_socket sock{ctx, std::move(s)};
    sock.async_handshake([](boost::system::error_code ec){printf("Done!\n");});

    io_context.run();
}

See it running: https://godbolt.org/z/8TEMrY

[reddwarf69]

I have requested that openSSL adds the guarantee in openssl/openssl#12631.