Просмотр исходного кода

Merge pull request #10449 from zhongwencool/authn-http-validation

fix: always check authn_http's header and ssl_option
zhongwencool 2 лет назад
Родитель
Сommit
451934d280

+ 1 - 1
apps/emqx_authn/src/emqx_authn.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authn, [
     {description, "EMQX Authentication"},
-    {vsn, "0.1.17"},
+    {vsn, "0.1.18"},
     {modules, []},
     {registered, [emqx_authn_sup, emqx_authn_registry]},
     {applications, [kernel, stdlib, emqx_resource, emqx_connector, ehttpc, epgsql, mysql, jose]},

+ 26 - 0
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.

+ 42 - 14
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,
@@ -107,8 +109,8 @@ common_fields() ->
 
 validations() ->
     [
-        {check_ssl_opts, fun check_ssl_opts/1},
-        {check_headers, fun check_headers/1}
+        {check_ssl_opts, fun ?MODULE:check_ssl_opts/1},
+        {check_headers, fun ?MODULE:check_headers/1}
     ].
 
 url(type) -> binary();
@@ -262,21 +264,47 @@ transform_header_name(Headers) ->
     ).
 
 check_ssl_opts(Conf) ->
-    {BaseUrl, _Path, _Query} = parse_url(get_conf_val("url", Conf)),
-    case BaseUrl of
-        <<"https://", _/binary>> ->
-            case get_conf_val("ssl.enable", Conf) of
-                true -> ok;
-                false -> false
+    case is_backend_http(Conf) of
+        true ->
+            Url = get_conf_val("url", Conf),
+            {BaseUrl, _Path, _Query} = parse_url(Url),
+            case BaseUrl of
+                <<"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)),
-    Headers = get_conf_val("headers", Conf),
-    Method =:= <<"post">> orelse (not maps:is_key(<<"content-type">>, Headers)).
+    case is_backend_http(Conf) of
+        true ->
+            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
@@ -311,7 +339,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))
         ),
@@ -324,7 +352,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
 }) ->

+ 1 - 1
apps/emqx_authz/src/emqx_authz.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authz, [
     {description, "An OTP application"},
-    {vsn, "0.1.17"},
+    {vsn, "0.1.18"},
     {registered, []},
     {mod, {emqx_authz_app, []}},
     {applications, [

+ 90 - 17
apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -5,27 +5,28 @@
 -module(emqx_conf_schema_tests).
 
 -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'],
-    BaseConf =
-        ""
-        "\n"
-        "     node {\n"
-        "        name = \"emqx1@127.0.0.1\"\n"
-        "        cookie = \"emqxsecretcookie\"\n"
-        "        data_dir = \"data\"\n"
-        "     }\n"
-        "     cluster {\n"
-        "        name = emqxcl\n"
-        "        discovery_strategy = static\n"
-        "        static.seeds = ~p\n"
-        "        core_nodes = ~p\n"
-        "     }\n"
-        "   "
-        "",
     lists:foreach(
         fun(Nodes) ->
-            ConfFile = iolist_to_binary(io_lib:format(BaseConf, [Nodes, Nodes])),
+            ConfFile = to_bin(?BASE_CONF, [Nodes, Nodes]),
             {ok, Conf} = hocon:binary(ConfFile, #{format => richmap}),
             ConfList = hocon_tconf:generate(emqx_conf_schema, Conf),
             ClusterDiscovery = proplists:get_value(
@@ -46,6 +47,75 @@ array_nodes_test() ->
     ),
     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"]),
+
+    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.
     {
@@ -66,3 +136,6 @@ doc_gen_test() ->
             ok
         end
     }.
+
+to_bin(Format, Args) ->
+    iolist_to_binary(io_lib:format(Format, Args)).

+ 1 - 1
apps/emqx_connector/src/emqx_connector.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_connector, [
     {description, "EMQX Data Integration Connectors"},
-    {vsn, "0.1.20"},
+    {vsn, "0.1.21"},
     {registered, []},
     {mod, {emqx_connector_app, []}},
     {applications, [

+ 0 - 2
apps/emqx_prometheus/TODO

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

+ 2 - 0
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.