Merge pull request #9357 from thalesmg/fix-crl-http-timeout-global-rv44

fix(crl): make http timeout global for all listeners
This commit is contained in:
Thales Macedo Garitezi 2022-11-11 13:05:28 -03:00 committed by GitHub
commit 5c169d2060
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 153 additions and 23 deletions

View File

@ -1550,7 +1550,6 @@ listener.ssl.external.cacertfile = {{ platform_etc_dir }}/certs/cacert.pem
## listener.ssl.external.ocsp_refresh_http_timeout = 15s
## Whether to enable CRL verification and caching for this listener.
## If set to true, requires specifying the CRL server URLs.
##
## Value: boolean
## Default: false
@ -1562,11 +1561,12 @@ listener.ssl.external.cacertfile = {{ platform_etc_dir }}/certs/cacert.pem
## Value: String
## listener.ssl.external.crl_cache_urls = http://my.crl.server/intermediate.crl.pem, http://my.other.crl.server/another.crl.pem
## The timeout for the HTTP request when fetching CRLs.
## The timeout for the HTTP request when fetching CRLs. This is
## global for all listeners.
##
## Value: Duration
## Default: 15 s
## listener.ssl.external.crl_cache_http_timeout = 15s
## crl_cache.http_timeout = 15s
## The period to refresh the CRLs from the servers. This is global
## for all URLs and listeners.

View File

