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.

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

Do not change the options in mongoose_config #3356

Merged
merged 9 commits into from
Oct 26, 2021
4 changes: 2 additions & 2 deletions big_tests/tests/gdpr_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ retrieve_mam_pm_and_muc_light_dont_interfere(Config) ->

[mam_helper:wait_for_archive_size(User, 2) || User <- [Alice, Bob]],

false = mongoose_helper:successful_rpc(mod_mam_meta, get_mam_module_opt,
false = mongoose_helper:successful_rpc(gen_mod, get_module_opt,
[host_type(), mod_mam, archive_groupchats, undefined]),

AliceDir = retrieve_all_personal_data(Alice, Config),
Expand Down Expand Up @@ -890,7 +890,7 @@ retrieve_mam_pm_and_muc_light_interfere(Config) ->
[mam_helper:wait_for_archive_size(User, 5) || User <- [Alice, Bob]],
mam_helper:wait_for_archive_size(Kate, 3),

true = mongoose_helper:successful_rpc(mod_mam_meta, get_mam_module_opt,
true = mongoose_helper:successful_rpc(gen_mod, get_module_opt,
[host_type(), mod_mam, archive_groupchats, undefined]),

AliceDir = retrieve_all_personal_data(Alice, Config),
Expand Down
66 changes: 20 additions & 46 deletions big_tests/tests/service_mongoose_system_metrics_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,8 @@
periodic_report_available/1,
all_clustered_mongooses_report_the_same_client_id/1,
system_metrics_are_reported_to_google_analytics_when_mim_starts/1,
system_metrics_are_reported_to_additional_google_analytics/1,
system_metrics_are_reported_to_configurable_google_analytics/1,
system_metrics_are_reported_to_a_json_file/1,
tracking_id_is_correctly_configured/1,
module_backend_is_reported/1,
mongoose_version_is_reported/1,
cluster_uptime_is_reported/1,
Expand Down Expand Up @@ -78,10 +76,8 @@ all() ->
periodic_report_available,
all_clustered_mongooses_report_the_same_client_id,
system_metrics_are_reported_to_google_analytics_when_mim_starts,
system_metrics_are_reported_to_additional_google_analytics,
system_metrics_are_reported_to_configurable_google_analytics,
system_metrics_are_reported_to_a_json_file,
tracking_id_is_correctly_configured,
module_backend_is_reported,
mongoose_version_is_reported,
cluster_uptime_is_reported,
Expand Down Expand Up @@ -154,11 +150,6 @@ init_per_testcase(all_clustered_mongooses_report_the_same_client_id, Config) ->
enable_system_metrics(mim()),
enable_system_metrics(mim2()),
Config;
init_per_testcase(system_metrics_are_reported_to_additional_google_analytics, Config) ->
create_events_collection(),
enable_system_metrics(mim()),
configure_additional_tracking_id(mim()),
Config;
init_per_testcase(system_metrics_are_reported_to_configurable_google_analytics, Config) ->
create_events_collection(),
enable_system_metrics_with_configurable_tracking_id(mim()),
Expand Down Expand Up @@ -194,11 +185,6 @@ end_per_testcase(all_clustered_mongooses_report_the_same_client_id , Config) ->
[ begin delete_prev_client_id(Node), disable_system_metrics(Node) end || Node <- Nodes ],
distributed_helper:remove_node_from_cluster(mim2(), Config),
Config;
end_per_testcase(system_metrics_are_reported_to_additional_google_analytics, Config) ->
clear_events_collection(),
remove_additional_tracking_id(mim()),
disable_system_metrics(mim()),
Config;
end_per_testcase(xmpp_stanzas_counts_are_reported = CN, Config) ->
clear_events_collection(),
disable_system_metrics(mim()),
Expand Down Expand Up @@ -233,23 +219,14 @@ all_clustered_mongooses_report_the_same_client_id(_Config) ->
system_metrics_are_reported_to_google_analytics_when_mim_starts(_Config) ->
mongoose_helper:wait_until(fun hosts_count_is_reported/0, true),
mongoose_helper:wait_until(fun modules_are_reported/0, true),
events_are_reported_to_primary_tracking_id(),
all_event_have_the_same_client_id().

tracking_id_is_correctly_configured(_Config) ->
TrackingId = distributed_helper:rpc(mim(), mongoose_config, get_opt, [google_analytics_tracking_id]),
case os:getenv("CI") of
"true" ->
?assertEqual(?TRACKING_ID_CI, TrackingId);
_ ->
?assertEqual(?TRACKING_ID, TrackingId)
end.

system_metrics_are_reported_to_additional_google_analytics(_Config) ->
mongoose_helper:wait_until(fun events_are_reported_to_additional_tracking_id/0, true).

system_metrics_are_reported_to_configurable_google_analytics(_Config) ->
mongoose_helper:wait_until(fun events_are_reported_to_additional_tracking_id/0, true),
mongoose_helper:wait_until(fun events_are_reported_to_configurable_tracking_id/0, true).
mongoose_helper:wait_until(fun hosts_count_is_reported/0, true),
mongoose_helper:wait_until(fun modules_are_reported/0, true),
events_are_reported_to_both_tracking_ids(),
all_event_have_the_same_client_id().

system_metrics_are_reported_to_a_json_file(_Config) ->
ReportFilePath = distributed_helper:rpc(mim(), mongoose_system_metrics_file, location, []),
Expand Down Expand Up @@ -425,31 +402,28 @@ system_metrics_service_is_enabled(Node) ->
system_metrics_service_is_disabled(Node) ->
not system_metrics_service_is_enabled(Node).

configure_additional_tracking_id(Node) ->
TrackingIdArgs = [extra_google_analytics_tracking_id, ?TRACKING_ID_EXTRA],
ok = mongoose_helper:successful_rpc(Node, mongoose_config, set_opt, TrackingIdArgs).

remove_additional_tracking_id(Node) ->
mongoose_helper:successful_rpc(
Node, mongoose_config, unset_opt, [ extra_google_analytics_tracking_id ]).

remove_service_from_config(Service) ->
Services = distributed_helper:rpc(mim3(), mongoose_config, get_opt, [services]),
NewServices = proplists:delete(Service, Services),
distributed_helper:rpc(mim3(), mongoose_config, set_opt, [services, NewServices]).

events_are_reported_to_additional_tracking_id() ->
Tab = ets:tab2list(?ETS_TABLE),
SetTab = sets:from_list([Tid || #event{tid = Tid} <- Tab]),
2 >= sets:size(SetTab).
events_are_reported_to_primary_tracking_id() ->
events_are_reported_to_tracking_ids([primary_tracking_id()]).

events_are_reported_to_both_tracking_ids() ->
events_are_reported_to_tracking_ids([primary_tracking_id(), ?TRACKING_ID_EXTRA]).

events_are_reported_to_configurable_tracking_id() ->
ConfigurableTrackingId = <<?TRACKING_ID_EXTRA>>,
primary_tracking_id() ->
case os:getenv("CI") of
"true" -> ?TRACKING_ID_CI;
_ -> ?TRACKING_ID
end.

events_are_reported_to_tracking_ids(ConfiguredTrackingIds) ->
Tab = ets:tab2list(?ETS_TABLE),
lists:any(
fun(#event{tid = TrackingId}) ->
TrackingId == ConfigurableTrackingId
end, Tab).
ActualTrackingIds = lists:usort([Tid || #event{tid = Tid} <- Tab]),
ExpectedTrackingIds = lists:sort([list_to_binary(Tid) || Tid <- ConfiguredTrackingIds]),
?assertEqual(ExpectedTrackingIds, ActualTrackingIds).

maybe_start_module(Module) ->
Options = [],
Expand Down
4 changes: 2 additions & 2 deletions big_tests/tests/vcard_helper.erl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
-compile([export_all, nowarn_export_all]).

-import(distributed_helper, [rpc/4, mim/0]).
-import(domain_helper, [host_type/0]).

is_vcard_ldap() ->
ldap == rpc(mim(), gen_mod, get_module_opt,
[ct:get_config({hosts, mim, domain}), mod_vcard, backend, mnesia]).
ldap == rpc(mim(), gen_mod, get_module_opt, [host_type(), mod_vcard, backend, mnesia]).
11 changes: 0 additions & 11 deletions src/auth/ejabberd_auth.erl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
-export([start/0,
start/1,
stop/1,
set_opts/2,
get_opt/3,
get_opt/2,
authorize/1,
Expand Down Expand Up @@ -121,16 +120,6 @@ hooks(HostType) ->
{remove_domain, HostType, ?MODULE, remove_domain, 10}
].

-spec set_opts(HostType :: mongooseim:host_type(),
KVs :: [tuple()]) -> ok.
set_opts(HostType, KVs) ->
OldOpts = mongoose_config:get_opt({auth_opts, HostType}),
AccFunc = fun({K, V}, Acc) ->
lists:keystore(K, 1, Acc, {K, V})
end,
NewOpts = lists:foldl(AccFunc, OldOpts, KVs),
mongoose_config:set_opt({auth_opts, HostType}, NewOpts).

-spec get_opt(HostType :: mongooseim:host_type(),
Opt :: atom(),
Default :: ejabberd:value()) -> undefined | ejabberd:value().
Expand Down
11 changes: 3 additions & 8 deletions src/auth/ejabberd_auth_jwt.erl
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,13 @@

-spec start(HostType :: mongooseim:host_type()) -> ok.
start(HostType) ->
UsernameKey = ejabberd_auth:get_opt(HostType, jwt_username_key),
true = is_atom(UsernameKey) andalso UsernameKey /= undefined,

JWTSecret = get_jwt_secret(HostType),
JWTAlgorithm = ejabberd_auth:get_opt(HostType, jwt_algorithm),
ejabberd_auth:set_opts(HostType,
[{jwt_secret, JWTSecret},
{jwt_algorithm, list_to_binary(JWTAlgorithm)}]),
persistent_term:put({?MODULE, HostType, jwt_secret}, JWTSecret),
ok.

-spec stop(HostType :: mongooseim:host_type()) -> ok.
stop(_HostType) ->
persistent_term:erase(jwt_secret),
ok.

-spec supports_sasl_module(binary(), cyrsasl:sasl_module()) -> boolean().
Expand All @@ -75,7 +70,7 @@ authorize(Creds) ->
LServer :: jid:lserver(),
Password :: binary()) -> boolean().
check_password(HostType, LUser, LServer, Password) ->
Key = case ejabberd_auth:get_opt(HostType, jwt_secret) of
Key = case persistent_term:get({?MODULE, HostType, jwt_secret}) of
Key1 when is_binary(Key1) -> Key1;
{env, Var} -> list_to_binary(os:getenv(Var))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Considering that env-vars are private to a process and they're impossible to change externally, I'd also cache the value of the env-var in the persistent_term entry, instead of repeatedly reading such env-var. The reason why somebody would prefer deploying the secret as an env-var is that UNIX process memory has much stronger security semantics than filesystem permissions. Otherwise, the value might be kept cached in the process memory for much faster access.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mostly agree, but for me this modification would be out of scope of this PR.

end,
Expand Down
5 changes: 2 additions & 3 deletions src/config/mongoose_config.erl
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@
get_opt/1,
get_opt/2]).

%% Test API - currently used by some modules, such usage will be eliminated
%% Options set here are not cleaned up by stop/0
%% Test API, do not use outside of test suites, options set here are not cleaned up by stop/0
-export([set_opt/2,
unset_opt/1]).

%% Shell utilities intended for debugging and system inspection
-export([config_state/0,
config_states/0]).

-ignore_xref([config_state/0, config_states/0]).
-ignore_xref([set_opt/2, unset_opt/1, config_state/0, config_states/0]).

-include("mongoose.hrl").
-include("ejabberd_config.hrl").
Expand Down
9 changes: 5 additions & 4 deletions src/config/mongoose_config_spec.erl
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,11 @@ auth_http() ->
auth_jwt() ->
#section{
items = #{<<"secret">> => auth_jwt_secret(),
<<"algorithm">> => #option{type = string,
validate = {enum, ["HS256", "RS256", "ES256",
"HS386", "RS386", "ES386",
"HS512", "RS512", "ES512"]},
<<"algorithm">> => #option{type = binary,
validate = {enum,
[<<"HS256">>, <<"RS256">>, <<"ES256">>,
<<"HS386">>, <<"RS386">>, <<"ES386">>,
<<"HS512">>, <<"RS512">>, <<"ES512">>]},
format = {kv, jwt_algorithm}},
<<"username_key">> => #option{type = atom,
validate = non_empty,
Expand Down
2 changes: 1 addition & 1 deletion src/ejabberd_app.erl
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ stop_modules() ->
fun(Host) ->
StopModuleFun =
fun({Module, _Args}) ->
gen_mod:stop_module_keep_config(Host, Module)
gen_mod:stop_module(Host, Module)
end,
case mongoose_config:lookup_opt({modules, Host}) of
{error, not_found} ->
Expand Down
46 changes: 2 additions & 44 deletions src/ejabberd_listener.erl
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,13 @@
start_listener/3,
stop_listeners/0,
stop_listener/2,
parse_listener_portip/2,
add_listener/3,
delete_listener/3,
delete_listener/2
parse_listener_portip/2
]).

%% Internal
-export([format_error/1, socket_error/6, opts_to_listener_args/2]).

-ignore_xref([add_listener/3, delete_listener/2, delete_listener/3, init/1,
-ignore_xref([init/1,
opts_to_listener_args/2, start_link/0, start_listener/3, stop_listener/2]).

-export_type([port_ip_proto/0]).
Expand Down Expand Up @@ -272,45 +269,6 @@ stop_listener(PortIPProto, _Module) ->
supervisor:terminate_child(ejabberd_listeners, PortIPProto),
supervisor:delete_child(ejabberd_listeners, PortIPProto).

%% @doc Add a listener and store in config if success
-type listener_option() :: inet | inet6 | {ip, tuple()} | atom() | tuple().
-spec add_listener(PortIPProto :: port_ip_proto(),
Module :: atom(),
Opts :: [listener_option()]
) -> 'ok' | {'error', _}.
add_listener(PortIPProto, Module, Opts) ->
{Port, IPT, _, _, Proto, _} = parse_listener_portip(PortIPProto, Opts),
PortIP1 = {Port, IPT, Proto},
case start_listener(PortIP1, Module, Opts) of
{ok, _Pid} ->
Ports = mongoose_config:get_opt(listen, []),
Ports1 = lists:keydelete(PortIP1, 1, Ports),
Ports2 = [{PortIP1, Module, Opts} | Ports1],
mongoose_config:set_opt(listen, Ports2),
ok;
{error, Error} ->
{error, Error}
end.

-spec delete_listener(PortIPProto :: port_ip_proto(),
Module :: atom())
-> 'ok' | {'error', 'not_found' | 'restarting' | 'running' | 'simple_one_for_one'}.
delete_listener(PortIPProto, Module) ->
delete_listener(PortIPProto, Module, []).

-spec delete_listener(PortIPProto :: port_ip_proto(),
Module :: atom(),
Opts :: [listener_option()])
-> 'ok' | {'error', 'not_found' | 'restarting' | 'running' | 'simple_one_for_one'}.
delete_listener(PortIPProto, Module, Opts) ->
%% this one stops a listener and deletes it from configuration, used while reloading config
{Port, IPT, _, _, Proto, _} = parse_listener_portip(PortIPProto, Opts),
PortIP1 = {Port, IPT, Proto},
Ports = mongoose_config:get_opt(listen, []),
Ports1 = lists:keydelete(PortIP1, 1, Ports),
mongoose_config:set_opt(listen, Ports1),
stop_listener(PortIP1, Module).

%%%
%%% Check options
%%%
Expand Down
35 changes: 23 additions & 12 deletions src/ejabberd_s2s.erl
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@
fromto :: fromto(),
pid :: pid()
}.
-record(s2s_shared, {
scope = global :: global, % this might become per host type
secret :: binary()
}).
-record(state, {}).

%%====================================================================
Expand Down Expand Up @@ -177,7 +181,7 @@ node_cleanup(Acc, Node) ->
-spec key({jid:lserver(), jid:lserver()}, binary()) ->
binary().
key({From, To}, StreamID) ->
Secret = get_or_generate_secret(),
Secret = get_shared_secret(),
SecretHashed = base16:encode(crypto:hash(sha256, Secret)),
HMac = crypto:mac(hmac, sha256, SecretHashed, [From, " ", To, " ", StreamID]),
NelsonVides marked this conversation as resolved.
Show resolved Hide resolved
base16:encode(HMac).
Expand All @@ -197,6 +201,10 @@ init([]) ->
mnesia:create_table(s2s, [{ram_copies, [node()]}, {type, bag},
{attributes, record_info(fields, s2s)}]),
mnesia:add_table_copy(s2s, node(), ram_copies),
mnesia:create_table(s2s_shared, [{ram_copies, [node()]},
{attributes, record_info(fields, s2s_shared)}]),
mnesia:add_table_copy(s2s_shared, node(), ram_copies),
{atomic, ok} = set_shared_secret(),
ejabberd_commands:register_commands(commands()),
ejabberd_hooks:add(node_cleanup, global, ?MODULE, node_cleanup, 50),
{ok, #state{}}.
Expand Down Expand Up @@ -594,15 +602,18 @@ get_s2s_state(S2sPid)->
end,
[{s2s_pid, S2sPid} | Infos].

get_or_generate_secret() ->
case mongoose_config:lookup_opt(s2s_shared) of
{error, not_found} ->
generate_and_store_secret();
{ok, Secret} ->
Secret
end.

generate_and_store_secret() ->
Secret = base16:encode(crypto:strong_rand_bytes(10)),
mongoose_config:set_opt(s2s_shared, Secret),
-spec get_shared_secret() -> binary().
get_shared_secret() ->
%% Currently there is only one key, in the future there might be one key per host type
[#s2s_shared{secret = Secret}] = ets:lookup(s2s_shared, global),
Secret.

-spec set_shared_secret() -> mnesia:t_result(ok).
set_shared_secret() ->
Secret = case mongoose_config:lookup_opt(s2s_shared) of
{ok, SecretFromConfig} ->
SecretFromConfig;
{error, not_found} ->
base16:encode(crypto:strong_rand_bytes(10))
NelsonVides marked this conversation as resolved.
Show resolved Hide resolved
end,
mnesia:transaction(fun() -> mnesia:write(#s2s_shared{secret = Secret}) end).
Loading