sys/net/nanocoap: fix invalid RST messages

An RST message has no token, so don't reply with a token when sending RST. This also adds unit tests to ensure this this exact bug does not sneak back in.
RIOT-OS · Dec 12, 2024 · c7af4b2 · c7af4b2
1 parent fa011a8
commit c7af4b2
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 11 deletions.
diff --git a/sys/net/application_layer/nanocoap/nanocoap.c b/sys/net/application_layer/nanocoap/nanocoap.c
@@ -656,23 +656,23 @@ ssize_t coap_build_reply(coap_pkt_t *pkt, unsigned code,
                          uint8_t *rbuf, unsigned rlen, unsigned payload_len)
 {
     unsigned tkl = coap_get_token_len(pkt);
+    unsigned type = COAP_TYPE_NON;
+
+    if (!code) {
+        /* if code is COAP_CODE_EMPTY (zero), assume Reset (RST) type.
+         * RST message have no token */
+        type = COAP_TYPE_RST;
+        tkl = 0;
+    }
+    else if (coap_get_type(pkt) == COAP_TYPE_CON) {
+        type = COAP_TYPE_ACK;
+    }
     unsigned len = sizeof(coap_hdr_t) + tkl;
 
     if ((len + payload_len) > rlen) {
         return -ENOSPC;
     }
 
-    /* if code is COAP_CODE_EMPTY (zero), assume Reset (RST) type */
-    unsigned type = COAP_TYPE_RST;
-    if (code) {
-        if (coap_get_type(pkt) == COAP_TYPE_CON) {
-            type = COAP_TYPE_ACK;
-        }
-        else {
-            type = COAP_TYPE_NON;
-        }
-    }
-
     uint32_t no_response;
     if (coap_opt_get_uint(pkt, COAP_OPT_NO_RESPONSE, &no_response) == 0) {
 

diff --git a/tests/unittests/tests-nanocoap/tests-nanocoap.c b/tests/unittests/tests-nanocoap/tests-nanocoap.c
@@ -1185,6 +1185,48 @@ static void test_nanocoap__token_length_ext_269(void)
     TEST_ASSERT_EQUAL_INT(14, hdr->ver_t_tkl & 0xf);
 }
 
+/*
+ * Test that a RST message can be generated and parsed
+ */
+static void test_nanocoap___rst_message(void)
+{
+    static const uint8_t rst_expected[4] = {
+        0x70, /* Version = 0b01, Type = 0b11 (RST), Token Length = 0b0000 */
+        0x00, /* Code = 0x00 */
+        0x13, 0x37 /* Message ID = 0x1337 */
+    };
+
+    uint8_t buf[16];
+    /* trivial case: build a reset message */
+    memset(buf, 0x55, sizeof(buf));
+    TEST_ASSERT_EQUAL_INT(sizeof(rst_expected),
+                          coap_build_hdr((void *)buf, COAP_TYPE_RST, NULL, 0,
+                                         0, 0x1337));
+    TEST_ASSERT(0 == memcmp(rst_expected, buf, sizeof(rst_expected)));
+    /* did it write past the expected bytes? */
+    TEST_ASSERT_EQUAL_INT(0x55, buf[sizeof(rst_expected)]);
+
+    /* now check that parsing it back works */
+    coap_pkt_t pkt;
+    TEST_ASSERT_EQUAL_INT(0, coap_parse(&pkt, buf, sizeof(rst_expected)));
+    TEST_ASSERT_EQUAL_INT(COAP_TYPE_RST, coap_get_type(&pkt));
+    TEST_ASSERT_EQUAL_INT(0, coap_get_code_raw(&pkt));
+    TEST_ASSERT_EQUAL_INT(0, coap_get_token_len(&pkt));
+
+    /* now check that generating a RST reply works */
+    static uint8_t con_request[8] = {
+        0x44, /* Version = 0b01, Type = 0b00 (CON), Token Length = 0b0100 */
+        0x01, /* Code = 0.01 (GET) */
+        0x13, 0x37, /* Message ID = 0x1337 */
+        0xde, 0xed, 0xbe, 0xef, /* Token = 0xdeadbeef */
+    };
+    memset(buf, 0x55, sizeof(buf));
+    TEST_ASSERT_EQUAL_INT(0, coap_parse(&pkt, con_request, sizeof(con_request)));
+    TEST_ASSERT_EQUAL_INT(sizeof(rst_expected), coap_build_reply(&pkt, 0, buf, sizeof(buf), 0));
+    TEST_ASSERT(0 == memcmp(rst_expected, buf, sizeof(rst_expected)));
+    TEST_ASSERT_EQUAL_INT(0x55, buf[sizeof(rst_expected)]);
+}
+
 Test *tests_nanocoap_tests(void)
 {
     EMB_UNIT_TESTFIXTURES(fixtures) {
@@ -1223,6 +1265,7 @@ Test *tests_nanocoap_tests(void)
         new_TestFixture(test_nanocoap__token_length_over_limit),
         new_TestFixture(test_nanocoap__token_length_ext_16),
         new_TestFixture(test_nanocoap__token_length_ext_269),
+        new_TestFixture(test_nanocoap___rst_message),
     };
 
     EMB_UNIT_TESTCALLER(nanocoap_tests, NULL, NULL, fixtures);