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]>
  • Loading branch information
maribu and mguetschow committed Jan 10, 2025
1 parent e62e388 commit b7a8b01
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 8 deletions.
48 changes: 48 additions & 0 deletions sys/include/net/nanocoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,54 @@ static inline void *coap_get_token(const coap_pkt_t *pkt)
return coap_hdr_data_ptr(pkt->hdr);
}

/**
* @brief Get the token from a CoAP over UDP packet from a buffer
*
* @warning This function will happily read past the buffer on invalid input.
* Avoid using this function. Use @ref coap_parse instead.
*
* @pre This assumes that the buffer has already been validated (e.g. using
* @ref coap_udp_parse).

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

View workflow job for this annotation

GitHub Actions / static-tests

unable to resolve reference to 'coap_udp_parse' for ref command
*/
static inline void *coap_get_token_from_udp_buf(uint8_t *buf)
{
coap_udp_hdr_t *hdr = (void *)buf;

uint8_t tkl = hdr->ver_t_tkl & 0xf;
size_t tkl_ext_len = coap_get_ext_tkl_field_len_from_tkl_val(tkl);
return buf + sizeof(coap_udp_hdr_t) + tkl_ext_len;
}

/**
* @brief Get the token length from a CoAP over UDP packet from a buffer
*
* @warning This function will happily read past the buffer on invalid input.
* Avoid using this function. Use @ref coap_parse instead.
*
* @pre This assumes that the buffer has already been validated (e.g. using
* @ref coap_udp_parse).

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

View workflow job for this annotation

GitHub Actions / static-tests

unable to resolve reference to 'coap_udp_parse' for ref command
*/
static inline size_t coap_get_token_len_from_udp_buf(const uint8_t *buf)
{
coap_udp_hdr_t *hdr = (void *)buf;
uint8_t tkl = hdr->ver_t_tkl & 0xf;
size_t tkl_ext_len = coap_get_ext_tkl_field_len_from_tkl_val(tkl);

if (tkl_ext_len == 0) {
return tkl;
}

if (tkl_ext_len == 1) {
/* In https://www.rfc-editor.org/rfc/rfc8974#section-2.1 the magic
* number 13 has no name, so we don't make up one here */
return buf[sizeof(coap_udp_hdr_t)] + 13;
}

/* In https://www.rfc-editor.org/rfc/rfc8974#section-2.1 the magic
* number 269 has no name, so we don't make up one here */
return byteorder_bebuftohs(buf + sizeof(coap_udp_hdr_t)) + 269;
}

/**
* @brief Get the total length of a CoAP packet in the packet buffer
*
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);
uint8_t *buf = (uint8_t *)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_get_token_len_from_udp_buf(buf);
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_get_token_from_udp_buf(buf);
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 b7a8b01

Please sign in to comment.