Merge pull request #8452 from zmstone/0708-fix-authz-no-rule-apply-for-superuser

fix(authz): should apply no rule on superuser
2022-07-08 21:37:13 +01:00 · 2022-07-08 21:37:13 +01:00 · 8b7e9a2359
parent e76b49c036 8bf5c20bdf
commit 8b7e9a2359
5 changed files with 83 additions and 32 deletions
--- a/CHANGES-5.0.md
+++ b/CHANGES-5.0.md
@ -7,21 +7,22 @@
  Prior to this change, the webhook only checks the connectivity of the TCP port using `gen_tcp:connect/2`, so
  if it's a HTTPs server, we didn't check if TLS handshake was successful.
  [commits/6b45d2ea](https://github.com/emqx/emqx/commit/6b45d2ea9fde6d3b4a5b007f7a8c5a1c573d141e)
-* The `create_at` field of rules is missing after emqx restarts. [commits/5fc09e6b](https://github.com/emqx/emqx/commit/5fc09e6b950c340243d7be627a0ce1700691221c)
-* The rule engine's jq function now works even when the path to the EMQX install dir contains spaces
+* The `created_at` field of rules is missing after emqx restarts. [commits/5fc09e6b](https://github.com/emqx/emqx/commit/5fc09e6b950c340243d7be627a0ce1700691221c)
+* The rule engine's jq function now works even when the path to the EMQX install dir contains spaces [jq#35](https://github.com/emqx/jq/pull/35) [#8455](https://github.com/emqx/emqx/pull/8455)
+* Avoid applying any ACL checks on superusers [#8452](https://github.com/emqx/emqx/pull/8452)

 # 5.0.3

 ## Bug fixes

-* Websocket listener failed to read headers `X-Forwared-For` and `X-Forwarded-Port` [8415](https://github.com/emqx/emqx/pull/8415)
-* Deleted `cluster_singleton` from MQTT bridge config document. This config is no longer applicable in 5.0 [8407](https://github.com/emqx/emqx/pull/8407)
-* Fix `emqx/emqx:latest` docker image publish to use the Erlang flavor, but not Elixir flavor [8414](https://github.com/emqx/emqx/pull/8414)
-* Changed the `exp` field in JWT auth to be optional rather than required to fix backwards compatability with 4.X releases. [8425](https://github.com/emqx/emqx/pull/8425)
+* Websocket listener failed to read headers `X-Forwarded-For` and `X-Forwarded-Port` [#8415](https://github.com/emqx/emqx/pull/8415)
+* Deleted `cluster_singleton` from MQTT bridge config document. This config is no longer applicable in 5.0 [#8407](https://github.com/emqx/emqx/pull/8407)
+* Fix `emqx/emqx:latest` docker image publish to use the Erlang flavor, but not Elixir flavor [#8414](https://github.com/emqx/emqx/pull/8414)
+* Changed the `exp` field in JWT auth to be optional rather than required to fix backwards compatability with 4.X releases. [#8425](https://github.com/emqx/emqx/pull/8425)

 ## Enhancements

-* Improve the speed of dashboard's HTTP API routing rule generation, which sometimes causes timeout [8438](https://github.com/emqx/emqx/pull/8438)
+* Improve the speed of dashboard's HTTP API routing rule generation, which sometimes causes timeout [#8438](https://github.com/emqx/emqx/pull/8438)

 # 5.0.2

@ -37,8 +38,8 @@ This had been the biggest obstacle for EMQX team to act agile enough in delivery

 ## Bug fixes

-* Fixed a typo in `bin/emqx` which affects MacOs release when trying to enable Erlang distribution over TLS [8398](https://github.com/emqx/emqx/pull/8398)
-* Restricted shell was accidentally disabled in 5.0.1, it has been added back. [8396](https://github.com/emqx/emqx/pull/8396)
+* Fixed a typo in `bin/emqx` which affects MacOs release when trying to enable Erlang distribution over TLS [#8398](https://github.com/emqx/emqx/pull/8398)
+* Restricted shell was accidentally disabled in 5.0.1, it has been added back. [#8396](https://github.com/emqx/emqx/pull/8396)

 # 5.0.1

@ -67,25 +68,25 @@ Exceptions:

 ## Enhancements

-* Removed management API auth for prometheus scraping endpoint /api/v5/prometheus/stats [8299](https://github.com/emqx/emqx/pull/8299)
-* Added more TCP options for exhook (gRPC) connections. [8317](https://github.com/emqx/emqx/pull/8317)
-* HTTP Servers used for authentication and authorization will now indicate the result via the response body. [8374](https://github.com/emqx/emqx/pull/8374) [8377](https://github.com/emqx/emqx/pull/8377)
-* Bulk subscribe/unsubscribe APIs [8356](https://github.com/emqx/emqx/pull/8356)
-* Added exclusive subscription [8315](https://github.com/emqx/emqx/pull/8315)
-* Provide authentication counter metrics [8352](https://github.com/emqx/emqx/pull/8352) [8375](https://github.com/emqx/emqx/pull/8375)
-* Do not allow admin user self-deletion [8286](https://github.com/emqx/emqx/pull/8286)
-* After restart, ensure to copy `cluster-override.conf` from the clustered node which has the greatest `tnxid`. [8333](https://github.com/emqx/emqx/pull/8333)
+* Removed management API auth for prometheus scraping endpoint /api/v5/prometheus/stats [#8299](https://github.com/emqx/emqx/pull/8299)
+* Added more TCP options for exhook (gRPC) connections. [#8317](https://github.com/emqx/emqx/pull/8317)
+* HTTP Servers used for authentication and authorization will now indicate the result via the response body. [#8374](https://github.com/emqx/emqx/pull/8374) [#8377](https://github.com/emqx/emqx/pull/8377)
+* Bulk subscribe/unsubscribe APIs [#8356](https://github.com/emqx/emqx/pull/8356)
+* Added exclusive subscription [#8315](https://github.com/emqx/emqx/pull/8315)
+* Provide authentication counter metrics [#8352](https://github.com/emqx/emqx/pull/8352) [#8375](https://github.com/emqx/emqx/pull/8375)
+* Do not allow admin user self-deletion [#8286](https://github.com/emqx/emqx/pull/8286)
+* After restart, ensure to copy `cluster-override.conf` from the clustered node which has the greatest `tnxid`. [#8333](https://github.com/emqx/emqx/pull/8333)

 ## Bug fixes

-* A bug fix ported from 4.x: allow deleting subscriptions from `client.subscribe` hookpoint callback result. [8304](https://github.com/emqx/emqx/pull/8304) [8347](https://github.com/emqx/emqx/pull/8377)
-* Fixed Erlang distribution over TLS [8309](https://github.com/emqx/emqx/pull/8309)
-* Made possible to override authentication configs from environment variables [8323](https://github.com/emqx/emqx/pull/8309)
-* Made authentication passwords in Mnesia database backward compatible to 4.x, so we can support data migration better. [8351](https://github.com/emqx/emqx/pull/8351)
-* Fix plugins upload for rpm/deb installations [8379](https://github.com/emqx/emqx/pull/8379)
-* Sync data/authz/acl.conf and data/certs from clustered nodes after a new node joins the cluster [8369](https://github.com/emqx/emqx/pull/8369)
-* Ensure auto-retry of failed resources [8371](https://github.com/emqx/emqx/pull/8371)
-* Fix the issue that the count of `packets.connack.auth_error` is inaccurate when the client uses a protocol version below MQTT v5.0 to access [8178](https://github.com/emqx/emqx/pull/8178)
+* A bug fix ported from 4.x: allow deleting subscriptions from `client.subscribe` hookpoint callback result. [#8304](https://github.com/emqx/emqx/pull/8304) [#8347](https://github.com/emqx/emqx/pull/8377)
+* Fixed Erlang distribution over TLS [#8309](https://github.com/emqx/emqx/pull/8309)
+* Made possible to override authentication configs from environment variables [#8323](https://github.com/emqx/emqx/pull/8309)
+* Made authentication passwords in Mnesia database backward compatible to 4.x, so we can support data migration better. [#8351](https://github.com/emqx/emqx/pull/8351)
+* Fix plugins upload for rpm/deb installations [#8379](https://github.com/emqx/emqx/pull/8379)
+* Sync data/authz/acl.conf and data/certs from clustered nodes after a new node joins the cluster [#8369](https://github.com/emqx/emqx/pull/8369)
+* Ensure auto-retry of failed resources [#8371](https://github.com/emqx/emqx/pull/8371)
+* Fix the issue that the count of `packets.connack.auth_error` is inaccurate when the client uses a protocol version below MQTT v5.0 to access [#8178](https://github.com/emqx/emqx/pull/8178)

 ## Others

--- a/apps/emqx/include/emqx_release.hrl
+++ b/apps/emqx/include/emqx_release.hrl
@ -32,7 +32,7 @@
 %% `apps/emqx/src/bpapi/README.md'

 %% Community edition
-define(EMQX_RELEASE_CE, "5.0.3").
+-define(EMQX_RELEASE_CE, "5.0.4").

 %% Enterprise edition
 -define(EMQX_RELEASE_EE, "5.0.0-alpha.1").
--- a/apps/emqx_authz/src/emqx_authz.app.src
+++ b/apps/emqx_authz/src/emqx_authz.app.src
@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authz, [
    {description, "An OTP application"},
-    {vsn, "0.1.2"},
+    {vsn, "0.1.3"},
    {registered, []},
    {mod, {emqx_authz_app, []}},
    {applications, [
--- a/apps/emqx_authz/src/emqx_authz.erl
+++ b/apps/emqx_authz/src/emqx_authz.erl
@ -53,11 +53,12 @@

 -type sources() :: [source()].

+-define(METRIC_SUPERUSER, 'authorization.superuser').
 -define(METRIC_ALLOW, 'authorization.matched.allow').
 -define(METRIC_DENY, 'authorization.matched.deny').
 -define(METRIC_NOMATCH, 'authorization.nomatch').

-define(METRICS, [?METRIC_ALLOW, ?METRIC_DENY, ?METRIC_NOMATCH]).
+-define(METRICS, [?METRIC_SUPERUSER, ?METRIC_ALLOW, ?METRIC_DENY, ?METRIC_NOMATCH]).

 -define(IS_ENABLED(Enable), ((Enable =:= true) or (Enable =:= <<"true">>))).

@ -308,6 +309,30 @@ authorize(
    Topic,
    DefaultResult,
    Sources
+) ->
+    case maps:get(is_superuser, Client, false) of
+        true ->
+            log_allowed(#{
+                username => Username,
+                ipaddr => IpAddress,
+                topic => Topic,
+                is_superuser => true
+            }),
+            emqx_metrics:inc(?METRIC_SUPERUSER),
+            {stop, allow};
+        false ->
+            authorize_non_superuser(Client, PubSub, Topic, DefaultResult, Sources)
+    end.
+
+authorize_non_superuser(
+    #{
+        username := Username,
+        peerhost := IpAddress
+    } = Client,
+    PubSub,
+    Topic,
+    DefaultResult,
+    Sources
 ) ->
    case do_authorize(Client, PubSub, Topic, sources_with_defaults(Sources)) of
        {{matched, allow}, AuthzSource} ->
@ -315,8 +340,7 @@ authorize(
                'client.check_authz_complete',
                [Client, PubSub, Topic, allow, AuthzSource]
            ),
-            ?SLOG(info, #{
-                msg => "authorization_permission_allowed",
+            log_allowed(#{
                username => Username,
                ipaddr => IpAddress,
                topic => Topic,
@ -356,6 +380,9 @@ authorize(
            {stop, DefaultResult}
    end.

+log_allowed(Meta) ->
+    ?SLOG(info, Meta#{msg => "authorization_permission_allowed"}).
+
 do_authorize(_Client, _PubSub, _Topic, []) ->
    nomatch;
 do_authorize(Client, PubSub, Topic, [#{enable := false} | Rest]) ->
--- a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl
+++ b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl
@ -84,8 +84,6 @@ t_ok(_Config) ->
        <<"rules">> => <<"{allow, {user, \"username\"}, publish, [\"t\"]}.">>
    }),

-    io:format("~p", [emqx_authz:acl_conf_file()]),
-
    ?assertEqual(
        allow,
        emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
@ -96,6 +94,31 @@ t_ok(_Config) ->
        emqx_access_control:authorize(ClientInfo, subscribe, <<"t">>)
    ).

+t_superuser(_Config) ->
+    ClientInfo = #{
+        clientid => <<"clientid">>,
+        username => <<"username">>,
+        is_superuser => true,
+        peerhost => {127, 0, 0, 1},
+        zone => default,
+        listener => {tcp, default}
+    },
+
+    %% no rules apply to superuser
+    ok = setup_config(?RAW_SOURCE#{
+        <<"rules">> => <<"{deny, {user, \"username\"}, publish, [\"t\"]}.">>
+    }),
+
+    ?assertEqual(
+        allow,
+        emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
+    ),
+
+    ?assertEqual(
+        allow,
+        emqx_access_control:authorize(ClientInfo, subscribe, <<"t">>)
+    ).
+
 t_invalid_file(_Config) ->
    ?assertMatch(
        {error, bad_acl_file_content},