diff --git a/src/device/router_device.erl b/src/device/router_device.erl index ede2f5c67..64359f468 100644 --- a/src/device/router_device.erl +++ b/src/device/router_device.erl @@ -23,6 +23,7 @@ dev_eui/1, dev_eui/2, keys/1, keys/2, nwk_s_key/1, + nwk_s_keys/1, app_s_key/1, devaddr/1, devaddrs/1, devaddrs/2, @@ -125,6 +126,10 @@ nwk_s_key(#device_v7{keys = []}) -> nwk_s_key(#device_v7{keys = [{NwkSKey, _AppSKey} | _]}) -> NwkSKey. +-spec nwk_s_keys(device()) -> list(binary()). +nwk_s_keys(#device_v7{keys = Keys}) -> + [NwkSKey || {NwkSKey, _AppSKey} <- Keys]. + -spec app_s_key(device()) -> binary() | undefined. app_s_key(#device_v7{keys = []}) -> undefined; diff --git a/src/device/router_device_routing.erl b/src/device/router_device_routing.erl index 106a922b0..4c4ccaa91 100644 --- a/src/device/router_device_routing.erl +++ b/src/device/router_device_routing.erl @@ -42,7 +42,8 @@ -export([ payload_b0/2, payload_mic/1, - payload_fcnt_low/1 + payload_fcnt_low/1, + get_device_for_payload/2 ]). %% biggest unsigned number in 23 bits @@ -302,7 +303,7 @@ get_device_for_payload(Payload, PubKeyBin) -> end; <<_MType:3, _MHDRRFU:3, _Major:2, DevAddr:4/binary, _/binary>> -> MIC = payload_mic(Payload), - case find_device(PubKeyBin, DevAddr, MIC, Payload) of + case find_device_for_uplink(PubKeyBin, DevAddr, MIC, Payload) of {ok, {Device, _NwkSKey, FCnt}} -> {ok, Device, FCnt}; E2 -> E2 end @@ -332,10 +333,7 @@ validate_payload_for_device(Device, Payload, PHash, PubKeyBin, Fcnt) -> {ok, _} -> case check_device_preferred_hotspots(Device, PubKeyBin) of none_preferred -> - case maybe_multi_buy_offer(Device, PHash) of - {ok, _} -> ok; - E -> E - end; + ok; preferred -> ok; not_preferred_hotspot -> @@ -769,6 +767,13 @@ check_device_is_active(Device, PubKeyBin) -> Device, PubKeyBin ), + spawn(fun() -> + DeviceID = router_device:id(Device), + lager:warning("routing for inactive device ~p, removing", [DeviceID]), + ok = router_ics_skf_worker:remove_device_ids([DeviceID]), + ok = router_ics_eui_worker:remove([DeviceID]) + end), + {error, ?DEVICE_INACTIVE}; true -> ok @@ -962,22 +967,22 @@ get_device(DevEUI, AppEUI, Msg, MIC) -> [] -> {error, api_not_found}; KeysAndDevices -> - find_device(Msg, MIC, KeysAndDevices) + find_device_for_join(Msg, MIC, KeysAndDevices) end. --spec find_device( +-spec find_device_for_join( Msg :: binary(), MIC :: binary(), [{binary(), router_device:device()}] ) -> {ok, Device :: router_device:device(), AppKey :: binary()} | {error, not_found}. -find_device(_Msg, _MIC, []) -> +find_device_for_join(_Msg, _MIC, []) -> {error, not_found}; -find_device(Msg, MIC, [{AppKey, Device} | T]) -> +find_device_for_join(Msg, MIC, [{AppKey, Device} | T]) -> case crypto:macN(cmac, aes_128_cbc, AppKey, Msg, 4) of MIC -> {ok, Device, AppKey}; _ -> - find_device(Msg, MIC, T) + find_device_for_join(Msg, MIC, T) end. -spec send_to_device_worker( @@ -1007,7 +1012,7 @@ send_to_device_worker( DeviceInfo = case Device0 of undefined -> - case find_device(PubKeyBin, DevAddr, MIC, Payload) of + case find_device_for_uplink(PubKeyBin, DevAddr, MIC, Payload) of {error, unknown_device} -> lager:warning( "unable to find device for packet [devaddr: ~p / ~p] [gateway: ~p]", @@ -1102,13 +1107,13 @@ send_to_device_worker_(FCnt, Packet, PacketTime, HoldTime, Pid, PubKeyBin, Regio end end. --spec find_device( +-spec find_device_for_uplink( PubKeyBin :: libp2p_crypto:pubkey_bin(), DevAddr :: binary(), MIC :: binary(), Payload :: binary() ) -> {ok, {router_device:device(), binary(), non_neg_integer()}} | {error, unknown_device}. -find_device(PubKeyBin, DevAddr, MIC, Payload) -> +find_device_for_uplink(PubKeyBin, DevAddr, MIC, Payload) -> Devices = get_and_sort_devices(DevAddr, PubKeyBin), case get_device_by_mic(MIC, Payload, Devices) of undefined -> @@ -1185,80 +1190,66 @@ get_and_sort_devices(DevAddr, PubKeyBin) -> get_device_by_mic(_MIC, _Payload, []) -> undefined; get_device_by_mic(ExpectedMIC, Payload, [Device | Devices]) -> - ExpectedFCnt = expected_fcnt(Device, Payload), - B0 = payload_b0(Payload, ExpectedFCnt), DeviceID = router_device:id(Device), - case router_device:nwk_s_key(Device) of - undefined -> + case router_device:nwk_s_keys(Device) of + [] -> lager:warning([{device_id, DeviceID}], "device did not have a nwk_s_key, deleting"), {ok, DB, [_DefaultCF, CF]} = router_db:get(), ok = router_device:delete(DB, CF, DeviceID), ok = router_device_cache:delete(DeviceID), get_device_by_mic(ExpectedMIC, Payload, Devices); - NwkSKey -> - case key_matches_mic(NwkSKey, B0, ExpectedMIC) of - true -> - lager:debug("device ~p NwkSKey matches FCnt ~b.", [DeviceID, ExpectedFCnt]), - {Device, NwkSKey, ExpectedFCnt}; + Keys -> + case find_right_key(ExpectedMIC, Payload, Keys) of false -> - lager:debug( - "device ~p NwkSKey does not match FCnt ~b. Searching device's other keys.", - [DeviceID, ExpectedFCnt] - ), - Keys = router_device:keys(Device), - case find_right_key(B0, ExpectedMIC, Payload, Device, Keys) of - false -> - lager:debug("Device does not match FCnt ~b. Searching next device.", [ - ExpectedFCnt - ]), - get_device_by_mic(ExpectedMIC, Payload, Devices); - {D, K} -> - lager:debug("Device ~p matches FCnt ~b.", [ - router_device:id(D), ExpectedFCnt - ]), - {D, K, ExpectedFCnt} - end + lager:debug("Device does not match any FCnt. Searching next device."), + get_device_by_mic(ExpectedMIC, Payload, Devices); + {true, NwkSKey, VerifiedFCnt} -> + lager:debug("Device ~p matches FCnt ~b.", [DeviceID, VerifiedFCnt]), + {Device, NwkSKey, VerifiedFCnt} end end. -spec find_right_key( - B0 :: binary(), MIC :: binary(), Payload :: binary(), - Device :: router_device:device(), - Keys :: list({binary() | undefined, binary() | undefined}) -) -> false | {error, any()} | {router_device:device(), binary()}. -find_right_key(_B0, _MIC, _Payload, _Device, []) -> + Keys :: list(binary()) +) -> false | {true, VerifiedNwkSKey :: binary(), VerifiedFCnt :: non_neg_integer()}. +find_right_key(_MIC, _Payload, []) -> false; -find_right_key(B0, MIC, Payload, Device, [{undefined, _} | Keys]) -> - find_right_key(B0, MIC, Payload, Device, Keys); -find_right_key(B0, MIC, Payload, Device, [{NwkSKey, _} | Keys]) -> - case key_matches_mic(NwkSKey, B0, MIC) of - true -> - {Device, NwkSKey}; - false -> - case key_matches_any_fcnt(NwkSKey, MIC, Payload) of - false -> find_right_key(B0, MIC, Payload, Device, Keys); - true -> {Device, NwkSKey} - end +find_right_key(MIC, Payload, [NwkSKey | Keys]) -> + case key_matches_any_fcnt(NwkSKey, MIC, Payload) of + false -> find_right_key(MIC, Payload, Keys); + {true, FCnt} -> {true, NwkSKey, FCnt} end. --spec key_matches_any_fcnt(binary(), binary(), binary()) -> boolean(). +-spec key_matches_any_fcnt(binary(), binary(), binary()) -> + false | {true, VerifiedFCnt :: non_neg_integer()}. key_matches_any_fcnt(NwkSKey, ExpectedMIC, Payload) -> FCntLow = payload_fcnt_low(Payload), - lists:any( + find_first( fun(HighBits) -> FCnt = binary:decode_unsigned( <>, little ), B0 = payload_b0(Payload, FCnt), - ComputedMIC = crypto:macN(cmac, aes_128_cbc, NwkSKey, B0, 4), - ComputedMIC =:= ExpectedMIC + {key_matches_mic(NwkSKey, B0, ExpectedMIC), FCnt} end, lists:seq(2#000, 2#111) ). +-spec find_first( + Fn :: fun((T) -> {boolean(), T}), + Els :: list(T) +) -> {true, T} | false. +find_first(_Fn, []) -> + false; +find_first(Fn, [Head | Tail]) -> + case Fn(Head) of + {true, _} = Val -> Val; + _ -> find_first(Fn, Tail) + end. + -spec key_matches_mic(binary(), binary(), binary()) -> boolean(). key_matches_mic(Key, B0, ExpectedMIC) -> ComputedMIC = crypto:macN(cmac, aes_128_cbc, Key, B0, 4), @@ -1270,72 +1261,6 @@ payload_fcnt_low(Payload) -> _/binary>> = Payload, FCntLow. --spec expected_fcnt( - Device :: router_device:device(), - Payload :: binary() -) -> non_neg_integer(). -expected_fcnt(Device, Payload) -> - %% This is a heuristic for handling replayed packets and one - %% (arguably incorrect) edge case. - - %% We need a 32-bit frame counter in order to compute the MIC. - - %% The payload contains only the 16 low-order bits of the the frame counter. - - %% If the bits from the payload are slightly less than, equal to, - %% or greater than the low order bits of our device's frame - %% counter, then our device's high bits are /probably/ the same as - %% the ones held by the end device. We will join our high bits to - %% the packets's low bits and hope that is the correct FCntUp. - - %% On the other hand, if the bits from the payload are - %% significantly less than the low order bits from the device's - %% frame counter, then there's a good chance the end device's low - %% bits have rolled back to zero. In this case, we're going to - %% increment our internal counter's high bits by one and join them - %% to the packet's low bits. - - PayloadFCntLow = payload_fcnt_low(Payload), - {PrevFCntLow, PrevFCntHigh, LowDiff} = - case router_device:fcnt(Device) of - undefined -> - {undefined, undefined, undefined}; - I when is_integer(I) -> - <> = << - I:32/integer-unsigned-little - >>, - {Low, High, abs(Low - PayloadFCntLow)} - end, - - if - PrevFCntLow =:= undefined andalso LowDiff =:= undefined -> - %% This is the first frame we've seen, so we're going to - %% assume the upper bits of the frame counter are all 0 - %% and we'll take the lower bits as-is; this will be the - %% new FCntUp. - PayloadFCntLow; - PayloadFCntLow >= PrevFCntLow -> - %% The FCnt on this packet is gte the last packet, meaning - %% it hasn't rolled back to 0. We're going to prepend the - %% high bits from our internal counter and assume that is - %% the correct FCntUp. - binary:decode_unsigned(<>); - PayloadFCntLow < PrevFCntLow andalso LowDiff =< 10 -> - %% The FCnt on this packet appears to be slightly lower - %% than our internal counter. We're going to append the - %% upper bits of our internal counter to the packet's - %% lower bits and take that as the new FCntUp. - binary:decode_unsigned(<>); - PayloadFCntLow < PrevFCntLow andalso LowDiff > 10 -> - %% The FCnt on this packets is significantly less than our - %% internal counter. We're goint to assume the lower 16 - %% bits have rolled back to zero. We'll increment the - %% upper bits and append them to the lower bits and take - %% that as the new FCntUp. - NewHigh = (PrevFCntHigh + 1) rem ?MODULO_16_BITS, - binary:decode_unsigned(<>) - end. - -spec payload_b0(binary(), non_neg_integer()) -> binary(). payload_b0(Payload, ExpectedFCnt) -> <> = diff --git a/src/device/router_device_worker.erl b/src/device/router_device_worker.erl index ebb37acd3..d21589654 100644 --- a/src/device/router_device_worker.erl +++ b/src/device/router_device_worker.erl @@ -698,7 +698,7 @@ handle_cast( end end; handle_cast( - {frame, _NwkSKey, PacketFCnt, Packet, PacketTime, _HoldTime, PubKeyBin, _Region, _Pid}, + {frame, _NwkSKey, PacketFCnt, _Packet, PacketTime, _HoldTime, PubKeyBin, _Region, _Pid}, #state{ device = Device, db = DB, @@ -706,8 +706,6 @@ handle_cast( is_active = false } = State ) -> - PHash = blockchain_helium_packet_v1:packet_hash(Packet), - ok = router_device_multibuy:max(PHash, 0), ok = router_utils:event_uplink_dropped_device_inactive( PacketTime, PacketFCnt, @@ -825,7 +823,6 @@ handle_cast( PubKeyBin ), lager:debug("did not have enough dc (~p) to send data", [_Reason]), - ok = router_device_multibuy:max(PHash, 0), ok = router_metrics:packet_trip_observe_end( PHash, PubKeyBin, @@ -857,7 +854,6 @@ handle_cast( Region ) end, - ok = router_device_multibuy:max(PHash, 0), ok = router_metrics:packet_trip_observe_end( PHash, PubKeyBin, @@ -1547,17 +1543,10 @@ validate_frame( FrameCache, OfferCache ) -> - Payload = - <> = blockchain_helium_packet_v1:payload(Packet), - - VerifiedFCnt = verified_fcnt_from_payload( - router_device_routing:payload_fcnt_low(Payload), - router_device:nwk_s_key(Device0), - router_device_routing:payload_mic(Payload), - Payload - ), + <> = blockchain_helium_packet_v1:payload(Packet), + DeviceFCnt = router_device:fcnt(Device0), case MType of @@ -1591,7 +1580,7 @@ validate_frame( PacketFCnt, LastSeenFCnt ]), - {error, {late_packet, VerifiedFCnt}}; + {error, {late_packet, PacketFCnt}}; undefined when FrameAck == 1 andalso PacketFCnt == DownlinkHandledAtFCnt andalso Window < ?RX_MAX_WINDOW @@ -1600,7 +1589,7 @@ validate_frame( "we got a late confirmed up packet for ~p: DownlinkHandledAt: ~p within window ~p", [PacketFCnt, DownlinkHandledAtFCnt, Window] ), - {error, {late_packet, VerifiedFCnt}}; + {error, {late_packet, PacketFCnt}}; undefined when FrameAck == 1 andalso PacketFCnt == DownlinkHandledAtFCnt andalso Window >= ?RX_MAX_WINDOW @@ -1618,12 +1607,12 @@ validate_frame( OfferCache, true ); - undefined when VerifiedFCnt < DeviceFCnt -> + undefined when PacketFCnt < DeviceFCnt -> lager:info( "we got a replay packet [verified: ~p] [device: ~p]", - [VerifiedFCnt, DeviceFCnt] + [PacketFCnt, DeviceFCnt] ), - {error, {late_packet, VerifiedFCnt}}; + {error, {late_packet, PacketFCnt}}; undefined -> lager:debug("we got a fresh packet [fcnt: ~p]", [PacketFCnt]), validate_frame_( @@ -2438,32 +2427,6 @@ maybe_will_downlink(Device, #frame{mtype = MType, adrackreq = ADRAckReqBit}) -> ADR = ADRAllowed andalso ADRAckReqBit == 1, DeviceQueue =/= [] orelse ACK == 1 orelse ADR orelse ChannelCorrection == false. --spec verified_fcnt_from_payload(non_neg_integer(), binary(), binary(), binary()) -> - non_neg_integer(). -verified_fcnt_from_payload(FCntLow, NwkSKey, ExpectedMIC, Payload) -> - find_first( - fun(HighBits) -> - FCnt = binary:decode_unsigned( - <>, - little - ), - B0 = router_device_routing:payload_b0(Payload, FCnt), - ComputedMIC = crypto:macN(cmac, aes_128_cbc, NwkSKey, B0, 4), - {ComputedMIC =:= ExpectedMIC, FCnt} - end, - lists:seq(2#000, 2#111) - ). - --spec find_first( - FN :: fun((HighBit :: non_neg_integer()) -> {Verified :: boolean(), FCnt :: non_neg_integer()}), - HighBits :: list(non_neg_integer()) -) -> non_neg_integer(). -find_first(Fn, [FCntHigh | Rest]) -> - case Fn(FCntHigh) of - {true, Found} -> Found; - _ -> find_first(Fn, Rest) - end. - -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). diff --git a/test/router_device_routing_SUITE.erl b/test/router_device_routing_SUITE.erl index 232c2f48b..c8a6b148d 100644 --- a/test/router_device_routing_SUITE.erl +++ b/test/router_device_routing_SUITE.erl @@ -24,7 +24,6 @@ handle_join_packet_from_strange_hotspot_test/1, handle_join_packet_from_preferred_hotspot_test/1, handle_join_packet_with_no_preferred_hotspot_test/1, - handle_packet_fcnt_32_bit_rollover_test/1, handle_packet_wrong_fcnt_test/1 ]). @@ -80,7 +79,6 @@ all_tests() -> handle_join_packet_from_strange_hotspot_test, handle_join_packet_from_preferred_hotspot_test, handle_join_packet_with_no_preferred_hotspot_test, - handle_packet_fcnt_32_bit_rollover_test, handle_packet_wrong_fcnt_test ]. @@ -641,85 +639,6 @@ multi_buy_test(Config) -> ok. -handle_packet_fcnt_32_bit_rollover_test(Config) -> - WaitForDeviceWorker = fun() -> - timer:sleep(500) - end, - ok = remove_devaddr_allocate_meck(), - test_utils:add_oui(Config), - - #{pubkey_bin := PubKeyBin} = test_utils:join_device(Config), - test_utils:wait_state_channel_message(1250), - - {ok, DB, CF} = router_db:get_devices(), - WorkerID = router_devices_sup:id(?CONSOLE_DEVICE_ID), - {ok, Device0} = router_device:get_by_id(DB, CF, WorkerID), - - Device1 = router_device:update([{fcnt, 16#FFFFFFFE}], Device0), - {ok, _} = router_device_cache:save(Device1), - - NwkSKey = router_device:nwk_s_key(Device1), - AppSKey = router_device:app_s_key(Device1), - FCnt = router_device:fcnt_next_val(Device1), - - ?assertEqual(16#FFFFFFFF, FCnt), - - %% Let's try sending the highest possible FCnt. - SCPacket = test_utils:frame_packet(?UNCONFIRMED_UP, PubKeyBin, NwkSKey, AppSKey, FCnt, #{ - dont_encode => true, - routing => true, - devaddr => router_device:devaddr(Device1) - }), - - ?assertEqual( - ok, router_device_routing:handle_packet(SCPacket, erlang:system_time(millisecond), self()) - ), - - WaitForDeviceWorker(), - - %% Packet was routed. - %% fcnt should now be 16#FFFFFFFF - %% next fcnt should now be 0 - - {ok, Device2} = router_device:get_by_id(DB, CF, WorkerID), - ?assertEqual(16#FFFFFFFF, router_device:fcnt(Device2)), - ?assertEqual(0, router_device:fcnt_next_val(Device2)), - - %% If we send 16#FFFFFFFF again, that should work because it's a legitimate replay. - ?assertEqual( - ok, router_device_routing:handle_packet(SCPacket, erlang:system_time(millisecond), self()) - ), - - %% Fun fact: there exists a race condition here where the worker - %% may reject our replayed packet because the frame cache TTL is - %% set very low during testing. The worker is expected to save - %% the device record with the updated fcnt in any case. That's - %% why we sleep instead of waiting for a console message - %% indicating the worker has processed the frame: because we don't - %% know what to expect here, an `uplink` or an `uplink_dropped`. - WaitForDeviceWorker(), - - %% Now, let's try sending 0. - SCPacket2 = test_utils:frame_packet(?UNCONFIRMED_UP, PubKeyBin, NwkSKey, AppSKey, 0, #{ - dont_encode => true, - routing => true, - devaddr => router_device:devaddr(Device1) - }), - ?assertEqual( - ok, router_device_routing:handle_packet(SCPacket2, erlang:system_time(millisecond), self()) - ), - - WaitForDeviceWorker(), - - %% That worked. - %% fcnt should now be 0 - %% next fcnt should now be 1 - - {ok, Device3} = router_device:get_by_id(DB, CF, WorkerID), - ?assertEqual(0, router_device:fcnt(Device3)), - ?assertEqual(1, router_device:fcnt_next_val(Device3)), - ok. - handle_packet_wrong_fcnt_test(Config) -> ok = remove_devaddr_allocate_meck(), test_utils:add_oui(Config),