fix(server): Don't send a copy of the PDU instead of applying the response to the original response PDU

libcoap/Cargo.toml

@@ -16,6 +16,11 @@ authors = ["Hugo Hakim Damer <[email protected]>"]
 categories = ["api-bindings", "network-programming", "embedded"]
 keywords = ["coap", "libcoap"]
 resolver = "2"
+# libcoap-rs currently does not have mechanisms for passing unwinds through the FFI layer
+# in libcoap-initiated callbacks. Therefore, we need the behavior described in
+# https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html#abort-on-uncaught-panics-in-extern-c-functions
+# to avoid soundness issues.
+rust-version = "1.81.0"
 
 [features]
 default = ["dtls-psk", "tcp", "dtls_openssl", "vendored", "libcoap-sys/default"]

libcoap/src/context.rs

@@ -13,7 +13,7 @@
 use std::ffi::CString;
 #[cfg(dtls)]
 use std::ptr::NonNull;
-use std::{any::Any, ffi::c_void, fmt::Debug, net::SocketAddr, ops::Sub, sync::Once, time::Duration};
+use std::{any::Any, ffi::c_void, fmt::Debug, net::SocketAddr, sync::Once, time::Duration};
 #[cfg(all(feature = "dtls-pki", unix))]
 use std::{os::unix::ffi::OsStrExt, path::Path};
 
@@ -25,9 +25,10 @@ use libcoap_sys::{
     coap_context_get_max_handshake_sessions, coap_context_get_max_idle_sessions, coap_context_get_session_timeout,
     coap_context_set_block_mode, coap_context_set_csm_max_message_size, coap_context_set_csm_timeout,
     coap_context_set_keepalive, coap_context_set_max_handshake_sessions, coap_context_set_max_idle_sessions,
-    coap_context_set_session_timeout, coap_context_t, coap_event_t, coap_free_context, coap_get_app_data,
-    coap_io_process, coap_new_context, coap_proto_t, coap_register_event_handler, coap_register_response_handler,
-    coap_set_app_data, coap_startup_with_feature_checks, COAP_BLOCK_SINGLE_BODY, COAP_BLOCK_USE_LIBCOAP, COAP_IO_WAIT,
+    coap_context_set_session_timeout, coap_context_t, coap_endpoint_t, coap_event_t, coap_free_context,
+    coap_get_app_data, coap_io_process, coap_new_context, coap_proto_t, coap_register_event_handler,
+    coap_register_response_handler, coap_set_app_data, coap_startup_with_feature_checks, COAP_BLOCK_SINGLE_BODY,
+    COAP_BLOCK_USE_LIBCOAP, COAP_IO_WAIT,
 };
 
 #[cfg(any(feature = "dtls-rpk", feature = "dtls-pki"))]
@@ -346,14 +347,19 @@ impl CoapContext<'_> {
     /// Performs a controlled shutdown of the CoAP context.
     ///
     /// This will perform all still outstanding IO operations until [coap_can_exit()] confirms that
