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

Support for gnutls #662

Open
shantanu1singh opened this issue Sep 16, 2020 · 7 comments
Open

Support for gnutls #662

shantanu1singh opened this issue Sep 16, 2020 · 7 comments

Comments

@shantanu1singh
Copy link

I was able to leverage a fork that I'll try to get merged of this repository (with minimal changes), to support the use of gnutls instead of OpenSSL with the mqtt_cpp library.

Would the owners be accepting of a pull-request that adds support for gnutls using this other repository 'as a reference in docs' or a git submodule?

The changes I've had to make have primarily been the following across files in the mqtt_cpp repo:

  1. Include gnutls headers instead of OpenSSL
#if defined(MQTT_USE_TLS)
#if !defined(MQTT_USE_GNU_TLS)
    #include <boost/asio/ssl.hpp>
#else
    #include <boost/asio/gnutls.hpp>
    #include <gnutls/gnutls.h>
#endif // !defined(MQTT_USE_GNU_TLS)
#endif // defined(MQTT_USE_TLS)
  1. Change namespace definitions for the ssl library:
namespace as = boost::asio;

#if defined(MQTT_USE_TLS)
#if !defined(MQTT_USE_GNU_TLS)
    namespace ssl = as::ssl;
#else
    namespace ssl = as::gnutls;
#endif // !defined(MQTT_USE_GNU_TLS)
#endif // defined(MQTT_USE_TLS)
@jonesmz
Copy link
Contributor

jonesmz commented Sep 16, 2020

I think your best path to success for getting support for this merged is going to be to isolate the logic for handling which SSL library to use into a single header file.

Basically just take the stuff you specified in your initial comment here, and put that into ssl_implementation.h, or something similar.

But I don't see why the changes you're proposing would be rejected. This seems like a very useful thing to have.

Further, it also helps with regards to using wolfssl, which is useful for embedded contexts like OpenWRT.

@redboltz
Copy link
Owner

@shantanu1singh, I'm not sure why you want to use gnutls.

Do you send a pull request for Boost.Asio to support gnutls ? If you did it, please let me know the link of the pull request.
If Boost.Asio support gnutls, maybe mqtt_cpp automatically support it via boost. Perhaps small change is required.

This is another point that the license of gnutls is LGPL. It is more constrained licence that Boost license. I don't want to contain LGPL code as the submodule of mqtt_cpp.
I guess that it is related topic for #636. The license of WolfTLS is GPLv2 or commacial license.

I'm not sure how Boost.Asio treat the license issue well.

Anyway, I want to keep that mqtt_cpp simply use Boost style.

@shantanu1singh
Copy link
Author

@jonesmz With respect to moving the logic to a separate header, I can do that.

@redboltz I'm using mqtt_cpp in a project and our customer is hesitant to use the OpenSSL ssleay license

Boost.Asio doesn't support gnutls right now. This repository provides a GnuTls wrapper for Boost.Asio and it uses the Boost license (Not LGPL)

With the proposed change, there won't be any explicit gnutls related function calls in the mqtt_cpp library. It'll only consume the headers of the wrapper mentioned above. I should have a pull-request ready sometime tomorrow for you to see if it's acceptable.

If you don't like the idea of using a git submodule, could we instead just add instructions in the ReadMe or in the wiki to tell potential users that if they wish to use GnuTLS with mqtt_cpp, they will need to install the headers present in the wrapper above on their device?

@redboltz
Copy link
Owner

@shantanu1singh , I understand your situation.
And I've read https://github.com/paullouisageneau/boost-asio-gnutls carefully.
There is no header file name conflict with Boost.Asio, it is good.

I accept

  1. Include gnutls headers instead of OpenSSL

Please don't use negative condition with else clause. See https://github.com/redboltz/mqtt_cpp/wiki/Coding-Rules

https://github.com/paullouisageneau/boost-asio-gnutls

  1. Change namespace definitions for the ssl library:
    It this required?
    I think that the existing code always use as::ssl not ssl. Am I missing something ?

submodule vs outside

I think that outside file is better for this situation. If user wants to use gnutls, user need to prepare gnutls as the outside module. Similarly, https://github.com/paullouisageneau/boost-asio-gnutls too. Just install it to the system include path or add compiler option for adding include path.

@shantanu1singh
Copy link
Author

@redboltz

Please don't use negative condition with else clause. See https://github.com/redboltz/mqtt_cpp/wiki/Coding-Rules

Will do

Change namespace definitions for the ssl library:
It this required?
I think that the existing code always use as::ssl not ssl. Am I missing something ?

Yes, mqtt_cpp currently uses as::ssl, but the equivalent namespace in boost-asio-gnutls is as::gnutls. That's why I was thinking of creating a new namespace alias ssl which refers to boost::asio::ssl for OpenSSL and boost::asio::gnutls when boost-asio-gnutls is used. The side-effect of that though is that I'll have to change all occurrences of as::ssl in mqtt_cpp to ssl. Did you have a better solution in mind?

I think that outside file is better for this situation. If user wants to use gnutls, user need to prepare gnutls as the outside module. Similarly, https://github.com/paullouisageneau/boost-asio-gnutls too. Just install it to the system include path or add compiler option for adding include path.

That works. I can add instructions to the ReadMe in mqtt_cpp or the wiki. Which one would you prefer?

@redboltz
Copy link
Owner

@redboltz

Please don't use negative condition with else clause. See https://github.com/redboltz/mqtt_cpp/wiki/Coding-Rules

Will do

Change namespace definitions for the ssl library:
It this required?
I think that the existing code always use as::ssl not ssl. Am I missing something ?

Yes, mqtt_cpp currently uses as::ssl, but the equivalent namespace in boost-asio-gnutls is as::gnutls. That's why I was thinking of creating a new namespace alias ssl which refers to boost::asio::ssl for OpenSSL and boost::asio::gnutls when boost-asio-gnutls is used. The side-effect of that though is that I'll have to change all occurrences of as::ssl in mqtt_cpp to ssl. Did you have a better solution in mind?

Thank you for elaborating. I understand. Replacing existing as::ssl is good.
I think that newly introduced namespace alias should be tls.
I guess that boost asio sub namespace ssl is by historical reason.

I think that outside file is better for this situation. If user wants to use gnutls, user need to prepare gnutls as the outside module. Similarly, https://github.com/paullouisageneau/boost-asio-gnutls too. Just install it to the system include path or add compiler option for adding include path.

That works. I can add instructions to the ReadMe in mqtt_cpp or the wiki. Which one would you prefer?

I think that https://github.com/redboltz/mqtt_cpp/wiki/Config is appropriate place.

In addition, you can add something at TLS support table on README.md.

@ineffective
Copy link

@redboltz I'm using mqtt_cpp in a project and our customer is hesitant to use the OpenSSL ssleay license

Currently OpenSSL 3.0.0 and higher is under Apache license.

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