From 241a1cf436585a7b3e1a342aca8806c74f354d91 Mon Sep 17 00:00:00 2001 From: Frank Zhong Date: Thu, 8 Feb 2024 10:57:05 +0000 Subject: [PATCH 1/4] Notify observers on sending s-mode messages --- api/oc_knx_client.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/oc_knx_client.c b/api/oc_knx_client.c index f2b56942d..e91f01a32 100644 --- a/api/oc_knx_client.c +++ b/api/oc_knx_client.c @@ -736,6 +736,8 @@ oc_do_s_mode_with_scope_and_check(int scope, const char *resource_url, char *rp, return; } + oc_notify_observers(my_resource); + value_size = oc_s_mode_get_resource_value(resource_url, rp, buffer, 50); // get the sender ia From cdc3d16712bb1d4868698c78daa08e000a3614f3 Mon Sep 17 00:00:00 2001 From: Frank Zhong Date: Thu, 8 Feb 2024 16:56:03 +0000 Subject: [PATCH 2/4] Allow first multicast msg from unsynchronised client --- api/oc_replay.c | 13 +++++++++---- api/oc_replay.h | 3 ++- messaging/coap/engine.c | 6 +++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/api/oc_replay.c b/api/oc_replay.c index b603c3e3d..9a5b04c2c 100644 --- a/api/oc_replay.c +++ b/api/oc_replay.c @@ -117,7 +117,7 @@ get_record(oc_string_t rx_kid, oc_string_t rx_kid_ctx) // return false if no entry found, or if SSN is outside replay window bool oc_replay_check_client(uint64_t rx_ssn, oc_string_t rx_kid, - oc_string_t rx_kid_ctx) + oc_string_t rx_kid_ctx, bool is_mcast) { /* With CoAP over UDP, you cannot guarantee messages are received in order. @@ -158,9 +158,14 @@ oc_replay_check_client(uint64_t rx_ssn, oc_string_t rx_kid, */ struct oc_replay_record *rec = get_record(rx_kid, rx_kid_ctx); - if (rec == NULL) - return false; - + if (rec == NULL) { + if (is_mcast) { + oc_replay_add_client(rx_ssn, rx_kid, rx_kid_ctx); + return true; + } else { + return false; + } + } // received message matched existing record, so this record is useful & // should be kept around - thus we update the time here rec->time = oc_clock_time(); diff --git a/api/oc_replay.h b/api/oc_replay.h index 8c3337804..1d5bb646d 100644 --- a/api/oc_replay.h +++ b/api/oc_replay.h @@ -46,12 +46,13 @@ void oc_replay_add_client(uint64_t rx_ssn, oc_string_t rx_kid, * @param rx_ssn Sender Sequence Number of newly received OSCORE request * @param rx_kid Key Identifier of received request * @param rx_kid_ctx Key ID Context of received request + * @param is_mcast Whether received message is multicast * @return true Client is synchronised, you may accept the frame with the given * SSN * @return false Client is not synchronised, you must challenge the frame */ bool oc_replay_check_client(uint64_t rx_ssn, oc_string_t rx_kid, - oc_string_t rx_kid_ctx); + oc_string_t rx_kid_ctx, bool is_mcast); /** * @brief Free all clients with a given KID. Should be used whenever the diff --git a/messaging/coap/engine.c b/messaging/coap/engine.c index bb6f51f7a..107a1670e 100644 --- a/messaging/coap/engine.c +++ b/messaging/coap/engine.c @@ -529,7 +529,11 @@ coap_receive(oc_message_t *msg) oscore_read_piv(msg->endpoint.request_piv, msg->endpoint.request_piv_len, &ssn); - client_is_sync = oc_replay_check_client(ssn, kid, kid_ctx); + if (msg->endpoint.flags & MULTICAST) { + client_is_sync = oc_replay_check_client(ssn, kid, kid_ctx, true); + } else { + client_is_sync = oc_replay_check_client(ssn, kid, kid_ctx, false); + } } // Server-side logic for sending responses with an echo option, From 48b8db1a9a6374c918407b85d7da2a6baf07d3d1 Mon Sep 17 00:00:00 2001 From: Frank Zhong Date: Thu, 8 Feb 2024 17:01:09 +0000 Subject: [PATCH 3/4] Fix replaytest --- api/unittest/replaytest.cpp | 74 ++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/api/unittest/replaytest.cpp b/api/unittest/replaytest.cpp index f12c2e349..ed7b267a5 100644 --- a/api/unittest/replaytest.cpp +++ b/api/unittest/replaytest.cpp @@ -15,40 +15,40 @@ TEST(ReplayProtection, OutOfOrderFrames) oc_replay_add_client(6, kid, kid_ctx); // receive some valid frames, shifting the window - EXPECT_TRUE(oc_replay_check_client(7, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(8, kid, kid_ctx)); + EXPECT_TRUE(oc_replay_check_client(7, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(8, kid, kid_ctx, false)); // replay the frames - EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx)); + EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx, false)); // receive some valid frames out of order - EXPECT_TRUE(oc_replay_check_client(4, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(2, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(5, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(3, kid, kid_ctx)); + EXPECT_TRUE(oc_replay_check_client(4, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(2, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(5, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(3, kid, kid_ctx, false)); // replay the frames some more - EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(2, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(3, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(4, kid, kid_ctx)); + EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(2, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(3, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(4, kid, kid_ctx, false)); // shift the window by a lot - EXPECT_TRUE(oc_replay_check_client(20, kid, kid_ctx)); + EXPECT_TRUE(oc_replay_check_client(20, kid, kid_ctx, false)); // replays should still be detected - EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx)); - EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx)); + EXPECT_FALSE(oc_replay_check_client(6, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(7, kid, kid_ctx, false)); + EXPECT_FALSE(oc_replay_check_client(8, kid, kid_ctx, false)); // some more valid out-of-order frames - EXPECT_TRUE(oc_replay_check_client(17, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(18, kid, kid_ctx)); - EXPECT_TRUE(oc_replay_check_client(19, kid, kid_ctx)); + EXPECT_TRUE(oc_replay_check_client(17, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(18, kid, kid_ctx, false)); + EXPECT_TRUE(oc_replay_check_client(19, kid, kid_ctx, false)); } TEST(ReplayProtection, MultipleClients) @@ -78,20 +78,20 @@ TEST(ReplayProtection, MultipleClients) // for every added client, test out a new valid packet & a replayed packet - EXPECT_FALSE(oc_replay_check_client(5, kid1, empty)); - EXPECT_TRUE(oc_replay_check_client(6, kid1, empty)); - EXPECT_FALSE(oc_replay_check_client(5, kid2, empty)); - EXPECT_TRUE(oc_replay_check_client(6, kid2, empty)); + EXPECT_FALSE(oc_replay_check_client(5, kid1, empty, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid1, empty, false)); + EXPECT_FALSE(oc_replay_check_client(5, kid2, empty, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid2, empty, false)); - EXPECT_FALSE(oc_replay_check_client(5, kid1, con1)); - EXPECT_TRUE(oc_replay_check_client(6, kid1, con1)); - EXPECT_FALSE(oc_replay_check_client(5, kid1, con2)); - EXPECT_TRUE(oc_replay_check_client(6, kid1, con2)); + EXPECT_FALSE(oc_replay_check_client(5, kid1, con1, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid1, con1, false)); + EXPECT_FALSE(oc_replay_check_client(5, kid1, con2, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid1, con2, false)); - EXPECT_FALSE(oc_replay_check_client(5, kid3, con2)); - EXPECT_TRUE(oc_replay_check_client(6, kid3, con2)); - EXPECT_FALSE(oc_replay_check_client(5, kid4, con2)); - EXPECT_TRUE(oc_replay_check_client(6, kid4, con2)); + EXPECT_FALSE(oc_replay_check_client(5, kid3, con2, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid3, con2, false)); + EXPECT_FALSE(oc_replay_check_client(5, kid4, con2, false)); + EXPECT_TRUE(oc_replay_check_client(6, kid4, con2, false)); } TEST(ReplayProtection, TimeBasedFree) @@ -110,7 +110,7 @@ TEST(ReplayProtection, TimeBasedFree) // check 20 most recently added values - should still be readable for (int i = 20; i < 30; ++i) { (*oc_string(kid))--; - EXPECT_TRUE(oc_replay_check_client(6, kid, empty)); + EXPECT_TRUE(oc_replay_check_client(6, kid, empty, false)); } } @@ -125,8 +125,8 @@ TEST(ReplayProtection, RplWdo) oc_replay_add_client(5, kid, empty); // outside the upper bound of the replay window - EXPECT_FALSE(oc_replay_check_client(55, kid, empty)); + EXPECT_FALSE(oc_replay_check_client(55, kid, empty, false)); // fake an update to the replay window upper bound g_oscore_replaywindow = 64; - EXPECT_TRUE(oc_replay_check_client(55, kid, empty)); + EXPECT_TRUE(oc_replay_check_client(55, kid, empty, false)); } \ No newline at end of file From 32725c0c67882b7109d1e16f171391124a9ab8a7 Mon Sep 17 00:00:00 2001 From: Frank Zhong Date: Thu, 8 Feb 2024 17:27:35 +0000 Subject: [PATCH 4/4] Make multicast trusting configurable in CmakeLists --- CMakeLists.txt | 1 + api/oc_replay.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f0c1c092..3618b136d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,6 +31,7 @@ set(CLANG_TIDY_ENABLED OFF CACHE BOOL "Enable clang-tidy analysis during compila set(OC_USE_STORAGE ON CACHE BOOL "Persistent storage of data.") set(OC_USE_MULTICAST_SCOPE_2 ON CACHE BOOL "devices send also group multicast events with scope2.") set(OC_REPLAY_PROTECTION_ENABLED ON CACHE BOOL "Enable replay protection using the Echo option") +set(OC_TRUST_FIRST_MCAST ON CACHE BOOL "Trust first multicast message from an unsynchronised client") set(KNX_BUILTIN_MBEDTLS ON CACHE BOOL "Use built-in mbedTLS, as opposed to external lib from different project") set(KNX_BUILTIN_TINYCBOR ON CACHE BOOL "Use built-in TinyCBOR, as opposed to external lib from different project") diff --git a/api/oc_replay.c b/api/oc_replay.c index 9a5b04c2c..3c8f4b45e 100644 --- a/api/oc_replay.c +++ b/api/oc_replay.c @@ -159,12 +159,16 @@ oc_replay_check_client(uint64_t rx_ssn, oc_string_t rx_kid, struct oc_replay_record *rec = get_record(rx_kid, rx_kid_ctx); if (rec == NULL) { +#if OC_TRUST_FIRST_MCAST if (is_mcast) { oc_replay_add_client(rx_ssn, rx_kid, rx_kid_ctx); return true; } else { return false; } +#else + return false; +#endif } // received message matched existing record, so this record is useful & // should be kept around - thus we update the time here