Skip to content

Supported alternative TLS library. #673

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

Merged
merged 1 commit into from
Nov 13, 2020
Merged

Supported alternative TLS library. #673

merged 1 commit into from
Nov 13, 2020

Conversation

redboltz
Copy link
Owner

@redboltz redboltz commented Oct 5, 2020

No description provided.

@redboltz
Copy link
Owner Author

redboltz commented Oct 5, 2020

This is currently just PoC.

@redboltz redboltz force-pushed the add_alt_tls_support branch from 93c6e0b to 1b33ca5 Compare October 5, 2020 09:10
@redboltz redboltz force-pushed the add_alt_tls_support branch 3 times, most recently from 2301e16 to c87dc47 Compare October 5, 2020 09:31
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #673 (3a222bb) into master (ca47db0) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #673   +/-   ##
=======================================
  Coverage   82.06%   82.06%           
=======================================
  Files          47       47           
  Lines        7082     7082           
=======================================
  Hits         5812     5812           
  Misses       1270     1270           

@redboltz
Copy link
Owner Author

redboltz commented Oct 5, 2020

Using alternative TLS library

mqtt_cpp uses OpenSSL by default. User can use alternative TLS library such as GNUTLS instead of OpenSSL.

How to use

Define the following three preprosessor macros.

macro meaning default
MQTT_TLS_INCLUDE TLS include path <boost/asio/ssl.hpp>
MQTT_TLS_WS_INCLUDE TLS (Websocket) include path <boost/beast/websocket/ssl.hpp>
MQTT_TLS_NS namespace of tls library boost::asio::ssl

If you define the macros, then the default value is overridden. Note that MQTT_TLS_WS_INCLUDE is required only if MQTT_USE_TLS is defined.

Design choice

mqtt_cpp doesn't care about alternative TLS libraries directly. In other words, it is out of scope. mqtt_cpp provides minimal support for the alternative TLS libraries.

@shantanu1singh
Copy link

#endif // defined(MQTT_USE_TLS)

This doesn't work with GnuTLS.

We'd need for this to be pulled out into a function in tls.hpp

This is what I'd done in my PR earlier:

inline constexpr bool is_tls_short_read(int error_val)
{

#if MQTT_USE_TLS == MQTT_TLS_OPENSSL
#if defined(SSL_R_SHORT_READ)
return ERR_GET_REASON(error_val) == SSL_R_SHORT_READ;
#else // defined(SSL_R_SHORT_READ)
return ERR_GET_REASON(error_val) == tls::error::stream_truncated;
#endif // defined(SSL_R_SHORT_READ)
#elif MQTT_USE_TLS == MQTT_TLS_GNUTLS
return error_val == tls::error::stream_truncated;
#endif // MQTT_USE_TLS == MQTT_TLS_OPENSSL

return false;

}


Refers to: include/mqtt/endpoint.hpp:4569 in c87dc47. [](commit_id = c87dc47, deletion_comment = False)

@redboltz
Copy link
Owner Author

redboltz commented Oct 17, 2020

I'm not sure how to do that. But could you write a PR to fix the issue ? The target PR is #673 (add_alt_tls_support branch)
However, I don't want to introduce any GNU_TLS feature is mqtt_cpp.
So you need to introduce some abstraction like

#if !defined(MQTT_TLS_INCLUDE)
#define MQTT_TLS_INCLUDE <boost/asio/ssl.hpp>
#endif // !defined(MQTT_TLS_INCLUDE)

By default, OpenSSL is used. If you override some macro (maybe you newly introduce), then GnuTLS tls short read is enabled.

@redboltz redboltz force-pushed the add_alt_tls_support branch 2 times, most recently from fbb5104 to 3af7fa0 Compare October 17, 2020 05:18
@redboltz
Copy link
Owner Author

Just add the macro MQTT_TLS_ERROR_COMPARISON.

You can customize it using -D option.

 -DMQTT_TLS_ERROR_COMPARISON\(v\)=v==tls::error::stream_truncated

