Skip to content

Commit

Permalink
sys/net/gcoap: get rid of API abuse
Browse files Browse the repository at this point in the history
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 <[email protected]>
Co-authored-by: benpicco <[email protected]>
  • Loading branch information
3 people committed Jan 10, 2025
1 parent e62e388 commit 20d7bd3
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
23 changes: 23 additions & 0 deletions sys/include/net/nanocoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 size_t coap_hdr_get_token(const coap_hdr_t *hdr);

Check failure on line 358 in sys/include/net/nanocoap.h

View workflow job for this annotation

GitHub Actions / static-tests

Member coap_hdr_get_token(const coap_hdr_t *hdr) (function) of group net_nanocoap is not documented.

/**
* @brief Get resource path associated with a CoAP request
Expand Down Expand Up @@ -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 void * coap_hdr_get_token(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.
*
Expand Down
13 changes: 5 additions & 8 deletions sys/net/application_layer/gcoap/gcoap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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],
Expand Down

0 comments on commit 20d7bd3

Please sign in to comment.