diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index f82aebe7c..18ddcaba2 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -441,6 +441,7 @@ post_config_update([listeners, Type, Name], {create, _Request}, NewConf, undefin start_listener(Type, Name, NewConf); post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf, _AppEnvs) -> try_clear_ssl_files(certs_dir(Type, Name), NewConf, OldConf), + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), case NewConf of #{enabled := true} -> restart_listener(Type, Name, {OldConf, NewConf}); _ -> ok @@ -448,6 +449,7 @@ post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf post_config_update([listeners, _Type, _Name], '$remove', undefined, undefined, _AppEnvs) -> ok; post_config_update([listeners, Type, Name], '$remove', undefined, OldConf, _AppEnvs) -> + ok = unregister_ocsp_stapling_refresh(Type, Name), case stop_listener(Type, Name, OldConf) of ok -> _ = emqx_authentication:delete_chain(listener_id(Type, Name)), @@ -460,10 +462,18 @@ post_config_update([listeners, Type, Name], {action, _Action, _}, NewConf, OldCo #{enabled := NewEnabled} = NewConf, #{enabled := OldEnabled} = OldConf, case {NewEnabled, OldEnabled} of - {true, true} -> restart_listener(Type, Name, {OldConf, NewConf}); - {true, false} -> start_listener(Type, Name, NewConf); - {false, true} -> stop_listener(Type, Name, OldConf); - {false, false} -> stop_listener(Type, Name, OldConf) + {true, true} -> + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), + restart_listener(Type, Name, {OldConf, NewConf}); + {true, false} -> + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), + start_listener(Type, Name, NewConf); + {false, true} -> + ok = unregister_ocsp_stapling_refresh(Type, Name), + stop_listener(Type, Name, OldConf); + {false, false} -> + ok = unregister_ocsp_stapling_refresh(Type, Name), + stop_listener(Type, Name, OldConf) end; post_config_update(_Path, _Request, _NewConf, _OldConf, _AppEnvs) -> ok. @@ -813,3 +823,16 @@ inject_crl_config( }; inject_crl_config(Conf) -> Conf. + +maybe_unregister_ocsp_stapling_refresh( + ssl = Type, Name, #{ssl_options := #{ocsp := #{enable_ocsp_stapling := false}}} = _Conf +) -> + unregister_ocsp_stapling_refresh(Type, Name), + ok; +maybe_unregister_ocsp_stapling_refresh(_Type, _Name, _Conf) -> + ok. + +unregister_ocsp_stapling_refresh(Type, Name) -> + ListenerId = listener_id(Type, Name), + emqx_ocsp_cache:unregister_listener(ListenerId), + ok. diff --git a/apps/emqx/src/emqx_ocsp_cache.erl b/apps/emqx/src/emqx_ocsp_cache.erl index 3bb10ee5c..ef0411b37 100644 --- a/apps/emqx/src/emqx_ocsp_cache.erl +++ b/apps/emqx/src/emqx_ocsp_cache.erl @@ -30,6 +30,7 @@ sni_fun/2, fetch_response/1, register_listener/2, + unregister_listener/1, inject_sni_fun/2 ]). @@ -107,6 +108,9 @@ fetch_response(ListenerID) -> register_listener(ListenerID, Opts) -> gen_server:call(?MODULE, {register_listener, ListenerID, Opts}, ?CALL_TIMEOUT). +unregister_listener(ListenerID) -> + gen_server:cast(?MODULE, {unregister_listener, ListenerID}). + -spec inject_sni_fun(emqx_listeners:listener_id(), map()) -> map(). inject_sni_fun(ListenerID, Conf0) -> SNIFun = emqx_const_v1:make_sni_fun(ListenerID), @@ -160,6 +164,18 @@ handle_call({register_listener, ListenerID, Conf}, _From, State0) -> handle_call(Call, _From, State) -> {reply, {error, {unknown_call, Call}}, State}. +handle_cast({unregister_listener, ListenerID}, State0) -> + State2 = + case maps:take(?REFRESH_TIMER(ListenerID), State0) of + error -> + State0; + {TRef, State1} -> + emqx_utils:cancel_timer(TRef), + State1 + end, + State = maps:remove({refresh_interval, ListenerID}, State2), + ?tp(ocsp_cache_listener_unregistered, #{listener_id => ListenerID}), + {noreply, State}; handle_cast(_Cast, State) -> {noreply, State}. diff --git a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl index 15ca29853..75c41b9fb 100644 --- a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl +++ b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl @@ -254,10 +254,15 @@ does_module_exist(Mod) -> end. assert_no_http_get() -> + Timeout = 0, + Error = should_be_cached, + assert_no_http_get(Timeout, Error). + +assert_no_http_get(Timeout, Error) -> receive {http_get, _URL} -> - error(should_be_cached) - after 0 -> + error(Error) + after Timeout -> ok end. @@ -702,7 +707,9 @@ do_t_update_listener(Config) -> %% the API converts that to an internally %% managed file <<"issuer_pem">> => IssuerPem, - <<"responder_url">> => <<"http://localhost:9877">> + <<"responder_url">> => <<"http://localhost:9877">>, + %% for quicker testing; min refresh in tests is 5 s. + <<"refresh_interval">> => <<"5s">> } } }, @@ -739,6 +746,70 @@ do_t_update_listener(Config) -> ) ), assert_http_get(1, 5_000), + + %% Disable OCSP Stapling; the periodic refreshes should stop + RefreshInterval = emqx_config:get([listeners, ssl, default, ssl_options, ocsp, refresh_interval]), + OCSPConfig1 = + #{ + <<"ssl_options">> => + #{ + <<"ocsp">> => + #{ + <<"enable_ocsp_stapling">> => false + } + } + }, + ListenerData3 = emqx_utils_maps:deep_merge(ListenerData2, OCSPConfig1), + {ok, {_, _, ListenerData4}} = update_listener_via_api(ListenerId, ListenerData3), + ?assertMatch( + #{ + <<"ssl_options">> := + #{ + <<"ocsp">> := + #{ + <<"enable_ocsp_stapling">> := false + } + } + }, + ListenerData4 + ), + + assert_no_http_get(2 * RefreshInterval, should_stop_refreshing), + + ok. + +t_double_unregister(_Config) -> + ListenerID = <<"ssl:test_ocsp">>, + Conf = emqx_config:get_listener_conf(ssl, test_ocsp, []), + ?check_trace( + begin + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:register_listener(ListenerID, Conf), + #{?snk_kind := ocsp_http_fetch_and_cache, listener_id := ListenerID}, + 5_000 + ), + assert_http_get(1), + + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:unregister_listener(ListenerID), + #{?snk_kind := ocsp_cache_listener_unregistered, listener_id := ListenerID}, + 5_000 + ), + + %% Should be idempotent and not crash + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:unregister_listener(ListenerID), + #{?snk_kind := ocsp_cache_listener_unregistered, listener_id := ListenerID}, + 5_000 + ), + ok + end, + [] + ), + ok. t_ocsp_responder_error_responses(_Config) ->