From 15c84ba15271f2f7a6074bd59e150f3159ff9153 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 1 Sep 2022 15:43:32 +0800 Subject: [PATCH 1/5] fix(jwt): support non-integer timestamp claims --- apps/emqx_auth_jwt/src/emqx_auth_jwt.erl | 42 +++++++++++++++---- apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl | 8 ++-- .../test/emqx_auth_jwt_SUITE.erl | 13 +++++- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl index 040f9b629..1f304a65e 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl @@ -26,6 +26,8 @@ , description/0 ]). +-export([binary_to_number/1]). + %%-------------------------------------------------------------------- %% Authentication callbacks %%-------------------------------------------------------------------- @@ -56,16 +58,12 @@ check_acl(ClientInfo = #{jwt_claims := Claims}, #{acl_claim_name := AclClaimName}) -> case Claims of #{AclClaimName := Acl, <<"exp">> := Exp} -> - try is_expired(Exp) of + case is_expired(Exp) of true -> ?DEBUG("acl_deny_due_to_jwt_expired", []), deny; false -> verify_acl(ClientInfo, Acl, PubSub, Topic) - catch - _:_ -> - ?DEBUG("acl_deny_due_to_invalid_jwt_exp", []), - deny end; #{AclClaimName := Acl} -> verify_acl(ClientInfo, Acl, PubSub, Topic); @@ -75,14 +73,40 @@ check_acl(ClientInfo = #{jwt_claims := Claims}, end. is_expired(Exp) when is_binary(Exp) -> - ExpInt = binary_to_integer(Exp), - is_expired(ExpInt); -is_expired(Exp) -> + case binary_to_number(Exp) of + {ok, Val} -> + is_expired(Val); + _ -> + ?DEBUG("acl_deny_due_to_invalid_jwt_exp:~p", [Exp]), + true + end; +is_expired(Exp) when is_integer(Exp) -> Now = erlang:system_time(second), - Now > Exp. + Now > Exp; +is_expired(Exp) -> + ?DEBUG("acl_deny_due_to_invalid_jwt_exp:~p", [Exp]), + true. description() -> "Authentication with JWT". +binary_to_number(Bin) -> + Checker = fun([], _) -> + false; + ([H | T], Self) -> + try + {ok, H(Bin)} + catch _:_ -> + Self(T, Self) + end + end, + + Checker([fun erlang:binary_to_integer/1, + fun(In) -> + Val = erlang:binary_to_float(In), + erlang:round(Val) + end], + Checker). + %%------------------------------------------------------------------------------ %% Verify Claims %%-------------------------------------------------------------------- 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 ac07a8640..a9b35dbec 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl @@ -215,13 +215,13 @@ with_int_value(Fun) -> case Value of Int when is_integer(Int) -> Fun(Int); Bin when is_binary(Bin) -> - case string:to_integer(Bin) of - {Int, <<>>} -> Fun(Int); + case emqx_auth_jwt:binary_to_number(Bin) of + {ok, Int} -> Fun(Int); _ -> false end; Str when is_list(Str) -> - case string:to_integer(Str) of - {Int, ""} -> Fun(Int); + case emqx_auth_jwt:binary_to_number(Str) of + {ok, Int} -> Fun(Int); _ -> false end end 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 c8eed8b41..934d80f41 100644 --- a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl +++ b/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl @@ -164,7 +164,18 @@ t_check_auth_str_exp(_Config) -> Result1 = emqx_access_control:authenticate(Plain#{password => Jwt1}), ct:pal("Auth result: ~p~n", [Result1]), - ?assertMatch({error, _}, Result1). + ?assertMatch({error, _}, Result1), + + Exp2 = float_to_binary(os:system_time(seconds) + 3.5), + + Jwt2 = sign([{clientid, <<"client1">>}, + {username, <<"plain">>}, + {exp, Exp2}], <<"HS256">>, <<"emqxsecret">>), + ct:pal("Jwt: ~p~n", [Jwt2]), + + Result2 = emqx_access_control:authenticate(Plain#{password => Jwt2}), + ct:pal("Auth result: ~p~n", [Result2]), + ?assertMatch({ok, #{auth_result := success, jwt_claims := _}}, Result2). t_check_claims(init, _Config) -> application:set_env(emqx_auth_jwt, verify_claims, [{sub, <<"value">>}]). From c999b43144d4ad381b4c05e933665689c4fbb4dd Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 1 Sep 2022 15:48:14 +0800 Subject: [PATCH 2/5] chore: bump emqx_auth_jwt version && update appup --- apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src | 2 +- apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src index 72e6e749b..44882dc1b 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src @@ -1,6 +1,6 @@ {application, emqx_auth_jwt, [{description, "EMQ X Authentication with JWT"}, - {vsn, "4.3.5"}, % strict semver, bump manually! + {vsn, "4.3.6"}, % strict semver, bump manually! {modules, []}, {registered, [emqx_auth_jwt_sup]}, {applications, [kernel,stdlib,jose]}, diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src index 2364f901e..d3dbad3ad 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src @@ -1,11 +1,17 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + [{"4.3.5",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]} + ]}, + {"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]}, {<<".*">>,[]}], - [{"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + [{"4.3.5",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]} + ]}, + {"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]}, {<<".*">>,[]}]}. From ddc25fc5c297c066352b10a5646c5fd04199c8d5 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 1 Sep 2022 16:27:15 +0800 Subject: [PATCH 3/5] fix(jwt): simplify binary_to_number function --- apps/emqx_auth_jwt/src/emqx_auth_jwt.erl | 26 +++++++++--------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl index 1f304a65e..1dee41677 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl @@ -90,22 +90,16 @@ is_expired(Exp) -> description() -> "Authentication with JWT". binary_to_number(Bin) -> - Checker = fun([], _) -> - false; - ([H | T], Self) -> - try - {ok, H(Bin)} - catch _:_ -> - Self(T, Self) - end - end, - - Checker([fun erlang:binary_to_integer/1, - fun(In) -> - Val = erlang:binary_to_float(In), - erlang:round(Val) - end], - Checker). + try + {ok, erlang:binary_to_integer(Bin)} + catch _:_ -> + try + Val = erlang:binary_to_float(Bin), + {ok, erlang:round(Val)} + catch _:_ -> + false + end + end. %%------------------------------------------------------------------------------ %% Verify Claims From a6cf74ea6f098b4862db02a8a3b106c7decc0161 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 1 Sep 2022 16:52:41 +0800 Subject: [PATCH 4/5] chore: fix emqx_auth_jwt appup --- .../emqx_auth_jwt/src/emqx_auth_jwt.appup.src | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src index d3dbad3ad..5868efc9e 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src @@ -1,17 +1,25 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"4.3.5",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, - {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]} - ]}, - {"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, - {"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + [{"4.3.5", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + {"4.3.4", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + {"4.3.3", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]}, {<<".*">>,[]}], - [{"4.3.5",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, - {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]} - ]}, - {"4.3.4",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, - {"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + [{"4.3.5", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + {"4.3.4", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, + {"4.3.3", + [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}, + {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]}, {<<".*">>,[]}]}. From 884ec15567c1fbbda104053aeb6ab1171c160d59 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 1 Sep 2022 17:05:39 +0800 Subject: [PATCH 5/5] fix(jwt): make binary_to_number function support list type --- apps/emqx_auth_jwt/src/emqx_auth_jwt.erl | 32 ++++++++++++-------- apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl | 8 ++--- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl index 1dee41677..acf367c56 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl @@ -26,7 +26,7 @@ , description/0 ]). --export([binary_to_number/1]). +-export([string_to_number/1]). %%-------------------------------------------------------------------- %% Authentication callbacks @@ -73,7 +73,7 @@ check_acl(ClientInfo = #{jwt_claims := Claims}, end. is_expired(Exp) when is_binary(Exp) -> - case binary_to_number(Exp) of + case string_to_number(Exp) of {ok, Val} -> is_expired(Val); _ -> @@ -89,17 +89,12 @@ is_expired(Exp) -> description() -> "Authentication with JWT". -binary_to_number(Bin) -> - try - {ok, erlang:binary_to_integer(Bin)} - catch _:_ -> - try - Val = erlang:binary_to_float(Bin), - {ok, erlang:round(Val)} - catch _:_ -> - false - end - end. +string_to_number(Bin) when is_binary(Bin) -> + string_to_number(Bin, fun erlang:binary_to_integer/1, fun erlang:binary_to_float/1); +string_to_number(Str) when is_list(Str) -> + string_to_number(Str, fun erlang:list_to_integer/1, fun erlang:list_to_float/1); +string_to_number(_) -> + false. %%------------------------------------------------------------------------------ %% Verify Claims @@ -145,3 +140,14 @@ match_topic(ClientInfo, AclTopic, Topic) -> TopicWords = emqx_topic:words(Topic), AclTopicRendered = emqx_access_rule:feed_var(ClientInfo, AclTopicWords), emqx_topic:match(TopicWords, AclTopicRendered). + +string_to_number(Str, IntFun, FloatFun) -> + try + {ok, IntFun(Str)} + catch _:_ -> + try + {ok, FloatFun(Str)} + catch _:_ -> + false + end + end. 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 a9b35dbec..0f09be22e 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl @@ -215,13 +215,13 @@ with_int_value(Fun) -> case Value of Int when is_integer(Int) -> Fun(Int); Bin when is_binary(Bin) -> - case emqx_auth_jwt:binary_to_number(Bin) of - {ok, Int} -> Fun(Int); + case emqx_auth_jwt:string_to_number(Bin) of + {ok, Num} -> Fun(Num); _ -> false end; Str when is_list(Str) -> - case emqx_auth_jwt:binary_to_number(Str) of - {ok, Int} -> Fun(Int); + case emqx_auth_jwt:string_to_number(Str) of + {ok, Num} -> Fun(Num); _ -> false end end