From 8198e3496fb8c8a15b8270f2d4be9f1740399b45 Mon Sep 17 00:00:00 2001
From: Ilya Averyanov <av@rubybox.ru>
Date: Fri, 6 May 2022 16:51:55 +0300
Subject: [PATCH] fix(jwt auth): fix claim validation

---
 apps/emqx_auth_jwt/src/emqx_auth_jwt.erl      |  2 +
 apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl  | 32 ++++++---
 .../test/emqx_auth_jwt_SUITE.erl              | 69 +++++++++++++++++++
 3 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl
index 2d7690a42..b900f2bed 100644
--- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl
+++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl
@@ -85,6 +85,8 @@ check_acl(ClientInfo = #{jwt_claims := Claims},
                     ?DEBUG("acl_deny_due_to_invalid_jwt_exp", []),
                     deny
             end;
+        #{AclClaimName := Acl} ->
+            verify_acl(ClientInfo, Acl, PubSub, Topic);
         _ ->
             ?DEBUG("no_acl_jwt_claim", []),
             ignore
diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl
index 05e45cec3..c018bf1bd 100644
--- a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl
+++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl
@@ -203,18 +203,32 @@ do_verify(JwsCompacted, [Jwk|More]) ->
 
 check_claims(Claims) ->
     Now = os:system_time(seconds),
-    Checker = [{<<"exp">>, fun(ExpireTime) ->
-                               Now < ExpireTime
-                           end},
-               {<<"iat">>, fun(IssueAt) ->
-                               IssueAt =< Now
-                           end},
-               {<<"nbf">>, fun(NotBefore) ->
-                               NotBefore =< Now
-                           end}
+    Checker = [{<<"exp">>, with_int_value(
+                             fun(ExpireTime) -> Now < ExpireTime end)},
+               {<<"iat">>, with_int_value(
+                             fun(IssueAt) -> IssueAt =< Now end)},
+               {<<"nbf">>, with_int_value(
+                             fun(NotBefore) -> NotBefore =< Now end)}
               ],
     do_check_claim(Checker, Claims).
 
+with_int_value(Fun) ->
+    fun(Value) ->
+            case Value of
+                Int when is_integer(Int) -> Fun(Int);
+                Bin when is_binary(Bin) ->
+                    case string:to_integer(Bin) of
+                        {Int, <<>>} -> Fun(Int);
+                        _ -> false
+                    end;
+                Str when is_list(Str) ->
+                    case string:to_integer(Str) of
+                        {Int, ""} -> Fun(Int);
+                        _ -> false
+                    end
+            end
+    end.
+
 do_check_claim([], Claims) ->
     Claims;
 do_check_claim([{K, F}|More], Claims) ->
diff --git a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl b/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl
index abfd7b3a8..2136d5384 100644
--- a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl
+++ b/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl
@@ -93,6 +93,52 @@ t_check_auth(_Config) ->
     ?assertEqual({error, invalid_signature}, Result2),
     ?assertMatch({error, _}, emqx_access_control:authenticate(Plain#{password => <<"asd">>})).
 
+t_check_auth_invalid_exp(init, _Config) ->
+    application:unset_env(emqx_auth_jwt, verify_claims).
+t_check_auth_invalid_exp(_Config) ->
+    Plain = #{clientid => <<"client1">>, username => <<"plain">>, zone => external},
+    Jwt0 = sign([{clientid, <<"client1">>},
+                 {username, <<"plain">>},
+                 {exp, [{foo, bar}]}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt0]),
+
+    Result0 = emqx_access_control:authenticate(Plain#{password => Jwt0}),
+    ct:pal("Auth result: ~p~n", [Result0]),
+    ?assertMatch({error, _}, Result0),
+
+    Jwt1 = sign([{clientid, <<"client1">>},
+                 {username, <<"plain">>},
+                 {exp, <<"foobar">>}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt1]),
+
+    Result1 = emqx_access_control:authenticate(Plain#{password => Jwt1}),
+    ct:pal("Auth result: ~p~n", [Result1]),
+    ?assertMatch({error, _}, Result1).
+
+t_check_auth_str_exp(init, _Config) ->
+    application:unset_env(emqx_auth_jwt, verify_claims).
+t_check_auth_str_exp(_Config) ->
+    Plain = #{clientid => <<"client1">>, username => <<"plain">>, zone => external},
+    Exp = integer_to_binary(os:system_time(seconds) + 3),
+
+    Jwt0 = sign([{clientid, <<"client1">>},
+                 {username, <<"plain">>},
+                 {exp, Exp}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt0]),
+
+    Result0 = emqx_access_control:authenticate(Plain#{password => Jwt0}),
+    ct:pal("Auth result: ~p~n", [Result0]),
+    ?assertMatch({ok, #{auth_result := success, jwt_claims := _}}, Result0),
+
+    Jwt1 = sign([{clientid, <<"client1">>},
+                 {username, <<"plain">>},
+                 {exp, <<"0">>}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt1]),
+
+    Result1 = emqx_access_control:authenticate(Plain#{password => Jwt1}),
+    ct:pal("Auth result: ~p~n", [Result1]),
+    ?assertMatch({error, _}, Result1).
+
 t_check_claims(init, _Config) ->
     application:set_env(emqx_auth_jwt, verify_claims, [{sub, <<"value">>}]).
 t_check_claims(_Config) ->
@@ -276,3 +322,26 @@ t_check_jwt_acl_expire(_Config) ->
        emqtt:subscribe(C, <<"a/b">>, 0)),
 
     ok = emqtt:disconnect(C).
+
+t_check_jwt_acl_no_exp(init, _Config) ->
+    application:set_env(emqx_auth_jwt, verify_claims, [{sub, <<"value">>}]).
+t_check_jwt_acl_no_exp(_Config) ->
+    Jwt = sign([{client_id, <<"client1">>},
+                {username, <<"plain">>},
+                {sub, value},
+                {acl, [{sub, [<<"a/b">>]}]}],
+               <<"HS256">>,
+               <<"emqxsecret">>),
+
+    {ok, C} = emqtt:start_link(
+                [{clean_start, true},
+                 {proto_ver, v5},
+                 {client_id, <<"client1">>},
+                 {password, Jwt}]),
+    {ok, _} = emqtt:connect(C),
+
+    ?assertMatch(
+       {ok, #{}, [0]},
+       emqtt:subscribe(C, <<"a/b">>, 0)),
+
+    ok = emqtt:disconnect(C).