@ -1712,7 +1712,7 @@ end}.
{datatype, string}
]}.
{mapping, "listener.ssl.$name.crl_cache_http_timeout", "emqx.listeners", [
{mapping, "crl_cache.http_timeout", "emqx.crl_cache_http_timeout", [
{default, "15s"},
{datatype, {duration, ms}}
]}.
@ -2339,7 +2339,7 @@ end}.
end,
CRLCheck = case cuttlefish:conf_get(Prefix ++ ".enable_crl_check", Conf, false) of
true ->
HTTPTimeout = cuttlefish:conf_get(Prefix ++ ".crl_cache_http_timeout", Conf, timer:seconds(15)),
HTTPTimeout = cuttlefish:conf_get("crl_cache.http_timeout", Conf, timer:seconds(15)),
%% {crl_check, true} doesn't work
[ {crl_check, peer}
, {crl_cache, {ssl_crl_cache, {internal, [{http, HTTPTimeout}]}}}

View File

@ -23,6 +23,7 @@
, start_link/1
, refresh/1
, evict/1
, refresh_config/0
]).
%% gen_server callbacks
@ -41,12 +42,14 @@
-define(LOG(Level, Format, Args),
logger:log(Level, "[~p] " ++ Format, [?MODULE | Args])).
-define(HTTP_TIMEOUT, timer:seconds(10)).
-define(HTTP_TIMEOUT, timer:seconds(15)).
-define(RETRY_TIMEOUT, 5_000).
-record(state,
{ refresh_timers = #{} :: #{binary() => timer:tref()}
, refresh_interval = timer:minutes(15) :: timer:time()
, http_timeout = ?HTTP_TIMEOUT :: timer:time()
, extra = #{} :: map() %% for future use
}).
%%--------------------------------------------------------------------
@ -54,16 +57,11 @@
%%--------------------------------------------------------------------
start_link() ->
Listeners = emqx:get_env(listeners, []),
URLs = collect_urls(Listeners),
RefreshIntervalMS0 = emqx:get_env(crl_cache_refresh_interval,
timer:minutes(15)),
MinimumRefreshInverval = timer:minutes(1),
RefreshIntervalMS = max(RefreshIntervalMS0, MinimumRefreshInverval),
start_link(#{urls => URLs, refresh_interval => RefreshIntervalMS}).
Config = gather_config(),
start_link(Config).
start_link(Opts = #{urls := _, refresh_interval := _}) ->
gen_server:start_link({local, ?MODULE}, ?MODULE, Opts, []).
start_link(Config = #{urls := _, refresh_interval := _, http_timeout := _}) ->
gen_server:start_link({local, ?MODULE}, ?MODULE, Config, []).
refresh(URL) ->
gen_server:cast(?MODULE, {refresh, URL}).
@ -71,13 +69,24 @@ refresh(URL) ->
evict(URL) ->
gen_server:cast(?MODULE, {evict, URL}).
%% to pick up changes from the config
-spec refresh_config() -> ok.
refresh_config() ->
gen_server:cast(?MODULE, refresh_config).
%%--------------------------------------------------------------------
%% gen_server behaviour
%%--------------------------------------------------------------------
init(#{urls := URLs, refresh_interval := RefreshIntervalMS}) ->
init(Config) ->
#{ urls := URLs
, refresh_interval := RefreshIntervalMS
, http_timeout := HTTPTimeoutMS
} = Config,
State = lists:foldl(fun(URL, Acc) -> ensure_timer(URL, Acc, 0) end,
#state{refresh_interval = RefreshIntervalMS},
#state{ refresh_interval = RefreshIntervalMS
, http_timeout = HTTPTimeoutMS
},
URLs),
{ok, State}.
@ -95,7 +104,7 @@ handle_cast({evict, URL}, State0 = #state{refresh_timers = RefreshTimers0}) ->
}),
{noreply, State};
handle_cast({refresh, URL}, State0) ->
case do_http_fetch_and_cache(URL) of
case do_http_fetch_and_cache(URL, State0#state.http_timeout) of
{error, Error} ->
?tp(crl_refresh_failure, #{error => Error, url => URL}),
?LOG(error, "failed to fetch crl response for ~p; error: ~p",
@ -105,16 +114,33 @@ handle_cast({refresh, URL}, State0) ->
?LOG(debug, "fetched crl response for ~p", [URL]),
{noreply, ensure_timer(URL, State0)}
end;
handle_cast(refresh_config, State0) ->
#{ urls := URLs
, http_timeout := HTTPTimeoutMS
, refresh_interval := RefreshIntervalMS
} = gather_config(),
State = lists:foldl(fun(URL, Acc) -> ensure_timer(URL, Acc, 0) end,
State0#state{ refresh_interval = RefreshIntervalMS
, http_timeout = HTTPTimeoutMS
},
URLs),
?tp(crl_cache_refresh_config, #{ refresh_interval => RefreshIntervalMS
, http_timeout => HTTPTimeoutMS
, urls => URLs
}),
State;
handle_cast(_Cast, State) ->
{noreply, State}.
handle_info({timeout, TRef, {refresh, URL}},
State = #state{refresh_timers = RefreshTimers}) ->
State = #state{ refresh_timers = RefreshTimers
, http_timeout = HTTPTimeoutMS
}) ->
case maps:get(URL, RefreshTimers, undefined) of
TRef ->
?tp(crl_refresh_timer, #{url => URL}),
?LOG(debug, "refreshing crl response for ~p", [URL]),
case do_http_fetch_and_cache(URL) of
case do_http_fetch_and_cache(URL, HTTPTimeoutMS) of
{error, Error} ->
?LOG(error, "failed to fetch crl response for ~p; error: ~p",
[URL, Error]),
@ -142,10 +168,9 @@ http_get(URL, HTTPTimeout) ->
[{body_format, binary}]
).
do_http_fetch_and_cache(URL) ->
do_http_fetch_and_cache(URL, HTTPTimeoutMS) ->
?tp(crl_http_fetch, #{crl_url => URL}),
%% FIXME: read from config
Resp = ?MODULE:http_get(URL, ?HTTP_TIMEOUT),
Resp = ?MODULE:http_get(URL, HTTPTimeoutMS),
case Resp of
{ok, {{_, 200, _}, _, Body}} ->
case parse_crls(Body) of
@ -174,6 +199,7 @@ ensure_timer(URL, State = #state{refresh_interval = Timeout}) ->
ensure_timer(URL, State, Timeout).
ensure_timer(URL, State = #state{refresh_timers = RefreshTimers0}, Timeout) ->
?tp(crl_cache_ensure_timer, #{url => URL, timeout => Timeout}),
MTimer = maps:get(URL, RefreshTimers0, undefined),
emqx_misc:cancel_timer(MTimer),
RefreshTimers = RefreshTimers0#{URL => emqx_misc:start_timer(
@ -197,3 +223,20 @@ collect_urls(Listeners) ->
end,
CRLOpts1),
lists:usort(CRLURLs).
-spec gather_config() -> #{ urls := [string()]
, refresh_interval := timer:time()
, http_timeout := timer:time()
}.
gather_config() ->
Listeners = emqx:get_env(listeners, []),
URLs = collect_urls(Listeners),
RefreshIntervalMS0 = emqx:get_env(crl_cache_refresh_interval,
timer:minutes(15)),
MinimumRefreshInverval = timer:minutes(1),
RefreshIntervalMS = max(RefreshIntervalMS0, MinimumRefreshInverval),
HTTPTimeoutMS = emqx:get_env(crl_cache_http_timeout, ?HTTP_TIMEOUT),
#{ urls => URLs
, refresh_interval => RefreshIntervalMS
, http_timeout => HTTPTimeoutMS
}.

View File

@ -56,6 +56,29 @@ init_per_testcase(t_not_cached_and_unreachable, Config) ->
[ {crl_pem, CRLPem}
, {crl_der, CRLDer}
| Config];
init_per_testcase(t_refresh_config, Config) ->
DataDir = ?config(data_dir, Config),
CRLFile = filename:join([DataDir, "crl.pem"]),
{ok, CRLPem} = file:read_file(CRLFile),
[{'CertificateList', CRLDer, not_encrypted}] = public_key:pem_decode(CRLPem),
TestPid = self(),
ok = meck:new(emqx_crl_cache, [non_strict, passthrough, no_history, no_link]),
meck:expect(emqx_crl_cache, http_get,
fun(URL, _HTTPTimeout) ->
TestPid ! {http_get, URL},
{ok, {{"HTTP/1.0", 200, 'OK'}, [], CRLPem}}
end),
OldListeners = emqx:get_env(listeners),
OldRefreshInterval = emqx:get_env(crl_cache_refresh_interval),
OldHTTPTimeout = emqx:get_env(crl_cache_http_timeout),
ok = setup_crl_options(Config, #{is_cached => false}),
[ {crl_pem, CRLPem}
, {crl_der, CRLDer}
, {old_configs, [ {listeners, OldListeners}
, {crl_cache_refresh_interval, OldRefreshInterval}
, {crl_cache_http_timeout, OldHTTPTimeout}
]}
| Config];
init_per_testcase(_TestCase, Config) ->
DataDir = ?config(data_dir, Config),
CRLFile = filename:join([DataDir, "crl.pem"]),
@ -86,6 +109,7 @@ end_per_testcase(TestCase, Config)
]),
application:stop(cowboy),
clear_crl_cache(),
ok = snabbkaffe:stop(),
ok;
end_per_testcase(t_not_cached_and_unreachable, _Config) ->
emqx_ct_helpers:stop_apps([]),
@ -95,10 +119,34 @@ end_per_testcase(t_not_cached_and_unreachable, _Config) ->
]}
]),
clear_crl_cache(),
ok = snabbkaffe:stop(),
ok;
end_per_testcase(t_refresh_config, Config) ->
OldConfigs = ?config(old_configs, Config),
meck:unload([emqx_crl_cache]),
emqx_ct_helpers:stop_apps([]),
emqx_ct_helpers:change_emqx_opts(
ssl_twoway, [ {crl_options, [ {crl_check_enabled, false}
, {crl_cache_urls, []}
]}
]),
clear_crl_cache(),
lists:foreach(
fun({Key, MValue}) ->
case MValue of
undefined -> ok;
Value -> application:set_env(emqx, Key, Value)
end
end,
OldConfigs),
application:stop(cowboy),
clear_crl_cache(),
ok = snabbkaffe:stop(),
ok;
end_per_testcase(_TestCase, _Config) ->
meck:unload([emqx_crl_cache]),
clear_crl_cache(),
ok = snabbkaffe:stop(),
ok.
%%--------------------------------------------------------------------
@ -257,6 +305,7 @@ t_init_refresh(Config) ->
URL2 = "http://localhost/crl2.pem",
Opts = #{ urls => [URL1, URL2]
, refresh_interval => timer:minutes(15)
, http_timeout => timer:seconds(15)
},
ok = snabbkaffe:start_trace(),
{ok, SubRef} = snabbkaffe:subscribe(
@ -421,6 +470,44 @@ t_filled_cache(Config) ->
emqtt:disconnect(C),
ok.
t_refresh_config(_Config) ->
URLs = [ "http://localhost:9878/some.crl.pem"
, "http://localhost:9878/another.crl.pem"
],
SortedURLs = lists:sort(URLs),
emqx_ct_helpers:change_emqx_opts(
ssl_twoway, [ {crl_options, [ {crl_check_enabled, true}
, {crl_cache_urls, URLs}
]}
]),
%% has to be more than 1 minute
NewRefreshInterval = timer:seconds(64),
NewHTTPTimeout = timer:seconds(7),
application:set_env(emqx, crl_cache_refresh_interval, NewRefreshInterval),
application:set_env(emqx, crl_cache_http_timeout, NewHTTPTimeout),
?check_trace(
?wait_async_action(
emqx_crl_cache:refresh_config(),
#{?snk_kind := crl_cache_refresh_config},
_Timeout = 10_000),
fun(Res, Trace) ->
?assertMatch({ok, {ok, _}}, Res),
?assertMatch(
[#{ urls := SortedURLs
, refresh_interval := NewRefreshInterval
, http_timeout := NewHTTPTimeout
}],
?of_kind(crl_cache_refresh_config, Trace),
#{ expected => #{ urls => SortedURLs
, refresh_interval => NewRefreshInterval
, http_timeout => NewHTTPTimeout
}
}),
?assertEqual(SortedURLs, ?projection(url, ?of_kind(crl_cache_ensure_timer, Trace))),
ok
end),
ok.
%% If the CRL is not cached when the client tries to connect and the
%% CRL server is unreachable, the client will be denied connection.
t_not_cached_and_unreachable(Config) ->