Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow first multicast msg from unsynchronised client #764

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Allow first multicast msg from unsynchronised client #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
241a1cf
Notify observers on sending s-mode messages
FrankZRS Feb 8, 2024
cdc3d16
Allow first multicast msg from unsynchronised client
FrankZRS Feb 8, 2024
48b8db1
Fix replaytest
FrankZRS Feb 8, 2024
32725c0
Make multicast trusting configurable in CmakeLists
FrankZRS Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions api/oc_knx_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions api/oc_replay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -158,9 +158,18 @@ 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add cmake variable to enable that it always does an echo option?

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
rec->time = oc_clock_time();
Expand Down
3 changes: 2 additions & 1 deletion api/oc_replay.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 37 additions & 37 deletions api/unittest/replaytest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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));
}
}

Expand All @@ -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));
}
6 changes: 5 additions & 1 deletion messaging/coap/engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading