Merge pull request #10469 from zhongwencool/port-authn-http-validation

Port authn http validation

apps/emqx_authn/src/emqx_authn_schema.erl

@@ -18,10 +18,12 @@
 
 -elvis([{elvis_style, invalid_dynamic_call, disable}]).
 -include_lib("hocon/include/hoconsc.hrl").
+-include("emqx_authn.hrl").
 
 -export([
     common_fields/0,
     roots/0,
+    validations/0,
     tags/0,
     fields/1,
     authenticator_type/0,
@@ -207,3 +209,27 @@ array(Name) ->
 
 array(Name, DescId) ->
     {Name, ?HOCON(?R_REF(Name), #{desc => ?DESC(DescId)})}.
+
+validations() ->
+    [
+        {check_http_ssl_opts, fun(Conf) ->
+            CheckFun = fun emqx_authn_http:check_ssl_opts/1,
+            validation(Conf, CheckFun)
+        end},
+        {check_http_headers, fun(Conf) ->
+            CheckFun = fun emqx_authn_http:check_headers/1,
+            validation(Conf, CheckFun)
+        end}
+    ].
+
+validation(Conf, CheckFun) when is_map(Conf) ->
+    validation(hocon_maps:get(?CONF_NS, Conf), CheckFun);
+validation(undefined, _) ->
+    ok;
+validation([], _) ->
+    ok;
+validation([AuthN | Tail], CheckFun) ->
+    case CheckFun(#{?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY => AuthN}) of
+        ok -> validation(Tail, CheckFun);
+        Error -> Error
+    end.

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -38,6 +38,8 @@
     headers/1
 ]).
 
+-export([check_headers/1, check_ssl_opts/1]).
+
 -export([
     refs/0,
     union_member_selector/1,
@@ -106,8 +108,8 @@ common_fields() ->
 
 validations() ->
     [
-        {check_ssl_opts, fun check_ssl_opts/1},
+        {check_ssl_opts, fun ?MODULE:check_ssl_opts/1},
-        {check_headers, fun check_headers/1}
+        {check_headers, fun ?MODULE:check_headers/1}
     ].
 
 url(type) -> binary();
@@ -261,21 +263,47 @@ transform_header_name(Headers) ->
     ).
 
 check_ssl_opts(Conf) ->
-    {BaseUrl, _Path, _Query} = parse_url(get_conf_val("url", Conf)),
+    case is_backend_http(Conf) of
-    case BaseUrl of
+        true ->
-        <<"https://", _/binary>> ->
+            Url = get_conf_val("url", Conf),
-            case get_conf_val("ssl.enable", Conf) of
+            {BaseUrl, _Path, _Query} = parse_url(Url),
-                true -> ok;
+            case BaseUrl of
-                false -> false
+                <<"https://", _/binary>> ->
+                    case get_conf_val("ssl.enable", Conf) of
+                        true ->
+                            ok;
+                        false ->
+                            <<"it's required to enable the TLS option to establish a https connection">>
+                    end;
+                <<"http://", _/binary>> ->
+                    ok
             end;
-        <<"http://", _/binary>> ->
+        false ->
             ok
     end.
 
 check_headers(Conf) ->
-    Method = to_bin(get_conf_val("method", Conf)),
+    case is_backend_http(Conf) of
-    Headers = get_conf_val("headers", Conf),
+        true ->
-    Method =:= <<"post">> orelse (not maps:is_key(<<"content-type">>, Headers)).
+            Headers = get_conf_val("headers", Conf),
+            case to_bin(get_conf_val("method", Conf)) of
+                <<"post">> ->
+                    ok;
+                <<"get">> ->
+                    case maps:is_key(<<"content-type">>, Headers) of
+                        false -> ok;
+                        true -> <<"HTTP GET requests cannot include content-type header.">>
+                    end
+            end;
+        false ->
+            ok
+    end.
+
+is_backend_http(Conf) ->
+    case get_conf_val("backend", Conf) of
+        http -> true;
+        _ -> false
+    end.
 
 parse_url(Url) ->
     case string:split(Url, "//", leading) of
@@ -310,7 +338,7 @@ parse_config(
         method => Method,
         path => Path,
         headers => ensure_header_name_type(Headers),
-        base_path_templete => emqx_authn_utils:parse_str(Path),
+        base_path_template => emqx_authn_utils:parse_str(Path),
         base_query_template => emqx_authn_utils:parse_deep(
             cow_qs:parse_qs(to_bin(Query))
         ),
@@ -323,7 +351,7 @@ parse_config(
 generate_request(Credential, #{
     method := Method,
     headers := Headers0,
-    base_path_templete := BasePathTemplate,
+    base_path_template := BasePathTemplate,
     base_query_template := BaseQueryTemplate,
     body_template := BodyTemplate
 }) ->

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -6,6 +6,116 @@
 
 -include_lib("eunit/include/eunit.hrl").
 
+%% erlfmt-ignore
+-define(BASE_CONF,
+    """
+             node {
+                name = \"emqx1@127.0.0.1\"
+                cookie = \"emqxsecretcookie\"
+                data_dir = \"data\"
+             }
+             cluster {
+                name = emqxcl
+                discovery_strategy = static
+                static.seeds = ~p
+                core_nodes = ~p
+             }
+    """).
+
+array_nodes_test() ->
+    ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'],
+    lists:foreach(
+        fun({Seeds, Nodes}) ->
+            ConfFile = to_bin(?BASE_CONF, [Seeds, Nodes]),
+            {ok, Conf} = hocon:binary(ConfFile, #{format => richmap}),
+            ConfList = hocon_tconf:generate(emqx_conf_schema, Conf),
+            ClusterDiscovery = proplists:get_value(
+                cluster_discovery, proplists:get_value(ekka, ConfList)
+            ),
+            ?assertEqual(
+                {static, [{seeds, ExpectNodes}]},
+                ClusterDiscovery,
+                Nodes
+            ),
+            ?assertEqual(
+                ExpectNodes,
+                proplists:get_value(core_nodes, proplists:get_value(mria, ConfList)),
+                Nodes
+            )
+        end,
+        [{["emqx1@127.0.0.1", "emqx2@127.0.0.1"], "emqx1@127.0.0.1, emqx2@127.0.0.1"}]
+    ),
+    ok.
+
+%% erlfmt-ignore
+-define(BASE_AUTHN_ARRAY,
+    """
+        authentication = [
+          {backend = \"http\"
+          body {password = \"${password}\", username = \"${username}\"}
+          connect_timeout = \"15s\"
+          enable_pipelining = 100
+          headers {\"content-type\" = \"application/json\"}
+          mechanism = \"password_based\"
+          method = \"~p\"
+          pool_size = 8
+          request_timeout = \"5s\"
+          ssl {enable = ~p, verify = \"verify_peer\"}
+          url = \"~ts\"
+        }
+        ]
+    """
+).
+
+-define(ERROR(Reason),
+    {emqx_conf_schema, [
+        #{
+            kind := validation_error,
+            reason := integrity_validation_failure,
+            result := _,
+            validation_name := Reason
+        }
+    ]}
+).
+
+authn_validations_test() ->
+    BaseConf = to_bin(?BASE_CONF, [["emqx1@127.0.0.1"], "emqx1@127.0.0.1,emqx1@127.0.0.1"]),
+
+    OKHttps = to_bin(?BASE_AUTHN_ARRAY, [post, true, <<"https://127.0.0.1:8080">>]),
+    Conf0 = <<BaseConf/binary, OKHttps/binary>>,
+    {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
+    ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap0))),
+
+    OKHttp = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"http://127.0.0.1:8080">>]),
+    Conf1 = <<BaseConf/binary, OKHttp/binary>>,
+    {ok, ConfMap1} = hocon:binary(Conf1, #{format => richmap}),
+    ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap1))),
+
+    DisableSSLWithHttps = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"https://127.0.0.1:8080">>]),
+    Conf2 = <<BaseConf/binary, DisableSSLWithHttps/binary>>,
+    {ok, ConfMap2} = hocon:binary(Conf2, #{format => richmap}),
+    ?assertThrow(
+        ?ERROR(check_http_ssl_opts),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap2)
+    ),
+
+    BadHeader = to_bin(?BASE_AUTHN_ARRAY, [get, true, <<"https://127.0.0.1:8080">>]),
+    Conf3 = <<BaseConf/binary, BadHeader/binary>>,
+    {ok, ConfMap3} = hocon:binary(Conf3, #{format => richmap}),
+    ?assertThrow(
+        ?ERROR(check_http_headers),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap3)
+    ),
+
+    BadHeaderWithTuple = binary:replace(BadHeader, [<<"[">>, <<"]">>], <<"">>, [global]),
+    Conf4 = <<BaseConf/binary, BadHeaderWithTuple/binary>>,
+    {ok, ConfMap4} = hocon:binary(Conf4, #{format => richmap}),
+    ?assertThrow(
+        ?ERROR(check_http_headers),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap4)
+    ),
+    ok.
+
 doc_gen_test() ->
     %% the json file too large to encode.
     {
@@ -26,3 +136,6 @@ doc_gen_test() ->
             ok
         end
     }.
+
+to_bin(Format, Args) ->
+    iolist_to_binary(io_lib:format(Format, Args)).

apps/emqx_prometheus/TODO

@@ -1,2 +0,0 @@
-1. Add more VM Metrics
-2. Add more emqx Metrics

changes/ce/fix-10449.en.md

@@ -0,0 +1,2 @@
+Validate the ssl_options and header configurations when creating authentication http (`authn_http`).
+Prior to this, incorrect `ssl` configuration could result in successful creation but the entire authn being unusable.