Fix recommended read buffer size

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- Corrected recommended read buffer size (16640 bytes instead of 16384)
+
+## 0.15.0 - 2023-08-26
+
 - Updated p256 dependency from 0.11 to 0.13.2 (#124)
 - Fix reading buffered data in multiple steps (#121, #122)
 - Fix error in NewSessionTicket message handling (#120)

src/asynch.rs

@@ -43,12 +43,12 @@ where
     /// Create a new TLS connection with the provided context and a async I/O implementation
     ///
     /// NOTE: The record read buffer should be sized to fit an encrypted TLS record. The size of this record
-    /// depends on the server configuration, but the maximum allowed value for a TLS record is 16 kB, which
-    /// should be a safe value to use.
+    /// depends on the server configuration, but the maximum allowed value for a TLS record is 16640 bytes,
+    /// which should be a safe value to use.
     ///
-    /// The write record buffer can be smaller than the read buffer. During write [`TLS_RECORD_OVERHEAD`] over overhead
-    /// is added per record, so the buffer must at least be this large. Large writes are split into multiple records if
-    /// depending on the size of the write buffer.
+    /// The write record buffer can be smaller than the read buffer. During writes [`TLS_RECORD_OVERHEAD`] bytes of
+    /// overhead is added per record, so the buffer must at least be this large. Large writes are split into multiple
+    /// records if depending on the size of the write buffer.
     /// The largest of the two buffers will be used to encode the TLS handshake record, hence either of the
     /// buffers must at least be large enough to encode a handshake.
     pub fn new(

src/blocking.rs

@@ -42,12 +42,12 @@ where
     /// Create a new TLS connection with the provided context and a blocking I/O implementation
     ///
     /// NOTE: The record read buffer should be sized to fit an encrypted TLS record. The size of this record
-    /// depends on the server configuration, but the maximum allowed value for a TLS record is 16 kB, which
-    /// should be a safe value to use.
+    /// depends on the server configuration, but the maximum allowed value for a TLS record is 16640 bytes,
+    /// which should be a safe value to use.
     ///
-    /// The write record buffer can be smaller than the read buffer. During write [`TLS_RECORD_OVERHEAD`] over overhead
-    /// is added per record, so the buffer must at least be this large. Large writes are split into multiple records if
-    /// depending on the size of the write buffer.
+    /// The write record buffer can be smaller than the read buffer. During writes [`TLS_RECORD_OVERHEAD`] bytes of
+    /// overhead is added per record, so the buffer must at least be this large. Large writes are split into multiple
+    /// records if depending on the size of the write buffer.
     /// The largest of the two buffers will be used to encode the TLS handshake record, hence either of the
     /// buffers must at least be large enough to encode a handshake.
     pub fn new(

src/buffer.rs

@@ -39,6 +39,7 @@ impl<'b> CryptoBuffer<'b> {
             self.len += 1;
             Ok(())
         } else {
+            error!("Failed to push byte");
             Err(TlsError::InsufficientSpace)
         }
     }
@@ -63,6 +64,10 @@ impl<'b> CryptoBuffer<'b> {
             self.buf[self.offset + idx] = val;
             Ok(())
         } else {
+            error!(
+                "Failed to set byte: index {} is out of range for {} elements",
+                idx, self.len
+            );
             Err(TlsError::InsufficientSpace)
         }
     }
@@ -92,6 +97,11 @@ impl<'b> CryptoBuffer<'b> {
 
     fn extend_internal(&mut self, other: &[u8]) -> Result<(), TlsError> {
         if self.space() < other.len() {
+            error!(
+                "Failed to extend buffer. Space: {} required: {}",
+                self.space(),
+                other.len()
+            );
             Err(TlsError::InsufficientSpace)
         } else {
             let start = self.offset + self.len;

src/config.rs

@@ -17,7 +17,7 @@ use typenum::{Sum, U10, U12, U16, U32};
 
 pub use crate::extensions::extension_data::max_fragment_length::MaxFragmentLength;
 
-const TLS_RECORD_MAX: usize = 16384;
+pub(crate) const TLS_RECORD_MAX: usize = 16384;
 pub const TLS_RECORD_OVERHEAD: usize = 128;
 
 // longest label is 12b -> buf <= 2 + 1 + 6 + longest + 1 + hash_out = hash_out + 22

src/handshake/certificate.rs

@@ -23,9 +23,10 @@ impl<'a> CertificateRef<'a> {
     }
 
     pub fn add(&mut self, entry: CertificateEntryRef<'a>) -> Result<(), TlsError> {
-        self.entries
-            .push(entry)
-            .map_err(|_| TlsError::InsufficientSpace)
+        self.entries.push(entry).map_err(|_| {
+            error!("CertificateRef: InsufficientSpace");
+            TlsError::InsufficientSpace
+        })
     }
 
     pub fn parse(buf: &mut ParseBuffer<'a>) -> Result<Self, TlsError> {

src/handshake/certificate_request.rs

@@ -39,7 +39,10 @@ impl<'a> TryFrom<CertificateRequestRef<'a>> for CertificateRequest {
         let mut request_context = Vec::new();
         request_context
             .extend_from_slice(cert.request_context)
-            .map_err(|_| TlsError::InsufficientSpace)?;
+            .map_err(|_| {
+                error!("CertificateRequest: InsufficientSpace");
+                TlsError::InsufficientSpace
+            })?;
         Ok(Self { request_context })
     }
 }

src/handshake/server_hello.rs

@@ -48,10 +48,10 @@ impl<'a> ServerHello<'a> {
 
         let extensions = ServerHelloExtension::parse_vector(buf)?;
 
-        // info!("server random {:x?}", random);
-        // info!("server session-id {:x?}", session_id.as_slice());
-        // info!("server cipher_suite {:x?}", cipher_suite);
-        // info!("server extensions {:?}", extensions);
+        // debug!("server random {:x}", random);
+        // debug!("server session-id {:x}", session_id.as_slice());
+        debug!("server cipher_suite {:?}", cipher_suite);
+        debug!("server extensions {:?}", extensions);
 
         Ok(Self {
             random,

src/lib.rs

@@ -129,7 +129,10 @@ impl embedded_io::Error for TlsError {
     fn kind(&self) -> embedded_io::ErrorKind {
         match self {
             Self::Io(k) => *k,
-            _ => embedded_io::ErrorKind::Other,
+            _ => {
+                error!("TLS error: {:?}", self);
+                embedded_io::ErrorKind::Other
+            }
         }
     }
 }

src/parse_buffer.rs

@@ -124,11 +124,22 @@ impl<'b> ParseBuffer<'b> {
         dest: &mut Vec<u8, N>,
         num_bytes: usize,
     ) -> Result<(), ParseError> {
-        if (dest.capacity() - dest.len()) < num_bytes {
+        let space = dest.capacity() - dest.len();
+        if space < num_bytes {
+            error!(
+                "Insufficient space in destination buffer. Space: {} required: {}",
+                space, num_bytes
+            );
             Err(ParseError::InsufficientSpace)
         } else if self.pos + num_bytes <= self.buffer.len() {
             dest.extend_from_slice(&self.buffer[self.pos..self.pos + num_bytes])
-                .map_err(|_| ParseError::InsufficientSpace)?;
+                .map_err(|_| {
+                    error!(
+                        "Failed to extend destination buffer. Space: {} required: {}",
+                        space, num_bytes
+                    );
+                    ParseError::InsufficientSpace
+                })?;
             self.pos += num_bytes;
             Ok(())
         } else {
@@ -145,9 +156,10 @@ impl<'b> ParseBuffer<'b> {
 
         let mut data = self.slice(data_length)?;
         while !data.is_empty() {
-            result
-                .push(read(&mut data)?)
-                .map_err(|_| ParseError::InsufficientSpace)?;
+            result.push(read(&mut data)?).map_err(|_| {
+                error!("Failed to store parse result");
+                ParseError::InsufficientSpace
+            })?;
         }
 
         Ok(result)

src/record.rs

@@ -174,11 +174,11 @@ pub struct RecordHeader {
 impl RecordHeader {
     pub fn content_type(&self) -> ContentType {
         // Content type already validated in read
-        ContentType::of(self.header[0]).unwrap()
+        unwrap!(ContentType::of(self.header[0]))
     }
 
     pub fn content_length(&self) -> usize {
-        // Content lenth already validated in read
+        // Content length already validated in read
         u16::from_be_bytes([self.header[3], self.header[4]]) as usize
     }
 

src/record_reader.rs

@@ -29,6 +29,9 @@ where
     CipherSuite: TlsCipherSuite,
 {
     pub fn new(buf: &'a mut [u8]) -> Self {
+        if buf.len() < 16640 {
+            warn!("Read buffer is smaller than 16640 bytes, which may cause problems!");
+        }
         Self {
             buf,
             decoded: 0,
@@ -47,6 +50,11 @@ where
         let header = RecordHeader::decode(header.try_into().unwrap())?;
 
         let content_length = header.content_length();
+        debug!(
+            "advance: {:?} - content_length = {} bytes",
+            header.content_type(),
+            content_length
+        );
         let data = self.advance(transport, content_length).await?;
         ServerRecord::decode(header, data, key_schedule.transcript_hash())
     }
@@ -115,6 +123,11 @@ where
     fn ensure_contiguous(&mut self, len: usize) -> Result<(), TlsError> {
         if self.decoded + len > self.buf.len() {
             if len > self.buf.len() {
+                error!(
+                    "Record too large for buffer. Size: {} Buffer size: {}",
+                    len,
+                    self.buf.len()
+                );
                 return Err(TlsError::InsufficientSpace);
             }
             self.buf