TLS boost::asio::ssl compatible library does not work

[fxdupont]

I tried to make a small server tool using the TLS "boost::asio::ssl compatible" library but it fails in handshake with an Unknown error from a "basic_flat_buffer too long" exception.

This looks like a mismatch between Botan and boost::beast so I give here the exact environment:

• I use a x86 mac running macOS Big Sur 11.1
• the Xcode version is 12.4 and I use the C++ compiler with the --std=c++11 option
• the botan homebrew package is a 2.17.3 where I patched the formula to add --with-boost
• the boost homebrew package is 1.75.0

I attached the botan and boost formulas (in the case a config flag matters) and the tool which is derived from the boost c++11 sample for the boost::asio::ssl library. I tried the client derived from the same sample directory and also the tls_client from Botan cli: both made the handshake to fail on the server side. I used wireshark to see the packets: there is nothing particular: the client sends a standard (and between 280-290 bytes) TLS ClientHello and the server closes the connection.

I can help if you need more details or to run test codes. The idea of the boost::asio::ssl compatible library was great and I really want to make it to work in the software I work on which offers the choice between Botan and OpenSSL crypto backends. Even I currently is not about security software I worked many times on security related codes: in fact this is something which comes back often: I think I never stay 10 years without it...

Another (very minor) point: the classes derived from boost::system::error_code should have a virtual destructor to not get problems with the clang/gcc -Wnon-virtual-dtor flag. BTW it is clear these classes will be never used through a pointer to the base class but the compiler does not know... Alternately you can disable locally the warning but IMHO this is a dubious practice.

boostbotan.rb.txt boost.rb.txt

botan_boost_sample_server.cpp.txt

[randombit]

@hrantzsch @reneme any ideas?

[hrantzsch]

Strange! I'm not sure if the homebrew packages are really the issue here. To be certain, can you try compiling this example of an HTTP server with those same versions of Botan and Boost? https://github.com/hrantzsch/botan-tls-stream-server-example

[fxdupont]

I tried the HTTP server example: it went through the handshake but the asynchronous read stalls. According to wireshark I have a Server Hello and co but the Application Data from the client does nothing on the server side until the read timeout is triggered... So it did not help.

I'll try a more direct approach: I'll put some prints and an assert on the boost code which failed with my example. But even if I find a workaround this won't change in the short term the fact that Botan is packaged without the boost part so it could be never used in production with our product (Kea a DHCP server done by ISC).

[hrantzsch]

Thanks for helping us investigate the issue. The read timeout sounds like a different kind of issue to me. We'll try to reproduce the issue on macOS.

Regarding the homebrew package, you could consider using the conan package instead. Botan with Boost is available there.

[reneme]

@hrantzsch and I found the issue in Botan: It seems that TLS::Stream<> does not cope well with being std::move()-ed around for some reason. We'll need to look into the details later.

Anyway, to make your example work, you just need to create said object in-place in your session instead of relying on std::move()-ing the TLS::Stream<>.

A diff says more than a 1000 words:

87,88c87,88
<   session(Botan::TLS::Stream<tcp::socket> socket)
<     : socket_(std::move(socket))
---
>   session(tcp::socket socket, Botan::TLS::Context& ctx)
>     : socket_(std::move(socket), ctx)
170,171c170,171
<                   Botan::TLS::Stream<tcp::socket>(
<                   std::move(socket), context_))->start();
---
>               std::move(socket),
>               context_)->start();

[fxdupont]

I have a mixed feeling about programming language packager but as Kea is a pure C++ software I'll pass a message to my colleagues in charge of packaging.

I was sleeping so I lost your comment. I agree the move can be a source of problems and of course this makes to sample tool to got further.

If you need the ticket to find why the TLS::Stream breaks with its move constructor I leave this ticket open. Otherwise I close it as with your proposed change the handshake no longer triggers the error.

[randombit]

We should ideally work out why TLS::Stream is not move compatible and fix it. Or at the very least delete the relevant conversions - having something compile and then just silently not work is bad news.

[reneme]

We'll have a look into the move-issue with TLS::Stream in the coming days and will close the issue afterwards.

[fxdupont]

I have an unit test reusing the TCP connection. For OpenSSL it means to call SSL_clear, I thought about using the move assignment to get a fresh TLS stream but even it could work when the move will be fixed currently I believe the server side should not be very happy with this, i.e. this is clearly outside use cases specified in RFCs... So I'll give up in this point: the unit test is from pure TCP and should not be extended to TLS.

So please close the issue at your hand.

[hrantzsch]

We validated that your example runs smoothly with the patch linked above :+1: