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.

Sign up for GitHub

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

Jump to bottom

Broker check certificate load error and perform periodic reloading #781

Merged
merged 1 commit into from
Jan 11, 2021
Merged

Broker check certificate load error and perform periodic reloading #781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions example/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ FOREACH (source_file ${exec_PROGRAMS})
GET_FILENAME_COMPONENT (source_file_we ${source_file} NAME_WE)
ADD_EXECUTABLE (${source_file_we} ${source_file})
TARGET_LINK_LIBRARIES (${source_file_we} mqtt_cpp_iface)

IF (WIN32 AND MQTT_USE_STATIC_OPENSSL)
TARGET_LINK_LIBRARIES (${source_file_we} Crypt32)
ENDIF ()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this update is required?

IF (MQTT_USE_LOG)
TARGET_COMPILE_DEFINITIONS (${source_file_we} PUBLIC $<IF:$<BOOL:${MQTT_USE_STATIC_BOOST}>,,BOOST_LOG_DYN_LINK>)
TARGET_LINK_LIBRARIES (${source_file_we} Boost::log)
Expand All @@ -58,19 +63,23 @@ FOREACH (source_file ${exec_PROGRAMS})
TARGET_LINK_LIBRARIES (${source_file_we} Boost::program_options)
ENDFOREACH ()

FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../example/broker.conf DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.crt.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.key.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/cacert.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY broker.conf DESTINATION "${CMAKE_CURRENT_BINARY_DIR}" )
FILE(COPY ../test/certs/mosquitto.org.crt DESTINATION "${CMAKE_CURRENT_BINARY_DIR}" )
FILE(COPY ../test/certs/server.crt.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}" )
FILE(COPY ../test/certs/server.key.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}" )
FILE(COPY ../test/certs/cacert.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}" )

IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/mosquitto.org.crt DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.crt.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.key.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/cacert.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
ELSE ()
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/mosquitto.org.crt DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.crt.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/server.key.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../test/certs/cacert.pem DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
ENDIF ()
FILE(COPY broker.conf DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
FILE(COPY ../test/certs/mosquitto.org.crt DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Release")
FILE(COPY ../test/certs/server.crt.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Release")
FILE(COPY ../test/certs/server.key.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Release")
FILE(COPY ../test/certs/cacert.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Release")

FILE(COPY broker.conf DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Debug)
FILE(COPY ../test/certs/mosquitto.org.crt DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Debug")
FILE(COPY ../test/certs/server.crt.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Debug")
FILE(COPY ../test/certs/server.key.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Debug")
FILE(COPY ../test/certs/cacert.pem DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/Debug")

ENDIF ()
12 changes: 9 additions & 3 deletions example/broker.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# Default configuration for MQTT-CPP Broker
verbose=1
certificate=broker.crt.pem
private_key=broker.key.pem
certificate=server.crt.pem
private_key=server.key.pem
Copy link
Owner

@redboltz redboltz Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following comment should be added. Please update to better English :)

# Reload interval for the certificate and private key files (seconds)
# It is useful for Let's Encrypt certs update.
# If not set or set to 0, then no reload.
#
# certificate_reload_interval=3600


# Reload interval for the certificate and private key files (hours)
# When configured the broker will perform automatic loading of
# cert/key update. If not set or set to 0 (default), then no
# reloading is performed.
# certificate_reload_interval=24

# Configuration for TCP
[tcp]
Expand All @@ -17,4 +23,4 @@ port=1883

# Configuration for Websocket with TLS
[wss]
# port=10433
# port=10443
101 changes: 84 additions & 17 deletions example/broker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,79 @@
#include <fstream>

#if defined(MQTT_USE_TLS)
boost::asio::ssl::context init_ctx(boost::program_options::variables_map const& vm)
boost::asio::ssl::context init_ctx()
{
if (vm.count("certificate") == 0 && vm.count("private_key") == 0) {
throw std::runtime_error("TLS requested but certificate and/or private_key not specified");
}

boost::asio::ssl::context ctx(boost::asio::ssl::context::tlsv12);
ctx.set_options(
boost::asio::ssl::context::default_workarounds |
boost::asio::ssl::context::single_dh_use);
ctx.use_certificate_file(vm["certificate"].as<std::string>(), boost::asio::ssl::context::pem);
ctx.use_private_key_file(vm["private_key"].as<std::string>(), boost::asio::ssl::context::pem);
return ctx;
}

template<typename Server>
void reload_ctx(Server& server, boost::asio::steady_timer& reload_timer,
std::string const& certificate_filename,
std::string const& key_filename,
unsigned int certificate_reload_interval,
char const* name, bool first_load = true)
{
MQTT_LOG("mqtt_broker", info) << "Reloading certificates for server " << name;

if (certificate_reload_interval > 0) {
reload_timer.expires_after(std::chrono::hours(certificate_reload_interval));
reload_timer.async_wait(
[&server, &reload_timer, certificate_filename, key_filename, certificate_reload_interval, name]
(boost::system::error_code const& e) {

BOOST_ASSERT(!e || e == boost::asio::error::operation_aborted);

if (!e) {
reload_ctx(server, reload_timer, certificate_filename, key_filename, certificate_reload_interval, name, false);
}
});
}

auto context = init_ctx();

boost::system::error_code ec;
context.use_certificate_file(certificate_filename, boost::asio::ssl::context::pem, ec);
if (ec) {
auto message = "Failed to load certificate file: " + ec.message();
if (first_load) {
throw std::runtime_error(message);
}

MQTT_LOG("mqtt_broker", warning) << message;
return;
}

context.use_private_key_file(key_filename, boost::asio::ssl::context::pem, ec);
if (ec) {
auto message = "Failed to load private key file: " + ec.message();
if (first_load) {
throw std::runtime_error(message);
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If certificate_reload_interval is 0 then don't set timer.

MQTT_LOG("mqtt_broker", warning) << message;
return;
}

server.get_ssl_context() = std::move(context);
}

template<typename Server>
void load_ctx(Server& server, boost::asio::steady_timer& reload_timer, boost::program_options::variables_map const& vm, char const* name)
{
if (vm.count("certificate") == 0 && vm.count("private_key") == 0) {
throw std::runtime_error("TLS requested but certificate and/or private_key not specified");
}

reload_ctx(server, reload_timer,
vm["certificate"].as<std::string>(),
vm["private_key"].as<std::string>(),
vm["certificate_reload_interval"].as<unsigned int>(),
name, true);
}
#endif // defined(MQTT_USE_TLS)

void run_broker(boost::program_options::variables_map const& vm)
Expand All @@ -38,33 +97,40 @@ void run_broker(boost::program_options::variables_map const& vm)
boost::asio::io_context ioc;
MQTT_NS::broker::broker_t b(ioc);

std::unique_ptr<test_server_no_tls> s;
MQTT_NS::optional<test_server_no_tls> s;
if (vm.count("tcp.port")) {
s = std::make_unique<test_server_no_tls>(ioc, b, vm["tcp.port"].as<uint16_t>());
s.emplace(ioc, b, vm["tcp.port"].as<uint16_t>());
}

#if defined(MQTT_USE_WS)
std::unique_ptr<test_server_no_tls_ws> s_ws;
MQTT_NS::optional<test_server_no_tls_ws> s_ws;
if (vm.count("ws.port")) {
s_ws = std::make_unique<test_server_no_tls_ws>(ioc, b, vm["ws.port"].as<uint16_t>());
s_ws.emplace(ioc, b, vm["ws.port"].as<uint16_t>());
}
#endif // defined(MQTT_USE_WS)

#if defined(MQTT_USE_TLS)
std::unique_ptr<test_server_tls> s_tls;
MQTT_NS::optional<test_server_tls> s_tls;
MQTT_NS::optional<boost::asio::steady_timer> s_lts_timer;

if (vm.count("tls.port")) {
s_tls = std::make_unique<test_server_tls>(ioc, init_ctx(vm), b, vm["tls.port"].as<uint16_t>());
s_tls.emplace(ioc, init_ctx(), b, vm["tls.port"].as<uint16_t>());
s_lts_timer.emplace(ioc);
load_ctx(s_tls.value(), s_lts_timer.value(), vm, "TLS");
}
#endif // defined(MQTT_USE_TLS)

#if defined(MQTT_USE_TLS) && defined(MQTT_USE_WS)
std::unique_ptr<test_server_tls_ws> s_tls_ws;
MQTT_NS::optional<test_server_tls_ws> s_tls_ws;
MQTT_NS::optional<boost::asio::steady_timer> s_tls_ws_timer;

if (vm.count("wss.port")) {
s_tls_ws = std::make_unique<test_server_tls_ws>(ioc, init_ctx(vm), b, vm["wss.port"].as<uint16_t>());
s_tls_ws.emplace(ioc, init_ctx(), b, vm["wss.port"].as<uint16_t>());
s_tls_ws_timer.emplace(ioc);
load_ctx(s_tls_ws.value(), s_tls_ws_timer.value(), vm, "WSS");
}
#endif // defined(MQTT_USE_TLS) && defined(MQTT_USE_WS)


ioc.run();
} catch(std::exception &e) {
MQTT_LOG("mqtt_broker", error) << e.what();
Expand All @@ -84,7 +150,8 @@ int main(int argc, char **argv) {
#endif // defined(MQTT_USE_LOG)
("certificate", boost::program_options::value<std::string>(), "Certificate file for TLS connections")
("private_key", boost::program_options::value<std::string>(), "Private key file for TLS connections")
;
("certificate_reload_interval", boost::program_options::value<unsigned int>()->default_value(0), "Reload interval for the certificate and private key files (hours)\n 0 - Disabled")
;

boost::program_options::options_description notls_desc("TCP Server options");
notls_desc.add_options()
Expand Down
4 changes: 4 additions & 0 deletions test/system/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ FOREACH (source_file ${check_PROGRAMS})
TARGET_LINK_LIBRARIES (
${source_file_we} mqtt_cpp_iface Boost::unit_test_framework
)
IF (WIN32 AND MQTT_USE_STATIC_OPENSSL)
TARGET_LINK_LIBRARIES (${source_file_we} Crypt32)
ENDIF ()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this update is required?

IF (MQTT_USE_LOG)
TARGET_COMPILE_DEFINITIONS (${source_file_we} PUBLIC $<IF:$<BOOL:${MQTT_USE_STATIC_BOOST}>,,BOOST_LOG_DYN_LINK>)
TARGET_LINK_LIBRARIES (
Expand Down
16 changes: 16 additions & 0 deletions test/system/test_server_tls.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,22 @@ class test_server_tls {
server_.close();
}

/**
* @brief Get boost asio ssl context.
* @return ssl context
*/
MQTT_NS::tls::context& get_ssl_context() {
return server_.get_ssl_context();
}

/**
* @brief Get boost asio ssl context.
* @return ssl context
*/
MQTT_NS::tls::context const& get_ssl_context() const {
return server_.get_ssl_context();
}

private:
MQTT_NS::server_tls<> server_;
MQTT_NS::broker::broker_t& b_;
Expand Down
16 changes: 16 additions & 0 deletions test/system/test_server_tls_ws.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,22 @@ class test_server_tls_ws {
server_.close();
}

/**
* @brief Get boost asio ssl context.
* @return ssl context
*/
MQTT_NS::tls::context& get_ssl_context() {
return server_.get_ssl_context();
}

/**
* @brief Get boost asio ssl context.
* @return ssl context
*/
MQTT_NS::tls::context const& get_ssl_context() const {
return server_.get_ssl_context();
}

private:
MQTT_NS::server_tls_ws<> server_;
MQTT_NS::broker::broker_t& b_;
Expand Down
4 changes: 4 additions & 0 deletions test/unit/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ FOREACH (source_file ${check_PROGRAMS})
TARGET_LINK_LIBRARIES (
${source_file_we} mqtt_cpp_iface Boost::unit_test_framework
)
IF (WIN32 AND MQTT_USE_STATIC_OPENSSL)
TARGET_LINK_LIBRARIES (${source_file_we} Crypt32)
ENDIF ()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this update is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. This is again maybe unrelated. But openssl static om Windows requires linking crypt32

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/questions/37522654/linking-with-openssl-lib-statically

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove this?
static link is out of scope here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that I already merged static support. Hmm...
Maybe it is required. What does 32 mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or cmake ?
https://gitlab.kitware.com/cmake/cmake/-/issues/19263

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I don't have much time to investigate it.
So if

    IF (MSVC AND MQTT_USE_STATIC_OPENSSL)
        TARGET_LINK_LIBRARIES (${source_file_we} Crypt32)
    ENDIF ()

is required for CI, then I apply it.

Otherwise, if this works well for both 64bit and 32bit, then I apply it.
If it works either 32bit or 64bit only, I don't apply it.

Which is the status ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works for both 64bit and 32bit

Copy link
Contributor Author

@kleunen kleunen Jan 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not needed for CI, because CI builds with dynamic openssl. I changed it because I build localy with static openssl on windows.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works for both 64bit and 32bit

Ok, I accept it.

IF (MQTT_USE_LOG)
TARGET_COMPILE_DEFINITIONS (${source_file_we} PUBLIC $<IF:$<BOOL:${MQTT_USE_STATIC_BOOST}>,,BOOST_LOG_DYN_LINK>)
TARGET_LINK_LIBRARIES (
Expand Down