diff --git a/src/ngx_http_voms_module.cpp b/src/ngx_http_voms_module.cpp index bf643446fba7f7da998057720f14f94fd25f0704..2cd9b1db2e852cf5c9a5e86e3eadfb19d1811461 100644 --- a/src/ngx_http_voms_module.cpp +++ b/src/ngx_http_voms_module.cpp @@ -22,6 +22,7 @@ extern "C" { #include <cassert> #include <chrono> #include <iostream> +#include <map> #include <memory> #include <numeric> #include <string> @@ -63,8 +64,6 @@ ngx_module_t ngx_http_voms_module = { NGX_MODULE_V1_PADDING // }; -static std::unique_ptr<vomsdata> vomsdata_ptr; - static ngx_int_t generic_getter( // ngx_http_request_t* r, ngx_http_variable_value_t* v, @@ -344,52 +343,62 @@ static MaybeVomsAc retrieve_voms_ac_from_proxy(ngx_http_request_t* r) return boost::none; } - if (!vomsdata_ptr) { - vomsdata_ptr.reset(new vomsdata); - } - auto ok = - vomsdata_ptr->Retrieve(client_cert.get(), client_chain, RECURSE_CHAIN); + vomsdata vd; + + auto ok = vd.Retrieve(client_cert.get(), client_chain, RECURSE_CHAIN); if (!ok) { - auto msg = vomsdata_ptr->ErrorMessage().c_str(); + auto msg = vd.ErrorMessage().c_str(); ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "%s", msg); return boost::none; } - if (vomsdata_ptr->data.empty()) { + if (vd.data.empty()) { ngx_log_error(NGX_LOG_DEBUG, r->connection->log, 0, "no ACs in proxy"); return boost::none; } - return vomsdata_ptr->data.front(); + return vd.data.front(); } +// note that an entry in the cache is always created if within a connection a +// request is made to one of the voms_* variables. If the presented +// credential is a normal certificate or a proxy without attribute certificates, +// the unique_ptr points to a MaybeVomsAc that is empty (in the sense of an +// std::optional) +static std::map<ngx_connection_t*, std::unique_ptr<MaybeVomsAc>> ac_cache; + static void clean_voms_ac(void* data) { - auto r = static_cast<ngx_http_request_t*>(data); - ngx_log_error(NGX_LOG_DEBUG, r->connection->log, 0, "%s", __func__); - - auto p = static_cast<MaybeVomsAc*>( - ngx_http_get_module_ctx(r, ngx_http_voms_module)); - delete p; + auto c = static_cast<ngx_connection_t*>(data); + ngx_log_error(NGX_LOG_DEBUG, c->log, 0, "%s", __func__); + auto n = ac_cache.erase(c); + // we erase from the cache exactly once per connection + assert(n == 1); } -static void cache_voms_ac(ngx_http_request_t* r, MaybeVomsAc* ac) +static void cache_voms_ac(ngx_http_request_t* r, + std::unique_ptr<MaybeVomsAc> acp) { - ngx_http_set_ctx(r, ac, ngx_http_voms_module); - auto cln = ngx_http_cleanup_add(r, 0); + ngx_log_error(NGX_LOG_DEBUG, r->connection->log, 0, "%s", __func__); + auto c = r->connection; + auto cln = ngx_pool_cleanup_add(c->pool, 0); if (cln) { + auto r = ac_cache.insert({c, std::move(acp)}); + // we insert into the cache exactly once per connection + assert(r.second); cln->handler = clean_voms_ac; - cln->data = r; + cln->data = c; } else { ngx_log_error( - NGX_LOG_ERR, r->connection->log, 0, "ngx_http_cleanup_add() failed"); + NGX_LOG_ERR, r->connection->log, 0, "ngx_pool_cleanup_add() failed"); } } static MaybeVomsAc* get_voms_ac_from_cache(ngx_http_request_t* r) { - return static_cast<MaybeVomsAc*>( - ngx_http_get_module_ctx(r, ngx_http_voms_module)); + ngx_log_error(NGX_LOG_DEBUG, r->connection->log, 0, "%s", __func__); + auto it = ac_cache.find(r->connection); + return it != ac_cache.end() ? it->second.get() : nullptr; } static MaybeVomsAc const& get_voms_ac(ngx_http_request_t* r) @@ -399,8 +408,9 @@ static MaybeVomsAc const& get_voms_ac(ngx_http_request_t* r) MaybeVomsAc* acp = get_voms_ac_from_cache(r); if (!acp) { - acp = new MaybeVomsAc(retrieve_voms_ac_from_proxy(r)); - cache_voms_ac(r, acp); + auto p = std::make_unique<MaybeVomsAc>(retrieve_voms_ac_from_proxy(r)); + acp = p.get(); + cache_voms_ac(r, std::move(p)); } return *acp; diff --git a/valgrind.suppress b/valgrind.suppress new file mode 100644 index 0000000000000000000000000000000000000000..9226cabd7f54d4346a6978aae2ecbefdd6e7b788 --- /dev/null +++ b/valgrind.suppress @@ -0,0 +1,62 @@ +{ + <insert_a_suppression_name_here> + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:CRYPTO_malloc + fun:SSL_SESSION_new + fun:ssl_get_new_session + fun:ssl23_connect + fun:ngx_ssl_handshake + fun:ngx_http_upstream_ssl_init_connection + fun:ngx_http_upstream_send_request_handler + fun:ngx_http_upstream_handler + fun:ngx_epoll_process_events + fun:ngx_process_events_and_timers + fun:ngx_single_process_cycle + fun:main +} +{ + <insert_a_suppression_name_here> + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:ngx_alloc + fun:ngx_set_environment + fun:ngx_single_process_cycle + fun:main +} +{ + <insert_a_suppression_name_here> + Memcheck:Free + fun:free + fun:__libc_freeres + fun:_vgnU_freeres + fun:__run_exit_handlers + fun:exit + fun:ngx_master_process_exit + fun:ngx_single_process_cycle + fun:main +} +{ + <insert_a_suppression_name_here> + Memcheck:Param + epoll_ctl(event) + fun:epoll_ctl + fun:ngx_epoll_test_rdhup + fun:ngx_epoll_init + fun:ngx_event_process_init + fun:ngx_single_process_cycle + fun:main +} +{ + <insert_a_suppression_name_here> + Memcheck:Param + epoll_ctl(event) + fun:epoll_ctl + fun:ngx_epoll_test_rdhup + fun:ngx_epoll_init + fun:ngx_event_process_init + fun:ngx_single_process_cycle + fun:main +}