search

oliora / ppconsul

C++ client for Consul (http://consul.io)
Boost Software License 1.0
152 stars 56 forks source link

SSL (TLS, https) support #12

Closed oliora closed 6 years ago

oliora commented 6 years ago

ppconsul should support TLS connections to the Consul. Because security!

kinofrally commented 6 years ago

Hi Andrey, when will TLS support will be added? Our project really needs ppconsul use https to connect consul server~

oliora commented 6 years ago

I was waiting when it comes here :) I can probably add it on this week or few days after.

kinofrally commented 6 years ago

Great, look forward for your committing, thank you so much~ ^_^

oliora commented 6 years ago

Hey @kinofrally, I've added TLS support in https_client branch. So far I've checked it only on macOS. In one-two days I'm going to add tests, verify that stuff works on all platforms and merge changes to master.

If you want you can try the new feature already. I'll be happy to get a feedback on its usage.

oliora commented 6 years ago

I've merged HTTPS branch to master. It should be fine now. Feel free to open another issue if something doesn't work.

kinofrally commented 6 years ago

Hi, Andrey, I tried this new feature today, here are some issues confusing me.

1、make error on libcurl 7.29.0 At first, my curl version is 7.29.0, and when I make the ppconsul project, I ran into below error:

libs/ppconsul/src/curl/http_client.cpp: In constructor ‘ppconsul::curl::HttpClient::HttpClient(const string&, const ppconsul::http::impl::TlsConfig&)’: libs/ppconsul/src/curl/http_client.cpp:175:20: error: ‘CURLOPT_SSL_VERIFYSTATUS’ was not declared in this scope setopt(CURLOPT_SSL_VERIFYSTATUS, 1); ^ make[2]: [src/CMakeFiles/ppconsul.dir/curl/http_client.cpp.o] Error 1 make[1]: [src/CMakeFiles/ppconsul.dir/all] Error 2 make: *** [all] Error 2

Then I found CURLOPT_SSL_VERIFYSTATUS was added into libcurl since 7.41.0, so the minimum version of libcurl maybe change to 7.41.0.

2、thread safety issue Currently our project underlying ssl library is OpenSSl 1.0.2k, and according to this link: https://curl.haxx.se/libcurl/c/threadsafe.html, we must set callbacks to guarantee thread safety. I was wondering should I call lock function before I call ppconsul API, and then call unlock function?

This issue maybe out of ppconsul scope, I am just not familiar with c++ programming, so look forward for your help, thank you~~~

oliora commented 6 years ago

Hi, @kinofrally

Thank you for reporting this.

I'll disable CURLOPT_SSL_VERIFYSTATUS usage in ppconsul if one is build with libcurl older than 7.41.0.

For OpenSSL locks I think it's outside of ppconsul scope primary because libcurl can be built with many different SSL backends and I don't want to go into dealing with each ones particular behavior. Thus you need to provide the OpenSSL callbacks in your application. Also I highly encourage you to update OpenSSL version you use if you care about security (this also eliminates the need of providing the callbacks :)

kinofrally commented 6 years ago

Hi, Andrey, I updated OpenSSL yesterday, everything worked fine. Thanks for your advice, it really helped us save a lot of trouble. ^_^