@shantanu1singh
Copy link

@redboltz Yep, that worked. Thanks!

There's one other thing though. In the CMakeLists.txt at the root and the CMakeLists.txt under include/, we are finding the OpenSSL library if MQTT_USE_TLS is set and linking to it. Would it be possible to disable that?

In order to use these changes, I had to add a find_package(OpenSSL) to my project's CMakeLists.txt

@redboltz
Copy link
Owner Author

redboltz commented Oct 21, 2020

I think that you can do as follows:

cmake -DMQTT_USE_TLS=OFF -DCMAKE_CXX_FLAGS="-DMQTT_USE_TLS -DMQTT_TLS_INCLUDE=...." ..

You can omit -DMQTT_USE_TLS=OFF. It is OFF by default.
The point is don't pass OpenSSL related flag to cmake, but pass TLS enable flag to the compiler.

@shantanu1singh
Copy link

@redboltz Ah cool, I'll try that. Thanks! I'll let you know soon if everything works as expected

@redboltz redboltz force-pushed the add_alt_tls_support branch from 3af7fa0 to 63d1c20 Compare November 7, 2020 12:34
@redboltz
Copy link
Owner Author

redboltz commented Nov 7, 2020

Due to #718 fix, MQTT_TLS_ERROR_COMPARISON is no longer required.

Added customization point for GNUTls.
@redboltz redboltz force-pushed the add_alt_tls_support branch from 63d1c20 to 3a222bb Compare November 8, 2020 03:03
@shantanu1singh
Copy link

@redboltz Sorry for getting back so late. Finally got to this again.

So, I ran the cmake command based on your recommendation:

cmake -DCMAKE_CXX_FLAGS='-I /usr/local/include/boost-asio-gnutls -I /usr/local/include/boost-beast-websockets-gnutls -L  /usr/local/lib/libgnutls.so -DMQTT_TLS_INCLUDE="<boost/asio/gnutls.hpp>" -DMQTT_TLS_WS_INCLUDE="<boost/beast/websocket/gnutls/ssl.hpp>" -DMQTT_TLS_NS=boost::asio::gnutls -DMQTT_USE_TLS' -DMQTT_BUILD_TESTS=OFF -DMQTT_BUILD_EXAMPLES=OFF -DMQTT_USE_WS=ON .. 

However, when I try to build an executable that depends on the mqtt_cpp library, I get some errors:

error: ‘make_tls_sync_client’ is not a member of ‘mqtt’
         mqttHandle_ = MQTT_NS::make_tls_sync_client(ioContext_, host, port, version);
/usr/local/include/mqtt/client.hpp:446:21: error: no matching function for call to ‘mqtt::client<mqtt::tcp_endpoint<boost::asio::gnutls::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >, boost::asio::io_context::strand>, 2>::setup_socket(std::shared_ptr<mqtt::tcp_endpoint<boost::asio::gnutls::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >, boost::asio::io_context::strand> >&)’
         setup_socket(socket_);
         ~~~~~~~~~~~~^~~~~~~~~

Everything works fine if I set the MQTT_USE_TLS flag to 'ON' in the Cmake options as well.

@redboltz
Copy link
Owner Author

redboltz commented Nov 11, 2020

If you defined MQTT_USE_TLS correctly, make_tls_sync_client should be defined.

See the following code:
https://github.com/redboltz/mqtt_cpp/blob/master/include/mqtt/sync_client.hpp#L86

It seems that you need to check actual compiler options using VERBOSE=1, and preprosessor output.

@shantanu1singh
Copy link

@redboltz Yep, my bad. It worked. Thanks for the help!

Will you be merging this change with master now? Also, any plans of an upcoming release that might include this change? Thanks!

@redboltz redboltz merged commit 05d6889 into master Nov 13, 2020
@redboltz redboltz deleted the add_alt_tls_support branch November 13, 2020 13:20
@redboltz
Copy link
Owner Author

merged. Release is not decided yet.

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

Successfully merging this pull request may close these issues.

2 participants