sys/net/gcoap: get rid of API abuse

maribu · mguetschow · benpicco · maribu · commit 590ca5d7ff74 · 2025-01-10T21:31:08.000+01:00
Calling `coap_get_token()` and `coap_get_token_length()` on an
(mostly) uninitialized `coap_pkt_t` did work so far due to
implementation details matching the expectations, but this is not
backed up by any API contract.

This fixes the API abuse by introducing and using a new API that does
read a token and token length from a CoAP over UDP packet out of a
buffer. This now provides the behavior expected by the caller and
commits to it via API contract.

Co-authored-by: mguetschow &lt;mikolai.guetschow@tu-dresden.de&gt;
Co-authored-by: benpicco &lt;benpicco@googlemail.com&gt;
diff --git a/sys/include/net/nanocoap.h b/sys/include/net/nanocoap.h
@@ -355,6 +355,7 @@ struct _coap_request_ctx {
 /* forward declarations */
 static inline uint8_t *coap_hdr_data_ptr(const coap_hdr_t *hdr);
 static inline size_t coap_hdr_get_token_len(const coap_hdr_t *hdr);
+static inline const void * coap_hdr_get_token(const coap_hdr_t *hdr);
 
 /**
  * @brief   Get resource path associated with a CoAP request
@@ -757,6 +758,28 @@ static inline size_t coap_hdr_get_token_len(const coap_hdr_t *hdr)
     return 0;
 }
 
+/**
+ * @brief       Get the Token of a CoAP over UDP (DTLS) packet
+ * @param[in]   hdr     CoAP over UDP header
+ * @return      The CoAP Token inside the packet that @p hdr belongs to
+ *
+ * @warning     This API is super goofy. It assumes that the packet is valid
+ *              and will read more than `sizeof(*hdr)` into the data `hdr`
+ *              points to while crossing fingers hard.
+ *
+ * @deprecated  This function was introduced to keep legacy code alive.
+ *              Introducing new callers should be avoided. In the RX path an
+ *              @ref coap_pkt_t will be available, so that you can call
+ *              @ref coap_get_token instead. In the TX path the token was
+ *              added by us, so we really should know.
+ */
+static inline const void * coap_hdr_get_token(const coap_hdr_t *hdr)
+{
+    uint8_t *token = (void *)hdr;
+    token += sizeof(*hdr) + coap_hdr_tkl_ext_len(hdr);
+    return token;
+}
+
 /**
  * @brief       Get the header length of a CoAP packet.
  *
diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c
@@ -945,17 +945,13 @@ static int _find_resource(gcoap_socket_type_t tl_type,
 static gcoap_request_memo_t* _find_req_memo_by_token(const sock_udp_ep_t *remote,
                                                      const uint8_t *token, size_t tkl)
 {
-    /* no need to initialize struct; we only care about buffer contents below */
-    coap_pkt_t memo_pdu_data;
-    coap_pkt_t *memo_pdu = &memo_pdu_data;
-
     for (int i = 0; i < CONFIG_GCOAP_REQ_WAITING_MAX; i++) {
         if (_coap_state.open_reqs[i].state == GCOAP_MEMO_UNUSED) {
             continue;
         }
 
         gcoap_request_memo_t *memo = &_coap_state.open_reqs[i];
-        memo_pdu->hdr = gcoap_request_memo_get_hdr(memo);
+        coap_hdr_t *hdr = gcoap_request_memo_get_hdr(memo);
 
         /* verbose debug to catch bugs with request/response matching */
 #if SOCK_HAS_IPV4
@@ -972,11 +968,12 @@ static gcoap_request_memo_t* _find_req_memo_by_token(const sock_udp_ep_t *remote
               tkl);
 #endif
 
-        if (coap_get_token_len(memo_pdu) != tkl) {
-            DEBUG("Token length mismatch %u\n", coap_get_token_len(memo_pdu));
+        size_t memo_tkl = coap_hdr_get_token_len(hdr);
+        if (memo_tkl != tkl) {
+            DEBUG("Token length mismatch %" PRIuSIZE "\n", memo_tkl);
             continue;
         }
-        const uint8_t *memo_token = coap_get_token(memo_pdu);
+        const uint8_t *memo_token = coap_hdr_get_token(hdr);
         if (memcmp(token, memo_token, tkl)) {
             DEBUG("Token mismatch 0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
                   memo_token[0], memo_token[1], memo_token[2], memo_token[3],
