Merge pull request #13010 from JimMoen/fix-jt808-reg-failed

fix(gw): jt808 REG_ACK failed due to faulty clientinfo

apps/emqx_gateway_jt808/src/emqx_gateway_jt808.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_gateway_jt808, [
     {description, "JT/T 808 Gateway"},
-    {vsn, "0.0.2"},
+    {vsn, "0.0.3"},
     {registered, []},
     {applications, [kernel, stdlib, emqx, emqx_gateway]},
     {env, []},

apps/emqx_gateway_jt808/src/emqx_jt808_channel.erl

@@ -230,9 +230,14 @@ handle_in(Frame = ?MSG(MType), Channel = #channel{conn_state = ConnState}) when
 handle_in(Frame, Channel = #channel{conn_state = connected}) ->
     ?SLOG(debug, #{msg => "recv_frame", frame => Frame}),
     do_handle_in(Frame, Channel);
+handle_in(Frame = ?MSG(MType), Channel) when
+    MType =:= ?MC_DEREGISTER
+->
+    ?SLOG(debug, #{msg => "recv_frame", frame => Frame, info => "jt808_client_deregister"}),
+    do_handle_in(Frame, Channel#channel{conn_state = disconnected});
 handle_in(Frame, Channel) ->
     ?SLOG(error, #{msg => "unexpected_frame", frame => Frame}),
-    {stop, unexpected_frame, Channel}.
+    {shutdown, unexpected_frame, Channel}.
 
 %% @private
 do_handle_in(Frame = ?MSG(?MC_GENERAL_RESPONSE), Channel = #channel{inflight = Inflight}) ->
@@ -241,19 +246,24 @@ do_handle_in(Frame = ?MSG(?MC_GENERAL_RESPONSE), Channel = #channel{inflight = I
     {ok, Channel#channel{inflight = NewInflight}};
 do_handle_in(Frame = ?MSG(?MC_REGISTER), Channel0) ->
     #{<<"header">> := #{<<"msg_sn">> := MsgSn}} = Frame,
-    case emqx_jt808_auth:register(Frame, Channel0#channel.auth) of
+    case
-        {ok, Authcode} ->
+        emqx_utils:pipeline(
-            {ok, Conninfo} = enrich_conninfo(Frame, Channel0#channel{authcode = Authcode}),
+            [
-            {ok, Channel} = enrich_clientinfo(Frame, Conninfo),
+                fun enrich_clientinfo/2,
-            handle_out({?MS_REGISTER_ACK, 0}, MsgSn, Channel);
+                fun enrich_conninfo/2,
-        {error, Reason} ->
+                fun set_log_meta/2
-            ?SLOG(error, #{msg => "register_failed", reason => Reason}),
+            ],
-            ResCode =
+            Frame,
-                case is_integer(Reason) of
+            Channel0
-                    true -> Reason;
+        )
-                    false -> 1
+    of
-                end,
+        {ok, _NFrame, Channel} ->
-            handle_out({?MS_REGISTER_ACK, ResCode}, MsgSn, Channel0)
+            case register_(Frame, Channel) of
+                {ok, NChannel} ->
+                    handle_out({?MS_REGISTER_ACK, 0}, MsgSn, NChannel);
+                {error, ResCode} ->
+                    handle_out({?MS_REGISTER_ACK, ResCode}, MsgSn, Channel)
+            end
     end;
 do_handle_in(Frame = ?MSG(?MC_AUTH), Channel0) ->
     #{<<"header">> := #{<<"msg_sn">> := MsgSn}} = Frame,
@@ -311,7 +321,7 @@ do_handle_in(
             {ok, Channel#channel{inflight = ack_msg(?MC_DRIVER_ID_REPORT, none, Inflight)}}
     end;
 do_handle_in(?MSG(?MC_DEREGISTER), Channel) ->
-    {stop, normal, Channel};
+    {shutdown, normal, Channel};
 do_handle_in(Frame = #{}, Channel = #channel{up_topic = Topic, inflight = Inflight}) ->
     {MsgId, MsgSn} = msgidsn(Frame),
     _ = do_publish(Topic, Frame),
@@ -859,6 +869,20 @@ is_driver_id_req_exist(#channel{inflight = Inflight}) ->
     Key = get_msg_ack(?MC_DRIVER_ID_REPORT, none),
     emqx_inflight:contain(Key, Inflight).
 
+register_(Frame, Channel0) ->
+    case emqx_jt808_auth:register(Frame, Channel0#channel.auth) of
+        {ok, Authcode} ->
+            {ok, Channel0#channel{authcode = Authcode}};
+        {error, Reason} ->
+            ?SLOG(error, #{msg => "register_failed", reason => Reason}),
+            ResCode =
+                case is_integer(Reason) of
+                    true -> Reason;
+                    false -> 1
+                end,
+            {error, ResCode}
+    end.
+
 authenticate(_AuthFrame, #channel{authcode = anonymous}) ->
     true;
 authenticate(AuthFrame, #channel{authcode = undefined, auth = Auth}) ->

apps/emqx_gateway_jt808/test/emqx_jt808_SUITE.erl

@@ -68,6 +68,22 @@ gateway.jt808 {
 }
 ">>).
 
+%% erlfmt-ignore
+-define(CONF_INVALID_AUTH_SERVER, <<"
+gateway.jt808 {
+  listeners.tcp.default {
+    bind = ", ?PORT_STR, "
+  }
+  proto {
+    auth {
+      allow_anonymous = false
+      registry = \"abc://abc\"
+      authentication = \"abc://abc\"
+    }
+  }
+}
+">>).
+
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
@@ -77,6 +93,9 @@ init_per_suite(Config) ->
 end_per_suite(_Config) ->
     ok.
 
+init_per_testcase(Case = t_case_invalid_auth_reg_server, Config) ->
+    Apps = boot_apps(Case, ?CONF_INVALID_AUTH_SERVER, Config),
+    [{suite_apps, Apps} | Config];
 init_per_testcase(Case = t_case02_anonymous_register_and_auth, Config) ->
     Apps = boot_apps(Case, ?CONF_ANONYMOUS, Config),
     [{suite_apps, Apps} | Config];
@@ -146,7 +165,7 @@ do_escape(<<C, Rest/binary>>, Acc) ->
 client_regi_procedure(Socket) ->
     client_regi_procedure(Socket, <<"123456">>).
 
-client_regi_procedure(Socket, ExpectedCode) ->
+client_regi_procedure(Socket, ExpectedAuthCode) ->
     %
     % send REGISTER
     %
@@ -170,7 +189,7 @@ client_regi_procedure(Socket, ExpectedCode) ->
     ok = gen_tcp:send(Socket, S1),
     {ok, Packet} = gen_tcp:recv(Socket, 0, 500),
 
-    AckPacket = <<MsgSn:?WORD, 0, ExpectedCode/binary>>,
+    AckPacket = <<MsgSn:?WORD, 0, ExpectedAuthCode/binary>>,
     Size2 = size(AckPacket),
     MsgId2 = ?MS_REGISTER_ACK,
     MsgSn2 = 0,
@@ -181,7 +200,7 @@ client_regi_procedure(Socket, ExpectedCode) ->
     ?LOGT("S2=~p", [binary_to_hex_string(S2)]),
     ?LOGT("Packet=~p", [binary_to_hex_string(Packet)]),
     ?assertEqual(S2, Packet),
-    {ok, ExpectedCode}.
+    {ok, ExpectedAuthCode}.
 
 client_auth_procedure(Socket, AuthCode) ->
     ?LOGT("start auth procedure", []),
@@ -2683,6 +2702,52 @@ t_case34_dl_0x8805_single_mm_data_ctrl(_Config) ->
 
     ok = gen_tcp:close(Socket).
 
+t_case_invalid_auth_reg_server(_Config) ->
+    {ok, Socket} = gen_tcp:connect({127, 0, 0, 1}, ?PORT, [binary, {active, false}]),
+    %
+    % send REGISTER
+    %
+    Manuf = <<"examp">>,
+    Model = <<"33333333333333333333">>,
+    DevId = <<"1234567">>,
+
+    Color = 3,
+    Plate = <<"ujvl239">>,
+    RegisterPacket =
+        <<58:?WORD, 59:?WORD, Manuf/binary, Model/binary, DevId/binary, Color, Plate/binary>>,
+    MsgId = ?MC_REGISTER,
+    PhoneBCD = <<16#00, 16#01, 16#23, 16#45, 16#67, 16#89>>,
+    MsgSn = 78,
+    Size = size(RegisterPacket),
+    Header =
+        <<MsgId:?WORD, ?RESERVE:2, ?NO_FRAGMENT:1, ?NO_ENCRYPT:3, ?MSG_SIZE(Size), PhoneBCD/binary,
+            MsgSn:?WORD>>,
+    S1 = gen_packet(Header, RegisterPacket),
+
+    %% Send REGISTER Packet
+    ok = gen_tcp:send(Socket, S1),
+    %% Receive REGISTER_ACK Packet
+    {ok, RecvPacket} = gen_tcp:recv(Socket, 0, 50_000),
+
+    %% No AuthCode when register failed
+    AuthCode = <<>>,
+
+    AckPacket = <<MsgSn:?WORD, 1, AuthCode/binary>>,
+    Size2 = size(AckPacket),
+    MsgId2 = ?MS_REGISTER_ACK,
+    MsgSn2 = 0,
+    Header2 =
+        <<MsgId2:?WORD, ?RESERVE:2, ?NO_FRAGMENT:1, ?NO_ENCRYPT:3, ?MSG_SIZE(Size2),
+            PhoneBCD/binary, MsgSn2:?WORD>>,
+    S2 = gen_packet(Header2, AckPacket),
+
+    ?LOGT("S1=~p", [binary_to_hex_string(S1)]),
+    ?LOGT("S2=~p", [binary_to_hex_string(S2)]),
+    ?LOGT("Received REGISTER_ACK Packet=~p", [binary_to_hex_string(RecvPacket)]),
+
+    ?assertEqual(S2, RecvPacket),
+    ok.
+
 t_create_ALLOW_invalid_auth_config(_Config) ->
     test_invalid_config(create, true).
 

changes/ee/fix-13010.en.md

@@ -0,0 +1 @@
+Fixed the issue where the JT/T 808 gateway could not correctly reply to the REGISTER_ACK message when requesting authentication from the registration service failed.