-    /// the context has no more outstanding IO and can be dropped without interrupting sessions.
+    /// the context has no more outstanding IO and can be dropped.
     pub fn shutdown(mut self, exit_wait_timeout: Option<Duration>) -> Result<(), IoProcessError> {
         let mut remaining_time = exit_wait_timeout;
         // Send remaining packets until we can cleanly shutdown.
         // SAFETY: Provided context is always valid as an invariant of this struct.
-        while unsafe { coap_can_exit(self.inner.borrow_mut().raw_context) } == 0 {
+        let raw_context = self.inner.borrow_mut().raw_context;
+
+        while unsafe { coap_can_exit(raw_context) } == 0 {
             let spent_time = self.do_io(remaining_time)?;
-            remaining_time = remaining_time.map(|v| v.sub(spent_time));
+            remaining_time = remaining_time.map(|v| v.saturating_sub(spent_time));
+            if remaining_time.is_some_and(|v| v.is_zero()) {
+                break;
+            }
         }
         Ok(())
     }
@@ -652,8 +658,12 @@ impl Drop for CoapContextInner<'_> {
         for session in std::mem::take(&mut self.server_sessions).into_iter() {
             session.drop_exclusively();
         }
-        // Clear endpoints because coap_free_context() would free their underlying raw structs.
-        self.endpoints.clear();
+        // Decompose endpoint wrappers into raw values (can't drop them directly, as doing so might
+        // not work as long as server-side sessions are still active).
+        let _endpoints: Vec<*mut coap_endpoint_t> = std::mem::take(&mut self.endpoints)
+            .into_iter()
+            .map(|v| v.into_raw())
+            .collect();
         // Extract reference to CoapContextInner from raw context and drop it.
         // SAFETY: Value is set upon construction of the inner context and never deleted.
         unsafe {

libcoap/src/lib.rs

@@ -20,7 +20,7 @@
 //!     - [x] DTLS
 //!         - [x] DTLS using PSK
 //!         - [x] DTLS using PKI/RPK
-//!     - [ ] TCP
+//!     - [x] TCP
 //!     - [ ] TLS
 //!     - [ ] OSCORE
 //!     - [ ] WebSockets
@@ -175,14 +175,14 @@
 //!         //
 //!         // The provided CoapResponse is already filled with the correct token to be
 //!         // interpreted as a response to the correct request by the client.
-//!         |completed: &mut (), session: &mut CoapServerSession, request: &CoapRequest, mut response: CoapResponse| {
+//!         |completed: &mut (), session: &mut CoapServerSession, request: &CoapRequest, response: &mut CoapResponse| {
 //!             // Set content of the response message to "Hello World!"
 //!             let data = Vec::<u8>::from("Hello World!".as_bytes());
 //!             response.set_data(Some(data));
 //!             // Set the response code to 2.00 "Content"
 //!             response.set_code(CoapResponseCode::Content);
-//!             // Send the response message.
-//!             session.send(response).expect("Unable to send response");
+//!             // Note that you must not explicitly send the response here, libcoap takes care of
+//!             // that
 //!         },
 //!     )),
 //! );

libcoap/src/message/mod.rs

@@ -15,35 +15,32 @@
 //! process of creating requests and responses and setting the appropriate options ([CoapRequest]
 //! and [CoapResponse]).
 
-use std::{ffi::c_void, mem::MaybeUninit, slice::Iter};
-use std::fmt::Write;
-
-use num_traits::FromPrimitive;
+use std::{ffi::c_void, fmt::Write, mem::MaybeUninit, slice::Iter};
 
 use libcoap_sys::{
-    coap_add_data, coap_add_data_large_request, coap_add_optlist_pdu, coap_add_token, coap_delete_optlist,
-    coap_delete_pdu, coap_get_data, coap_insert_optlist, coap_new_optlist, coap_opt_length, coap_opt_t, coap_opt_value,
-    coap_option_iterator_init, coap_option_next, coap_option_num_t, coap_optlist_t, coap_pdu_get_code,
-    coap_pdu_get_mid, coap_pdu_get_token, coap_pdu_get_type, coap_pdu_init, coap_pdu_set_code, coap_pdu_set_type,
-    coap_pdu_t, coap_session_t,
+    coap_add_data, coap_add_data_large_request, coap_add_optlist_pdu, coap_add_token, coap_check_option,
+    coap_delete_optlist, coap_delete_pdu, coap_get_data, coap_insert_optlist, coap_new_optlist, coap_opt_iterator_t,
+    coap_opt_length, coap_opt_t, coap_opt_value, coap_option_iterator_init, coap_option_next, coap_option_num_t,
+    coap_optlist_t, coap_pdu_get_code, coap_pdu_get_mid, coap_pdu_get_token, coap_pdu_get_type, coap_pdu_init,
+    coap_pdu_set_code, coap_pdu_set_type, coap_pdu_t, coap_session_t,
 };
+use num_traits::FromPrimitive;
 pub use request::CoapRequest;
 pub use response::CoapResponse;
 
 use crate::{
+    context::ensure_coap_started,
     error::{MessageConversionError, OptionValueError},
     protocol::{
-        Block, CoapMatch, CoapMessageCode, CoapMessageType, CoapOptionNum, CoapOptionType, ContentFormat, ETag,
-        HopLimit, MaxAge, NoResponse, Observe, ProxyScheme, ProxyUri, Size, UriHost, UriPath, UriPort, UriQuery,
+        Block, CoapMatch, CoapMessageCode, CoapMessageType, CoapOptionNum, CoapOptionType, ContentFormat, ETag, Echo,
+        HopLimit, MaxAge, NoResponse, Observe, Oscore, ProxyScheme, ProxyUri, RequestTag, Size, UriHost, UriPath,
+        UriPort, UriQuery,
     },
     session::CoapSessionCommon,
-    types::CoapMessageId,
-};
-use crate::context::ensure_coap_started;
-use crate::protocol::{Echo, Oscore, RequestTag};
-use crate::types::{
-    decode_var_len_u16, decode_var_len_u32, decode_var_len_u8, encode_var_len_u16, encode_var_len_u32,
-    encode_var_len_u8,
+    types::{
+        decode_var_len_u16, decode_var_len_u32, decode_var_len_u8, encode_var_len_u16, encode_var_len_u32,
+        encode_var_len_u8, CoapMessageId,
+    },
 };
 
 pub mod request;
@@ -424,12 +421,11 @@ impl CoapMessage {
     ///
     /// The caller is responsible for freeing the returned PDU, either by calling [coap_send()](libcoap_sys::coap_send()) or
     /// [coap_delete_pdu()].
-    pub fn into_raw_pdu<'a, S: CoapSessionCommon<'a> + ?Sized>(
+    pub fn into_raw_pdu<'a, S: CoapSessionCommon<'a>+?Sized>(
         mut self,
         session: &S,
     ) -> Result<*mut coap_pdu_t, MessageConversionError> {
         let message = self.as_message_mut();
-
         // SAFETY: all values are valid, cannot cause UB.
         let pdu = unsafe {
             coap_pdu_init(
@@ -466,7 +462,7 @@ impl CoapMessage {
     ///
     /// # Safety
     /// raw_pdu must point to a valid mutable instance of coap_pdu_t.
-    pub(crate) unsafe fn apply_to_raw_pdu<'a, S: CoapSessionCommon<'a> + ?Sized>(
+    pub(crate) unsafe fn apply_to_raw_pdu<'a, S: CoapSessionCommon<'a>+?Sized>(
         mut self,
         raw_pdu: *mut coap_pdu_t,
         session: &S,
@@ -475,14 +471,49 @@ impl CoapMessage {
         coap_pdu_set_type(raw_pdu, self.type_.to_raw_pdu_type());
         coap_pdu_set_code(raw_pdu, self.code.to_raw_pdu_code());
         let message = self.as_message_mut();
-        let token: &[u8] = message.token.as_ref().ok_or(MessageConversionError::MissingToken)?;
-        if coap_add_token(raw_pdu, token.len(), token.as_ptr()) == 0 {
-            return Err(MessageConversionError::Unknown);
+        let cur_token = coap_pdu_get_token(raw_pdu);
+
+        let cur_token = (cur_token.length > 0).then(|| core::slice::from_raw_parts(cur_token.s, cur_token.length));
+        // Return error if no token was supplied.
+        if cur_token.is_none() && message.token.is_none() {
+            return Err(MessageConversionError::MissingToken);
         }
+        // Update token if necessary.
+        if message.token.is_some() && cur_token != message.token.as_deref() {
+            let token = message.token.as_ref().unwrap();
+            if coap_add_token(raw_pdu, token.len(), token.as_ptr()) == 0 {
+                return Err(MessageConversionError::Unknown);
+            }
+        }
+
         let mut optlist = None;
         let option_iter = std::mem::take(&mut message.options).into_iter();
         for option in option_iter {
             let optnum = option.number();
+            // TODO this is a really ugly workaround, remove this as soon as we have builder-like
+            //      interfaces for PDUs.
+            //      Do not duplicate Observe option as reapplying to the same PDU would cause the option
+            //      to be added twice otherwise.
+            if let CoapOption::Observe(option) = option {
+                // Check if Observe has already been set.
+                let mut scratch: MaybeUninit<coap_opt_iterator_t> = MaybeUninit::uninit();
+                let cur_option = unsafe { coap_check_option(raw_pdu, optnum, scratch.as_mut_ptr()) };
+                if !cur_option.is_null() {
+                    // Observe has already been set, get value and compare with the one set for this message.
+                    let cur_value = unsafe {
+                        core::slice::from_raw_parts(coap_opt_value(cur_option), coap_opt_length(cur_option) as usize)
+                    };
+                    let cur_value = decode_var_len_u32(cur_value);
+                    // If Observe option values differ, return an appropriate error.
+                    if option != cur_value {
+                        return Err(MessageConversionError::NonRepeatableOptionRepeated(
+                            CoapOptionType::Observe,
+                        ));
+                    }
+                    // Otherwise, skip adding the option value to the optlist to avoid a duplicate.
+                    continue;
+                }
+            }
             let entry = option
                 .into_optlist_entry()
                 .map_err(|e| MessageConversionError::InvalidOptionValue(CoapOptionType::try_from(optnum).ok(), e))?;

libcoap/src/message/response.rs

@@ -7,12 +7,14 @@
  * See the README as well as the LICENSE file for more information.
  */
 
-use crate::error::{MessageConversionError, MessageTypeError, OptionValueError};
-use crate::message::{CoapMessage, CoapMessageCommon, CoapOption, construct_path_string, construct_query_string};
-use crate::protocol::{
-    CoapMessageCode, CoapMessageType, CoapOptionType, CoapResponseCode, ContentFormat, Echo, ETag, MaxAge, Observe,
+use crate::{
+    error::{MessageConversionError, MessageTypeError, OptionValueError},
+    message::{construct_path_string, construct_query_string, CoapMessage, CoapMessageCommon, CoapOption},
+    protocol::{
+        CoapMessageCode, CoapMessageType, CoapOptionType, CoapResponseCode, ContentFormat, ETag, Echo, MaxAge, Observe,
+    },
+    types::CoapUri,
 };
-use crate::types::CoapUri;
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CoapResponse {