From 6b9fb6511b4209a225b37513125cc8fa9675de8e Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Mon, 4 Dec 2023 13:08:11 +0100 Subject: [PATCH 1/3] Change domain validation logic - Changed domain validation logic - Moved some helpers to `test/common` to use ct:logger_backend in small tests --- src/config/mongoose_config_validator.erl | 54 ++++++++++++------- .../common}/distributed_helper.erl | 0 .../common}/logger_ct_backend.erl | 0 .../tests => test/common}/mongoose_helper.erl | 0 test/config_parser_SUITE.erl | 34 +++++++++++- 5 files changed, 66 insertions(+), 22 deletions(-) rename {big_tests/tests => test/common}/distributed_helper.erl (100%) rename {big_tests/tests => test/common}/logger_ct_backend.erl (100%) rename {big_tests/tests => test/common}/mongoose_helper.erl (100%) diff --git a/src/config/mongoose_config_validator.erl b/src/config/mongoose_config_validator.erl index 241255cefc..fd20ceb780 100644 --- a/src/config/mongoose_config_validator.erl +++ b/src/config/mongoose_config_validator.erl @@ -20,7 +20,7 @@ -spec validate(mongoose_config_parser_toml:option_value(), mongoose_config_spec:option_type(), validator()) -> any(). -validate(V, binary, domain) -> validate_binary_domain(V); +validate(V, binary, domain) -> validate_domain(V); validate(V, binary, url) -> validate_non_empty_binary(V); validate(V, binary, non_empty) -> validate_non_empty_binary(V); validate(V, binary, subdomain_template) -> validate_subdomain_template(V); @@ -122,19 +122,43 @@ validate_jid(Jid) -> validate_ldap_filter(Value) -> {ok, _} = eldap_filter:parse(Value). -validate_domain(Domain) when is_list(Domain) -> - #jid{luser = <<>>, lresource = <<>>} = jid:from_binary(list_to_binary(Domain)), - validate_domain_res(Domain). +validate_subdomain_template(SubdomainTemplate) -> + case mongoose_subdomain_utils:make_subdomain_pattern(SubdomainTemplate) of + {fqdn, Domain} -> + validate_domain(Domain); + Pattern -> + Domain = binary_to_list(mongoose_subdomain_utils:get_fqdn(Pattern, <<"example.com">>)), + case inet_parse:domain(Domain) of + true -> + ok; + false -> + error(#{what => validate_subdomain_template_failed, + text => <<"Invalid subdomain template">>, + subdomain_template => SubdomainTemplate}) + end + end. + +validate_domain(Domain) when is_binary(Domain) -> + validate_domain(binary_to_list(Domain)); +validate_domain(Domain) -> + validate_domain_name(Domain), + resolve_domain(Domain). + +validate_domain_name(Domain) -> + case inet_parse:domain(Domain) of + true -> + ok; + false -> + error(#{what => validate_domain_failed, + text => <<"Invalid domain name">>, + domain => Domain}) + end. -validate_domain_res(Domain) -> +resolve_domain(Domain) -> case inet_res:gethostbyname(Domain) of {ok, _} -> ok; - {error,formerr} -> - error(#{what => cfg_validate_domain_failed, - reason => formerr, text => <<"Invalid domain name">>, - domain => Domain}); - {error,Reason} -> %% timeout, nxdomain + {error, Reason} -> %% timeout, nxdomain ?LOG_WARNING(#{what => cfg_validate_domain, reason => Reason, domain => Domain, text => <<"Couldn't resolve domain. " @@ -142,16 +166,6 @@ validate_domain_res(Domain) -> ignore end. -validate_binary_domain(Domain) when is_binary(Domain) -> - #jid{luser = <<>>, lresource = <<>>} = jid:from_binary(Domain), - validate_domain_res(binary_to_list(Domain)). - -validate_subdomain_template(SubdomainTemplate) -> - Pattern = mongoose_subdomain_utils:make_subdomain_pattern(SubdomainTemplate), - Domain = mongoose_subdomain_utils:get_fqdn(Pattern, <<"example.com">>), - %% TODO: do we want warning printed by validate_domain_res, especially when - %% validating modules.mod_event_pusher_push.virtual_pubsub_hosts option - validate_binary_domain(Domain). validate_url(Url) -> validate_non_empty_string(Url). diff --git a/big_tests/tests/distributed_helper.erl b/test/common/distributed_helper.erl similarity index 100% rename from big_tests/tests/distributed_helper.erl rename to test/common/distributed_helper.erl diff --git a/big_tests/tests/logger_ct_backend.erl b/test/common/logger_ct_backend.erl similarity index 100% rename from big_tests/tests/logger_ct_backend.erl rename to test/common/logger_ct_backend.erl diff --git a/big_tests/tests/mongoose_helper.erl b/test/common/mongoose_helper.erl similarity index 100% rename from big_tests/tests/mongoose_helper.erl rename to test/common/mongoose_helper.erl diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 74240f8a5c..9a0424581b 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -56,7 +56,8 @@ all() -> {group, shaper_acl_access}, {group, s2s}, {group, modules}, - {group, services}]. + {group, services}, + {group, logs}]. groups() -> [{file, [parallel], [sample_pgsql, @@ -230,7 +231,8 @@ groups() -> modules_without_config, incorrect_module]}, {services, [parallel], [service_domain_db, - service_mongoose_system_metrics]} + service_mongoose_system_metrics]}, + {logs, [parallel], [no_warning_about_subdomain_patterns]} ]. init_per_suite(Config) -> @@ -1862,6 +1864,8 @@ mod_http_upload(_Config) -> ?errh(T(RequiredOpts#{<<"token_bytes">> => 0})), ?errh(T(RequiredOpts#{<<"max_file_size">> => 0})), ?errh(T(RequiredOpts#{<<"host">> => <<"is this a host? no.">>})), + ?errh(T(RequiredOpts#{<<"host">> => <<"invalid-.com">>})), + ?errh(T(RequiredOpts#{<<"host">> => <<"-invalid.com">>})), ?errh(T(RequiredOpts#{<<"host">> => [<<"invalid.sub@HOST@">>]})), ?errh(T(RequiredOpts#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), ?errh(T(RequiredOpts#{<<"host">> => [<<"not.supported.any.more.@HOSTS@">>]})), @@ -2281,8 +2285,10 @@ mod_muc_light(_Config) -> T(#{<<"rooms_in_rosters">> => true})), ?errh(T(#{<<"backend">> => <<"frontend">>})), ?errh(T(#{<<"host">> => <<"what is a domain?!">>})), + ?errh(T(#{<<"host">> => <<"invalid..com">>})), ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), + ?errh(T(#{<<"host">> => [<<"inv@lidsub.@HOST@">>]})), ?errh(T(#{<<"equal_occupants">> => <<"true">>})), ?errh(T(#{<<"legacy_mode">> => 1234})), ?errh(T(#{<<"rooms_per_user">> => 0})), @@ -2418,6 +2424,8 @@ mod_pubsub(_Config) -> test_wpool(P ++ [wpool], fun(Opts) -> T(#{<<"wpool">> => Opts}) end), ?errh(T(#{<<"host">> => <<"">>})), ?errh(T(#{<<"host">> => <<"is this a host? no.">>})), + ?errh(T(#{<<"host">> => <<"invalid domain.com">>})), + ?errh(T(#{<<"host">> => <<"inv@lid.com">>})), ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), ?errh(T(#{<<"backend">> => <<"amnesia">>})), @@ -2750,6 +2758,7 @@ mod_vcard(_Config) -> ?cfgh(P ++ [ldap, binary_search_fields], [<<"PHOTO">>], T(#{<<"backend">> => <<"ldap">>, <<"ldap">> => #{<<"binary_search_fields">> => [<<"PHOTO">>]}})), ?errh(T(#{<<"host">> => 1})), + ?errh(T(#{<<"host">> => <<" ">>})), ?errh(T(#{<<"host">> => <<"is this a host? no.">>})), ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), @@ -2871,6 +2880,27 @@ service_mongoose_system_metrics(_Config) -> ?err(T(#{<<"tracking_id">> => #{<<"secret">> => 666, <<"id">> => 666}})), ?err(T(#{<<"report">> => <<"maybe">>})). +no_warning_about_subdomain_patterns(_Config) -> + check_module_defaults(mod_vcard), + check_iqdisc(mod_vcard), + P = [modules, mod_vcard], + T = fun(Opts) -> #{<<"modules">> => #{<<"mod_vcard">> => Opts}} end, + + Node = #{node => mongooseim@localhost}, + logger_ct_backend:start(Node), + logger_ct_backend:capture(warning, Node), + + ?cfgh(P ++ [host], {prefix, <<"vjud.">>}, + T(#{<<"host">> => <<"vjud.@HOST@">>})), + + logger_ct_backend:stop_capture(Node), + logger_ct_backend:stop(Node), + + FilterFun = fun(_, Msg) -> + re:run(Msg, "example.com") /= nomatch + end, + [] = logger_ct_backend:recv(FilterFun). + %% Helpers for module tests check_iqdisc(Module) -> From 04a0f5d5eacb5866e9f0edd52434c0551451d31e Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Tue, 5 Dec 2023 09:53:34 +0100 Subject: [PATCH 2/3] Additional tests for domain validation --- test/config_parser_SUITE.erl | 43 ++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 9a0424581b..cc820d60cd 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -232,7 +232,8 @@ groups() -> incorrect_module]}, {services, [parallel], [service_domain_db, service_mongoose_system_metrics]}, - {logs, [parallel], [no_warning_about_subdomain_patterns]} + {logs, [], [no_warning_about_subdomain_patterns, + no_warning_for_resolvable_domain]} ]. init_per_suite(Config) -> @@ -2892,14 +2893,52 @@ no_warning_about_subdomain_patterns(_Config) -> ?cfgh(P ++ [host], {prefix, <<"vjud.">>}, T(#{<<"host">> => <<"vjud.@HOST@">>})), + ?cfgh(P ++ [host], {fqdn, <<"vjud.test">>}, + T(#{<<"host">> => <<"vjud.test">>})), logger_ct_backend:stop_capture(Node), logger_ct_backend:stop(Node), FilterFun = fun(_, Msg) -> + re:run(Msg, "test") /= nomatch orelse re:run(Msg, "example.com") /= nomatch end, - [] = logger_ct_backend:recv(FilterFun). + Logs = logger_ct_backend:recv(FilterFun), + + ?assertNotEqual(0, length(Logs)), + AnyContainsExampleCom = lists:any(fun({_, Msg}) -> + re:run(Msg, "example.com") /= nomatch + end, Logs), + ?eq(false, AnyContainsExampleCom). + +no_warning_for_resolvable_domain(_Config) -> + T = fun(Opts) -> #{<<"modules">> => #{<<"mod_http_upload">> => Opts}} end, + P = [modules, mod_http_upload], + RequiredOpts = #{<<"s3">> => http_upload_s3_required_opts()}, + + Node = #{node => mongooseim@localhost}, + logger_ct_backend:start(Node), + logger_ct_backend:capture(warning, Node), + + ?cfgh(P ++ [host], {fqdn, <<"example.org">>}, + T(RequiredOpts#{<<"host">> => <<"example.org">>})), + ?cfgh(P ++ [host], {fqdn, <<"something.invalid">>}, + T(RequiredOpts#{<<"host">> => <<"something.invalid">>})), + + logger_ct_backend:stop_capture(Node), + logger_ct_backend:stop(Node), + + FilterFun = fun(_, Msg) -> + re:run(Msg, "example.org") /= nomatch orelse + re:run(Msg, "something.invalid") /= nomatch + end, + Logs = logger_ct_backend:recv(FilterFun), + + ?assertNotEqual(0, length(Logs)), + ResolvableDomainInLogs = lists:any(fun({_, Msg}) -> + re:run(Msg, "example.org") /= nomatch + end, Logs), + ?eq(false, ResolvableDomainInLogs). %% Helpers for module tests From e0d1fd184db1b0a8cb8400d0a62bd00eff6f05eb Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Wed, 6 Dec 2023 08:45:08 +0100 Subject: [PATCH 3/3] Refactor invalid host testing --- test/config_parser_SUITE.erl | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index cc820d60cd..7861521d56 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -1867,8 +1867,9 @@ mod_http_upload(_Config) -> ?errh(T(RequiredOpts#{<<"host">> => <<"is this a host? no.">>})), ?errh(T(RequiredOpts#{<<"host">> => <<"invalid-.com">>})), ?errh(T(RequiredOpts#{<<"host">> => <<"-invalid.com">>})), - ?errh(T(RequiredOpts#{<<"host">> => [<<"invalid.sub@HOST@">>]})), - ?errh(T(RequiredOpts#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), + ?errh(T(RequiredOpts#{<<"host">> => [<<"valid.@HOST@">>]})), + ?errh(T(RequiredOpts#{<<"host">> => <<"invalid.sub@HOST@">>})), + ?errh(T(RequiredOpts#{<<"host">> => <<"invalid.sub.@HOST@.as.well">>})), ?errh(T(RequiredOpts#{<<"host">> => [<<"not.supported.any.more.@HOSTS@">>]})), check_iqdisc(mod_http_upload, RequiredOpts). @@ -1993,8 +1994,9 @@ mod_mam_muc(_Config) -> ?cfgh(P ++ [host], {prefix, <<"muc.">>}, T(#{<<"host">> => <<"muc.@HOST@">>})), ?cfgh(P ++ [host], {fqdn, <<"muc.test">>}, T(#{<<"host">> => <<"muc.test">>})), ?errh(T(#{<<"host">> => <<"is this a host? no.">>})), - ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), - ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), + ?errh(T(#{<<"host">> => [<<"valid.@HOST@">>]})), + ?errh(T(#{<<"host">> => <<"invalid.sub@HOST@">>})), + ?errh(T(#{<<"host">> => <<"invalid.sub.@HOST@.as.well">>})), ?errh(T(#{<<"archive_groupchats">> => true})), % pm-only ?errh(T(#{<<"same_mam_id_for_peers">> => true})). % pm-only @@ -2115,8 +2117,9 @@ mod_muc(_Config) -> T(<<"hibernated_room_timeout">>, 0)), ?errh(T(<<"host">>, <<>>)), ?errh(T(<<"host">>, <<"is this a host? no.">>)), - ?errh(T(<<"host">>, [<<"invalid.sub@HOST@">>])), - ?errh(T(<<"host">>, [<<"invalid.sub.@HOST@.as.well">>])), + ?errh(T(<<"host">>, [<<"valid.@HOST@">>])), + ?errh(T(<<"host">>, <<"invalid.sub@HOST@">>)), + ?errh(T(<<"host">>, <<"invalid.sub.@HOST@.as.well">>)), ?errh(T(<<"backend">>, <<"amnesia">>)), ?errh(T(<<"access">>, <<>>)), ?errh(T(<<"access_create">>, 1)), @@ -2287,9 +2290,10 @@ mod_muc_light(_Config) -> ?errh(T(#{<<"backend">> => <<"frontend">>})), ?errh(T(#{<<"host">> => <<"what is a domain?!">>})), ?errh(T(#{<<"host">> => <<"invalid..com">>})), - ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), - ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), - ?errh(T(#{<<"host">> => [<<"inv@lidsub.@HOST@">>]})), + ?errh(T(#{<<"host">> => [<<"valid.@HOST@">>]})), + ?errh(T(#{<<"host">> => <<"invalid.sub@HOST@">>})), + ?errh(T(#{<<"host">> => <<"invalid.sub.@HOST@.as.well">>})), + ?errh(T(#{<<"host">> => <<"inv@lidsub.@HOST@">>})), ?errh(T(#{<<"equal_occupants">> => <<"true">>})), ?errh(T(#{<<"legacy_mode">> => 1234})), ?errh(T(#{<<"rooms_per_user">> => 0})), @@ -2427,8 +2431,9 @@ mod_pubsub(_Config) -> ?errh(T(#{<<"host">> => <<"is this a host? no.">>})), ?errh(T(#{<<"host">> => <<"invalid domain.com">>})), ?errh(T(#{<<"host">> => <<"inv@lid.com">>})), - ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), - ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), + ?errh(T(#{<<"host">> => [<<"valid.@HOST@">>]})), + ?errh(T(#{<<"host">> => <<"invalid.sub@HOST@">>})), + ?errh(T(#{<<"host">> => <<"invalid.sub.@HOST@.as.well">>})), ?errh(T(#{<<"backend">> => <<"amnesia">>})), ?errh(T(#{<<"access_createnode">> => <<"">>})), ?errh(T(#{<<"max_items_node">> => -1})), @@ -2761,8 +2766,9 @@ mod_vcard(_Config) -> ?errh(T(#{<<"host">> => 1})), ?errh(T(#{<<"host">> => <<" ">>})), ?errh(T(#{<<"host">> => <<"is this a host? no.">>})), - ?errh(T(#{<<"host">> => [<<"invalid.sub@HOST@">>]})), - ?errh(T(#{<<"host">> => [<<"invalid.sub.@HOST@.as.well">>]})), + ?errh(T(#{<<"host">> => [<<"valid.@HOST@">>]})), + ?errh(T(#{<<"host">> => <<"invalid.sub@HOST@">>})), + ?errh(T(#{<<"host">> => <<"invalid.sub.@HOST@.as.well">>})), ?errh(T(#{<<"search">> => 1})), ?errh(T(#{<<"backend">> => <<"mememesia">>})), ?errh(T(#{<<"matches">> => -1})),