Skip to content

Commit

Permalink
sys/net/gcoap: reduce insanity of hack
Browse files Browse the repository at this point in the history
gcoap contains a hack where a `coap_pkt_t` is pulled out of thin air,
parts of the members are left uninitialized and a function is called on
that mostly uninitialized data while crossing fingers hard that the
result will be correct. (With the current implementation of the used
function this hack does actually work.)

Estimated level of insanity: 😱😱😱😱😱

This adds to insane functions to get the length of a token and the
length of a header of a CoAP packet while crossing fingers hard that
the packet is valid and that the functions do not overread.

Estimated level of insanity: 😱😱😱

The newly introduced insane functions are used to replace the old
insane hack, resulting in an estimated reduction of insanity of 😱😱.

Side note: This actually does fix a bug, as the old code did not take
           into account the length of the extended TKL field in case of
           RFC 8974 being used. But that is a bug in the abused API,
           and not in the caller abusing the API.
  • Loading branch information
maribu committed Nov 1, 2024
1 parent 00e25ad commit 5a0a688
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 4 deletions.
49 changes: 49 additions & 0 deletions sys/include/net/nanocoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,55 @@ static inline void coap_hdr_set_type(coap_hdr_t *hdr, unsigned type)
hdr->ver_t_tkl &= ~0x30;
hdr->ver_t_tkl |= type << 4;
}

/**
* @brief Get the token length of a CoAP over UDP (DTLS) packet
* @param[in] hdr CoAP over UDP header
* return The size of the token in bytes
*
* @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 size_t coap_hdr_get_token_len(const coap_hdr_t *hdr)
{
const uint8_t *buf = (const void *)hdr;
switch (coap_hdr_tkl_ext_len(hdr)) {
case 0:
return hdr->ver_t_tkl & 0xf;
case 1:
return buf[sizeof(coap_hdr_t)] + 13;
case 2:
return byteorder_bebuftohs(buf + sizeof(coap_hdr_t)) + 269;
}

return 0;
}

/**
* @brief Get the header length of a CoAP packet.
*
* @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_total_hdr_len instead. In the TX path the header
* was created by us (e.g. using @ref coap_build_hdr which returns
* the header size), so we really should know already.
*/
static inline size_t coap_hdr_len(const coap_hdr_t *hdr)
{
return sizeof(*hdr) + coap_hdr_tkl_ext_len(hdr) + coap_hdr_get_token_len(hdr);
}
/**@}*/

/**
Expand Down
7 changes: 3 additions & 4 deletions sys/net/application_layer/gcoap/gcoap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1454,10 +1454,9 @@ static ssize_t _cache_build_response(nanocoap_cache_entry_t *ce, coap_pkt_t *pdu

static void _copy_hdr_from_req_memo(coap_pkt_t *pdu, gcoap_request_memo_t *memo)
{
coap_pkt_t req_pdu;

req_pdu.hdr = gcoap_request_memo_get_hdr(memo);
memcpy(pdu->hdr, req_pdu.hdr, coap_get_total_hdr_len(&req_pdu));
const coap_hdr_t *hdr = gcoap_request_memo_get_hdr(memo);
size_t hdr_len = coap_hdr_len(hdr);
memcpy(pdu->hdr, hdr, hdr_len);
}

static void _receive_from_cache_cb(void *ctx)
Expand Down

0 comments on commit 5a0a688

Please sign in to comment.