From 29e655826b506ae960d7485f5fec953b748ddeae Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Mon, 10 Dec 2018 10:01:24 +0000 Subject: Introduce extra mutex to avoid need for unlock guard Also add instructions on how to regenerate keys/certs for tests. --- .../auto_reloading_tls_crypto_engine_test.cpp | 42 ++++++++++++++++++++++ .../net/tls/auto_reloading_tls_crypto_engine.cpp | 37 +++++++------------ .../net/tls/auto_reloading_tls_crypto_engine.h | 9 ++--- 3 files changed, 59 insertions(+), 29 deletions(-) (limited to 'vespalib/src') diff --git a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp index 7926f71e812..034dc99b72c 100644 --- a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp +++ b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp @@ -15,6 +15,48 @@ using namespace vespalib; using namespace vespalib::net::tls; using namespace std::chrono_literals; +/* + +Keys and certificates used for these tests generated with commands: + +openssl ecparam -name prime256v1 -genkey -noout -out ca.key + +# test_ca.pem: +openssl req -new -x509 -nodes -key ca.key \ + -sha256 -out test_ca.pem \ + -subj '/C=US/L=LooneyVille/O=ACME/OU=ACME test CA/CN=acme.example.com' \ + -days 10000 + +openssl ecparam -name prime256v1 -genkey -noout -out test_key.pem + +openssl req -new -key test_key.pem -out host1.csr \ + -subj '/C=US/L=LooneyVille/O=Wile. E. Coyote, Ltd./CN=wile.example.com' \ + -sha256 + +# cert1_pem: +openssl x509 -req -in host1.csr \ + -CA ca.pem \ + -CAkey ca.key \ + -CAcreateserial \ + -out cert1.pem \ + -days 10000 \ + -sha256 + +openssl req -new -key test_key.pem -out host2.csr \ + -subj '/C=US/L=LooneyVille/O=Wile. E. Coyote, Ltd./CN=wile.example.com' \ + -sha256 + +# cert2_pem: +openssl x509 -req -in host2.csr \ + -CA ca.pem \ + -CAkey ca.key \ + -CAcreateserial \ + -out cert2.pem \ + -days 10000 \ + -sha256 + +*/ + constexpr const char* cert1_pem = R"(-----BEGIN CERTIFICATE----- MIIBszCCAVgCCQCXsYrXQWS0bzAKBggqhkjOPQQDAjBkMQswCQYDVQQGEwJVUzEU MBIGA1UEBwwLTG9vbmV5VmlsbGUxDTALBgNVBAoMBEFDTUUxFTATBgNVBAsMDEFD diff --git a/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.cpp b/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.cpp index 665b19cdaaf..1b84c98a46d 100644 --- a/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.cpp @@ -31,23 +31,13 @@ std::shared_ptr try_create_engine_from_tls_config(const vespali } } -// Unlocks on construction, re-locks on destruction. -struct UnlockGuard { - std::unique_lock& _lock; - explicit UnlockGuard(std::unique_lock& lock) : _lock(lock) { - _lock.unlock(); - } - ~UnlockGuard() { - _lock.lock(); - } -}; - } // anonymous namespace AutoReloadingTlsCryptoEngine::AutoReloadingTlsCryptoEngine(vespalib::string config_file_path, TimeInterval reload_interval) - : _mutex(), - _cond(), + : _thread_mutex(), + _thread_cond(), + _engine_mutex(), _shutdown(false), _config_file_path(std::move(config_file_path)), _current_engine(tls_engine_from_config_file(_config_file_path)), @@ -58,9 +48,9 @@ AutoReloadingTlsCryptoEngine::AutoReloadingTlsCryptoEngine(vespalib::string conf AutoReloadingTlsCryptoEngine::~AutoReloadingTlsCryptoEngine() { { - std::unique_lock lock(_mutex); + std::lock_guard lock(_thread_mutex); _shutdown = true; - _cond.notify_all(); + _thread_cond.notify_all(); } _reload_thread.join(); } @@ -70,30 +60,27 @@ std::chrono::steady_clock::time_point AutoReloadingTlsCryptoEngine::make_future_ } void AutoReloadingTlsCryptoEngine::run_reload_loop() { - std::unique_lock lock(_mutex); + std::unique_lock lock(_thread_mutex); auto reload_at_time = make_future_reload_time_point(); while (!_shutdown) { - if (_cond.wait_until(lock, reload_at_time) == std::cv_status::timeout) { + if (_thread_cond.wait_until(lock, reload_at_time) == std::cv_status::timeout) { LOG(debug, "TLS config reload time reached, reloading file '%s'", _config_file_path.c_str()); - try_replace_current_engine(lock); + try_replace_current_engine(); reload_at_time = make_future_reload_time_point(); } // else: spurious wakeup or shutdown } } -void AutoReloadingTlsCryptoEngine::try_replace_current_engine(std::unique_lock& held_lock) { - std::shared_ptr new_engine; - { - UnlockGuard guard(held_lock); - new_engine = try_create_engine_from_tls_config(_config_file_path); - } +void AutoReloadingTlsCryptoEngine::try_replace_current_engine() { + std::shared_ptr new_engine = try_create_engine_from_tls_config(_config_file_path); if (new_engine) { + std::lock_guard guard(_engine_mutex); _current_engine = std::move(new_engine); } } AutoReloadingTlsCryptoEngine::EngineSP AutoReloadingTlsCryptoEngine::acquire_current_engine() const { - std::lock_guard guard(_mutex); + std::lock_guard guard(_engine_mutex); return _current_engine; } diff --git a/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.h b/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.h index 327179dbd17..4e3a4074b03 100644 --- a/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.h +++ b/vespalib/src/vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.h @@ -17,16 +17,17 @@ public: using EngineSP = std::shared_ptr; using TimeInterval = std::chrono::steady_clock::duration; private: - mutable std::mutex _mutex; - std::condition_variable _cond; + mutable std::mutex _thread_mutex; + std::condition_variable _thread_cond; + mutable std::mutex _engine_mutex; bool _shutdown; const vespalib::string _config_file_path; - EngineSP _current_engine; // Access must be under _mutex + EngineSP _current_engine; // Access must be under _engine_mutex TimeInterval _reload_interval; std::thread _reload_thread; void run_reload_loop(); - void try_replace_current_engine(std::unique_lock& held_lock); + void try_replace_current_engine(); std::chrono::steady_clock::time_point make_future_reload_time_point() const noexcept; public: -- cgit v1.2.3