From 0b41ee051104cf7c878b3afaed810371a43f0b87 Mon Sep 17 00:00:00 2001 From: Jon Shallow Date: Mon, 27 Jan 2025 16:15:55 +0000 Subject: [PATCH] cache_response: Cache the latest response to a CON request If the ACK (not empty) response gets lost, the repeated CON request will just get the cached response without invoking further analysis. This stops things like OSCORE reporting retry issues. This functionality needs to be enabled by setting the COAP_BLOCK_CACHE_RESPONSE bit when calling coap_context_set_block_mode(). If not enabled, then, for OSCORE the response is sent as a separate response (empty ACK followed by CON response), which is how code previous to this PR works. --- examples/coap-server.c | 2 +- include/coap3/coap_block.h | 1 + include/coap3/coap_block_internal.h | 6 +- include/coap3/coap_cache_internal.h | 2 +- include/coap3/coap_net_internal.h | 3 +- include/coap3/coap_pdu_internal.h | 16 +++- include/coap3/coap_session_internal.h | 4 +- man/coap-server.txt.in | 4 +- man/coap_block.txt.in | 6 ++ src/coap_block.c | 2 +- src/coap_io.c | 2 +- src/coap_net.c | 106 ++++++++++++++++++++++---- src/coap_oscore.c | 34 +++++---- src/coap_pdu.c | 8 ++ src/coap_session.c | 3 + 15 files changed, 159 insertions(+), 40 deletions(-) diff --git a/examples/coap-server.c b/examples/coap-server.c index 772dfcf8de..22b4bbb5c0 100644 --- a/examples/coap-server.c +++ b/examples/coap-server.c @@ -1625,7 +1625,7 @@ usage(const char *program, const char *version) { "\t \t\tif the -A option is used\n" "\t-L value\tSum of one or more COAP_BLOCK_* flag valuess for block\n" "\t \t\thandling methods. Default is 1 (COAP_BLOCK_USE_LIBCOAP)\n" - "\t \t\t(Sum of one or more of 1,2,4 64 and 128)\n" + "\t \t\t(Sum of one or more of 1,2,4 64, 128 and 256)\n" "\t-N \t\tMake \"observe\" responses NON-confirmable. Even if set\n" "\t \t\tevery fifth response will still be sent as a confirmable\n" "\t \t\tresponse (RFC 7641 requirement)\n" diff --git a/include/coap3/coap_block.h b/include/coap3/coap_block.h index f2a94672f1..3404bb2f8f 100644 --- a/include/coap3/coap_block.h +++ b/include/coap3/coap_block.h @@ -67,6 +67,7 @@ typedef struct { #define COAP_BLOCK_STLESS_BLOCK2 0x40 /* (svr)Server is stateless for handling Block2 */ #define COAP_BLOCK_NOT_RANDOM_BLOCK1 0x80 /* (svr)Disable server handling random order block1 */ +#define COAP_BLOCK_CACHE_RESPONSE 0x100 /* (svr)Cache CON request's response */ /* WARNING: Added defined values must not encroach into 0xff000000 which are defined elsewhere */ /** diff --git a/include/coap3/coap_block_internal.h b/include/coap3/coap_block_internal.h index 6c4c805e25..9dea3e72b9 100644 --- a/include/coap3/coap_block_internal.h +++ b/include/coap3/coap_block_internal.h @@ -44,14 +44,16 @@ COAP_BLOCK_NO_PREEMPTIVE_RTAG | \ COAP_BLOCK_STLESS_FETCH | \ COAP_BLOCK_STLESS_BLOCK2 | \ - COAP_BLOCK_NOT_RANDOM_BLOCK1) + COAP_BLOCK_NOT_RANDOM_BLOCK1 | \ + COAP_BLOCK_CACHE_RESPONSE) #else /* ! COAP_Q_BLOCK_SUPPORT */ #define COAP_BLOCK_SET_MASK (COAP_BLOCK_USE_LIBCOAP | \ COAP_BLOCK_SINGLE_BODY | \ COAP_BLOCK_NO_PREEMPTIVE_RTAG | \ COAP_BLOCK_STLESS_FETCH | \ COAP_BLOCK_STLESS_BLOCK2 | \ - COAP_BLOCK_NOT_RANDOM_BLOCK1) + COAP_BLOCK_NOT_RANDOM_BLOCK1 | \ + COAP_BLOCK_CACHE_RESPONSE) #endif /* ! COAP_Q_BLOCK_SUPPORT */ #define COAP_BLOCK_MAX_SIZE_MASK 0x7000000 /* (svr)Mask to get the max supported block size */ diff --git a/include/coap3/coap_cache_internal.h b/include/coap3/coap_cache_internal.h index 33a82c9a3e..a3db11f6a4 100644 --- a/include/coap3/coap_cache_internal.h +++ b/include/coap3/coap_cache_internal.h @@ -178,7 +178,7 @@ int coap_digest_update(coap_digest_ctx_t *digest_ctx, /** * Finalize the coap_digest information into the provided - * @p digest_buffer. + * @p digest_buffer. coap_digest_free() is always called, even if error. * * Internal function. * diff --git a/include/coap3/coap_net_internal.h b/include/coap3/coap_net_internal.h index d06ac2c22b..4e2cb183c2 100644 --- a/include/coap3/coap_net_internal.h +++ b/include/coap3/coap_net_internal.h @@ -446,7 +446,8 @@ int coap_check_code_class(coap_session_t *session, coap_pdu_t *pdu); * * @param session The CoAP session. * @param pdu The CoAP PDU to send. - * @param request_pdu Not currently used. + * @param request_pdu The request pdu if caching last CON request / response, + * else NULL (the usual calling case). * * @return The message id of the sent message or @c * COAP_INVALID_MID on error. diff --git a/include/coap3/coap_pdu_internal.h b/include/coap3/coap_pdu_internal.h index 0f100447c4..cb7434aab8 100644 --- a/include/coap3/coap_pdu_internal.h +++ b/include/coap3/coap_pdu_internal.h @@ -385,7 +385,7 @@ coap_pdu_t *coap_new_pdu_lkd(coap_pdu_type_t type, coap_pdu_code_t code, * where the sending and receiving PDUs need to be explicitly deleted. * * Note: If called with a reference count > 0, the reference count is - * decremented and the PDU still exists. + * decremented and the PDU is not deleted. * * @param pdu The PDU for free off. */ @@ -412,6 +412,20 @@ coap_pdu_t *coap_pdu_duplicate_lkd(const coap_pdu_t *old_pdu, const uint8_t *token, coap_opt_filter_t *drop_options); +/** + * Increment reference counter on a pdu to stop it prematurely getting freed off + * when coap_delete_pdu() is called. + * + * In other words, if coap_pdu_reference_lkd() is called once, then the first call + * to coap_delete_pdu() will decrement the reference counter, but not delete the PDU + * and the second call to coap_delete_pdu() then actually deletes the PDU. + * + * Note: This function must be called in the locked state. + * + * @param pdu The CoAP PDU. + * @return same as PDU + */ +coap_pdu_t *coap_pdu_reference_lkd(coap_pdu_t *pdu); /** @} */ #endif /* COAP_COAP_PDU_INTERNAL_H_ */ diff --git a/include/coap3/coap_session_internal.h b/include/coap3/coap_session_internal.h index ea0db889ea..f284864c75 100644 --- a/include/coap3/coap_session_internal.h +++ b/include/coap3/coap_session_internal.h @@ -222,7 +222,9 @@ struct coap_session_t { coap_response_t last_con_handler_res; /**< The result of calling the response handler of the last CON */ #if COAP_SERVER_SUPPORT - coap_bin_const_t *client_cid; /**< Contains client CID or NULL */ + coap_bin_const_t *client_cid; /**< Contains client CID or NULL */ + coap_pdu_t *cached_pdu; /**< Cached copy of last ACK response PDU */ + coap_digest_t cached_pdu_cksum; /**< Checksum of last CON request PDU */ #endif /* COAP_SERVER_SUPPORT */ #if COAP_CLIENT_SUPPORT coap_pdu_t *resp_pdu; /**< PDU returned in coap_send_recv() call */ diff --git a/man/coap-server.txt.in b/man/coap-server.txt.in index 0bd7e3aba3..5f538969dd 100644 --- a/man/coap-server.txt.in +++ b/man/coap-server.txt.in @@ -128,13 +128,15 @@ OPTIONS - General *-L* value:: Sum of one or more COAP_BLOCK_* flag values for different block handling - methods. Default is 1 (COAP_BLOCK_USE_LIBCOAP). + methods. Default is 1 (COAP_BLOCK_USE_LIBCOAP). This vlue can be set as a + hex value - e.g. 0x103. COAP_BLOCK_USE_LIBCOAP 1 COAP_BLOCK_SINGLE_BODY 2 COAP_BLOCK_TRY_Q_BLOCK 4 COAP_BLOCK_STLESS_BLOCK2 64 COAP_BLOCK_NOT_RANDOM_BLOCK1 128 + COAP_BLOCK_CACHE_RESPONSE 256 *-N* :: Send NON-confirmable message for "observe" responses. If option *-N* is diff --git a/man/coap_block.txt.in b/man/coap_block.txt.in index b57243d553..de866b11ae 100644 --- a/man/coap_block.txt.in +++ b/man/coap_block.txt.in @@ -159,6 +159,7 @@ session _block_mode_. #define COAP_BLOCK_STLESS_FETCH 0x20 /* (Client) Assume server supports stateless FETCH */ #define COAP_BLOCK_STLESS_BLOCK2 0x40 /* (Server) Server is stateless for Block2 transfers */ #define COAP_BLOCK_NOT_RANDOM_BLOCK1 0x80 /* (Server) Disable server handling random order block1 */ +#define COAP_BLOCK_CACHE_RESPONSE 0x100 /* (Server) Cache CON request's response */ ---- _block_mode_ is an or'd set of zero or more COAP_BLOCK_* definitions. @@ -224,6 +225,11 @@ for Block1 by the client. *NOTE:* This is ignored if Q-Block1 is being used. +If *COAP_BLOCK_CACHE_RESPONSE* is set, then the server will cache the response +for the latest unreliable CON request so that if the CON request is repeated, +the cached response gets re-transmitted. If it is not set, then the response is +sent as a separate response (empty ACK sent first) using CON. + *Function: coap_context_set_max_block_size()* The *coap_context_set_max_block_size*() function is used to set the diff --git a/src/coap_block.c b/src/coap_block.c index e1be358217..f36d8bb840 100644 --- a/src/coap_block.c +++ b/src/coap_block.c @@ -4147,7 +4147,7 @@ coap_handle_response_get_block(coap_context_t *context, } else { /* processing coap_send_recv() call */ session->resp_pdu = rcvd; - rcvd->ref++; + coap_pdu_reference_lkd(session->resp_pdu); /* Will get freed off when PDU is freed off */ rcvd->data_free = lg_crcv->body_data; lg_crcv->body_data = NULL; diff --git a/src/coap_io.c b/src/coap_io.c index 8fab581444..e940ec9954 100644 --- a/src/coap_io.c +++ b/src/coap_io.c @@ -1870,7 +1870,7 @@ coap_io_process_with_fds_lkd(coap_context_t *ctx, uint32_t timeout_ms, coap_win_error_to_errno(); #endif if (errno != EINTR) { - coap_log_debug("%s", coap_socket_strerror()); + coap_log_err("select: %s", coap_socket_strerror()); return -1; } } diff --git a/src/coap_net.c b/src/coap_net.c index 56882601b7..816b65c111 100644 --- a/src/coap_net.c +++ b/src/coap_net.c @@ -1230,8 +1230,7 @@ coap_client_delay_first(coap_session_t *session) { coap_session_state_t current_state = session->state; if (session->delay_recursive) { - assert(0); - return 1; + return 0; } else { session->delay_recursive = 1; } @@ -1675,14 +1674,45 @@ coap_send_lkd(coap_session_t *session, coap_pdu_t *pdu) { return COAP_INVALID_MID; } +#if COAP_SERVER_SUPPORT +static int +coap_pdu_cksum(const coap_pdu_t *pdu, coap_digest_t *digest_buffer) { + coap_digest_ctx_t *digest_ctx = coap_digest_setup(); + + if (!digest_ctx || !pdu) { + goto fail; + } + if (pdu->used_size && pdu->token) { + if (!coap_digest_update(digest_ctx, pdu->token, pdu->used_size)) { + goto fail; + } + } + if (!coap_digest_update(digest_ctx, (const uint8_t *)&pdu->type, sizeof(pdu->type))) { + goto fail; + } + if (!coap_digest_update(digest_ctx, (const uint8_t *)&pdu->code, sizeof(pdu->code))) { + goto fail; + } + if (!coap_digest_final(digest_ctx, digest_buffer)) + return 0; + + return 1; + +fail: + coap_digest_free(digest_ctx); + return 0; +} +#endif /* COAP_SERVER_SUPPORT */ + coap_mid_t coap_send_internal(coap_session_t *session, coap_pdu_t *pdu, coap_pdu_t *request_pdu) { uint8_t r; ssize_t bytes_written; coap_opt_iterator_t opt_iter; +#if ! COAP_SERVER_SUPPORT (void)request_pdu; - +#endif /* COAP_SERVER_SUPPORT */ pdu->session = session; if (pdu->code == COAP_RESPONSE_CODE(508)) { /* @@ -1864,6 +1894,19 @@ coap_send_internal(coap_session_t *session, coap_pdu_t *pdu, coap_pdu_t *request #endif /* COAP_OSCORE_SUPPORT */ bytes_written = coap_send_pdu(session, pdu, NULL); +#if COAP_SERVER_SUPPORT + if ((session->block_mode & COAP_BLOCK_CACHE_RESPONSE) && + session->cached_pdu != pdu && + request_pdu && COAP_PROTO_NOT_RELIABLE(session->proto) && + COAP_PDU_IS_REQUEST(request_pdu) && + COAP_PDU_IS_RESPONSE(pdu) && pdu->type == COAP_MESSAGE_ACK) { + coap_delete_pdu_lkd(session->cached_pdu); + session->cached_pdu = pdu; + coap_pdu_reference_lkd(session->cached_pdu); + coap_pdu_cksum(request_pdu, &session->cached_pdu_cksum); + } +#endif /* COAP_SERVER_SUPPORT */ + if (bytes_written == COAP_PDU_DELAYED) { /* do not free pdu as it is stored with session for later use */ return pdu->mid; @@ -1971,7 +2014,7 @@ coap_send_recv_lkd(coap_session_t *session, coap_pdu_t *request_pdu, session->block_mode |= COAP_BLOCK_SINGLE_BODY; session->doing_send_recv = 1; /* So the user needs to delete the PDU */ - request_pdu->ref++; + coap_pdu_reference_lkd(request_pdu); mid = coap_send_lkd(session, request_pdu); if (mid == COAP_INVALID_MID) { if (!session->doing_send_recv) @@ -2019,7 +2062,7 @@ coap_send_recv_lkd(coap_session_t *session, coap_pdu_t *request_pdu, ticks_so_far = now - start; time_so_far_ms = (uint32_t)((ticks_so_far * 1000) / COAP_TICKS_PER_SECOND); ret = time_so_far_ms; - /* Give PDU to user */ + /* Give PDU to user who will be calling coap_delete_pdu() */ *response_pdu = session->resp_pdu; session->resp_pdu = NULL; if (*response_pdu == NULL) { @@ -2033,8 +2076,7 @@ coap_send_recv_lkd(coap_session_t *session, coap_pdu_t *request_pdu, fail: session->block_mode = block_mode; session->doing_send_recv = 0; - if (session->resp_pdu && session->resp_pdu->ref) - session->resp_pdu->ref--; + /* delete referenced copy */ coap_delete_pdu_lkd(session->resp_pdu); session->resp_pdu = NULL; coap_delete_bin_const(session->req_token); @@ -3183,7 +3225,8 @@ static coap_str_const_t coap_default_uri_wellknown = { static coap_resource_t resource_uri_wellknown; static void -handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu) { +handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu, + coap_pdu_t *orig_pdu) { coap_method_handler_t h = NULL; coap_pdu_t *response = NULL; coap_opt_filter_t opt_filter; @@ -3586,10 +3629,10 @@ handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu } } - /* TODO for non-proxy requests */ if (resource == context->proxy_uri_resource && COAP_PROTO_NOT_RELIABLE(session->proto) && - pdu->type == COAP_MESSAGE_CON) { + pdu->type == COAP_MESSAGE_CON && + !(session->block_mode & COAP_BLOCK_CACHE_RESPONSE)) { /* Make the proxy response separate and fix response later */ send_early_empty_ack = 1; } @@ -3717,7 +3760,7 @@ handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu goto finish; } #endif /* COAP_Q_BLOCK_SUPPORT */ - if (coap_send_internal(session, response, NULL) == COAP_INVALID_MID) { + if (coap_send_internal(session, response, orig_pdu ? orig_pdu : pdu) == COAP_INVALID_MID) { coap_log_debug("cannot send response for mid=0x%04x\n", mid); } } else { @@ -3890,7 +3933,7 @@ handle_response(coap_context_t *context, coap_session_t *session, coap_binary_equal(session->req_token, &rcvd->actual_token)) { /* processing coap_send_recv() call */ session->resp_pdu = rcvd; - rcvd->ref++; + coap_pdu_reference_lkd(rcvd); coap_send_ack_lkd(session, rcvd); session->last_con_handler_res = COAP_RESPONSE_OK; } else if (context->response_handler) { @@ -4027,6 +4070,7 @@ coap_dispatch(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu) { coap_queue_t *sent = NULL; coap_pdu_t *response; + coap_pdu_t *orig_pdu = NULL; coap_opt_filter_t opt_filter; int is_ping_rst; int packet_is_bad = 0; @@ -4055,6 +4099,35 @@ coap_dispatch(coap_context_t *context, coap_session_t *session, coap_option_filter_clear(&opt_filter); +#if COAP_SERVER_SUPPORT + /* See if this a repeat request */ + if (COAP_PDU_IS_REQUEST(pdu) && session->cached_pdu && + (session->block_mode & COAP_BLOCK_CACHE_RESPONSE)) { + coap_digest_t digest; + + coap_pdu_cksum(pdu, &digest); + if (memcmp(&digest, &session->cached_pdu_cksum, sizeof(digest)) == 0) { +#if COAP_OSCORE_SUPPORT + uint8_t oscore_encryption = session->oscore_encryption; + + session->oscore_encryption = 0; +#endif /* COAP_OSCORE_SUPPORT */ + /* Account for coap_send_internal() doing a coap_delete_pdu() and + cached_pdu must not be removed */ + coap_pdu_reference_lkd(session->cached_pdu); + coap_log_debug("Retransmit response to duplicate request\n"); + if (coap_send_internal(session, session->cached_pdu, NULL) != COAP_INVALID_MID) { +#if COAP_OSCORE_SUPPORT + session->oscore_encryption = oscore_encryption; +#endif /* COAP_OSCORE_SUPPORT */ + return; + } +#if COAP_OSCORE_SUPPORT + session->oscore_encryption = oscore_encryption; +#endif /* COAP_OSCORE_SUPPORT */ + } + } +#endif /* COAP_SERVER_SUPPORT */ #if COAP_OSCORE_SUPPORT if (!COAP_PDU_IS_SIGNALING(pdu) && coap_option_check_critical(session, pdu, &opt_filter) == 0) { @@ -4118,12 +4191,16 @@ coap_dispatch(coap_context_t *context, coap_session_t *session, if (decrypt) { /* find message id in sendqueue to stop retransmission and get sent */ coap_remove_from_queue(&context->sendqueue, session, pdu->mid, &sent); + /* Bump ref so pdu is not freed of, and keep a pointer to it */ + orig_pdu = pdu; + coap_pdu_reference_lkd(orig_pdu); if ((dec_pdu = coap_oscore_decrypt_pdu(session, pdu)) == NULL) { if (session->recipient_ctx == NULL || session->recipient_ctx->initial_state == 0) { coap_log_warn("OSCORE: PDU could not be decrypted\n"); } coap_delete_node_lkd(sent); + coap_delete_pdu_lkd(orig_pdu); return; } else { session->oscore_encryption = 1; @@ -4368,7 +4445,7 @@ coap_dispatch(coap_context_t *context, coap_session_t *session, #endif /* !COAP_DISABLE_TCP */ #if COAP_SERVER_SUPPORT if (COAP_PDU_IS_REQUEST(pdu)) - handle_request(context, session, pdu); + handle_request(context, session, pdu, orig_pdu); else #endif /* COAP_SERVER_SUPPORT */ #if COAP_CLIENT_SUPPORT @@ -4414,6 +4491,7 @@ coap_dispatch(coap_context_t *context, coap_session_t *session, coap_handle_event_lkd(context, COAP_EVENT_BAD_PACKET, session); } } + coap_delete_pdu_lkd(orig_pdu); coap_delete_node_lkd(sent); #if COAP_OSCORE_SUPPORT coap_delete_pdu_lkd(dec_pdu); @@ -4595,7 +4673,7 @@ coap_check_async(coap_context_t *context, coap_tick_t now) { LL_FOREACH_SAFE(context->async_state, async, tmp) { if (async->delay != 0 && async->delay <= now) { /* Send off the request to the application */ - handle_request(context, async->session, async->pdu); + handle_request(context, async->session, async->pdu, NULL); /* Remove this async entry as it has now fired */ coap_free_async_lkd(async->session, async); diff --git a/src/coap_oscore.c b/src/coap_oscore.c index 28de41b685..61a68f762c 100644 --- a/src/coap_oscore.c +++ b/src/coap_oscore.c @@ -667,22 +667,24 @@ coap_oscore_new_pdu_encrypted_lkd(coap_session_t *session, oscore_delete_association(session, association); association = NULL; - /* - * If this is a response ACK with data, make it a separate response - * by sending an Empty ACK and changing osc_pdu's MID and type. This - * then allows lost response ACK (now CON) with data to be recovered. - */ - if (coap_request == 0 && osc_pdu->type == COAP_MESSAGE_ACK && - COAP_RESPONSE_CLASS(pdu->code) == 2 && - COAP_PROTO_NOT_RELIABLE(session->proto)) { - coap_pdu_t *empty = coap_pdu_init(COAP_MESSAGE_ACK, - 0, - osc_pdu->mid, - 0); - if (empty) { - if (coap_send_internal(session, empty, NULL) != COAP_INVALID_MID) { - osc_pdu->mid = coap_new_message_id_lkd(session); - osc_pdu->type = COAP_MESSAGE_CON; + if (!(session->block_mode & COAP_BLOCK_CACHE_RESPONSE)) { + /* + * If this is a response ACK with data, make it a separate response + * by sending an Empty ACK and changing osc_pdu's MID and type. This + * then allows lost response ACK (now CON) with data to be recovered. + */ + if (coap_request == 0 && osc_pdu->type == COAP_MESSAGE_ACK && + COAP_RESPONSE_CLASS(pdu->code) == 2 && + COAP_PROTO_NOT_RELIABLE(session->proto)) { + coap_pdu_t *empty = coap_pdu_init(COAP_MESSAGE_ACK, + 0, + osc_pdu->mid, + 0); + if (empty) { + if (coap_send_internal(session, empty, NULL) != COAP_INVALID_MID) { + osc_pdu->mid = coap_new_message_id_lkd(session); + osc_pdu->type = COAP_MESSAGE_CON; + } } } } diff --git a/src/coap_pdu.c b/src/coap_pdu.c index 3485851310..5cfa88da69 100644 --- a/src/coap_pdu.c +++ b/src/coap_pdu.c @@ -1618,3 +1618,11 @@ coap_pdu_set_mid(coap_pdu_t *pdu, coap_mid_t mid) { #endif /* UINT_MAX > 65535 */ pdu->mid = mid; } + +coap_pdu_t * +coap_pdu_reference_lkd(coap_pdu_t *pdu) { + if (pdu != NULL) { + ++pdu->ref; + } + return pdu; +} diff --git a/src/coap_session.c b/src/coap_session.c index b0a355591d..207416e4c4 100644 --- a/src/coap_session.c +++ b/src/coap_session.c @@ -591,6 +591,9 @@ coap_session_free(coap_session_t *session) { #endif /* COAP_CLIENT_SUPPORT */ coap_delete_bin_const(session->last_token); coap_delete_bin_const(session->echo); +#if COAP_SERVER_SUPPORT + coap_delete_pdu_lkd(session->cached_pdu); +#endif /* COAP_SERVER_SUPPORT */ coap_log_debug("***%s: session %p: closed\n", coap_session_str(session), (void *)session);