From e80cccc3bb8bcee0f31bea4ccc8630c63e15a8a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 21 Apr 2023 15:08:34 +0200 Subject: [PATCH 1/3] Make max fragment length optional and send --- src/config.rs | 35 ++++++++++++++++++++++++++++++++--- src/handshake/client_hello.rs | 4 ++++ src/max_fragment_length.rs | 5 +++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/config.rs b/src/config.rs index 75fe191e..b597ff2a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,7 +1,7 @@ use crate::cipher_suites::CipherSuite; use crate::handshake::certificate::CertificateRef; use crate::handshake::certificate_verify::CertificateVerify; -use crate::max_fragment_length::MaxFragmentLength; +pub use crate::max_fragment_length::MaxFragmentLength; use crate::named_groups::NamedGroup; use crate::signature_schemes::SignatureScheme; use crate::TlsError; @@ -130,7 +130,7 @@ where pub(crate) cipher_suite: PhantomData, pub(crate) signature_schemes: Vec, pub(crate) named_groups: Vec, - pub(crate) max_fragment_length: MaxFragmentLength, + pub(crate) max_fragment_length: Option, pub(crate) ca: Option>, pub(crate) cert: Option>, } @@ -178,7 +178,7 @@ where cipher_suite: PhantomData, signature_schemes: Vec::new(), named_groups: Vec::new(), - max_fragment_length: MaxFragmentLength::Bits10, + max_fragment_length: None, psk: None, server_name: None, ca: None, @@ -237,6 +237,35 @@ where self } + /// Configures the maximum plaintext fragment size. + /// + /// This option may help reduce memory size, as smaller fragment lengths require smaller + /// read/write buffers. Note that embedded-tls does not currently use this option to fragment + /// writes. Note that the buffers need to include some overhead over the configured fragment + /// length. + /// + /// From [RFC 6066, Section 4. Maximum Fragment Length Negotiation](https://www.rfc-editor.org/rfc/rfc6066#page-8): + /// + /// > Without this extension, TLS specifies a fixed maximum plaintext + /// > fragment length of 2^14 bytes. It may be desirable for constrained + /// > clients to negotiate a smaller maximum fragment length due to memory + /// > limitations or bandwidth limitations. + /// + /// > For example, if the negotiated length is 2^9=512, then, when using currently defined + /// > cipher suites ([...]) and null compression, the record-layer output can be at most + /// > 805 bytes: 5 bytes of headers, 512 bytes of application data, 256 bytes of padding, + /// > and 32 bytes of MAC. + pub fn with_max_fragment_length(mut self, max_fragment_length: MaxFragmentLength) -> Self { + self.max_fragment_length = Some(max_fragment_length); + self + } + + /// Resets the max fragment length to 14 bits (16384). + pub fn reset_max_fragment_length(mut self) -> Self { + self.max_fragment_length = None; + self + } + pub fn with_ca(mut self, ca: Certificate<'a>) -> Self { self.ca = Some(ca); self diff --git a/src/handshake/client_hello.rs b/src/handshake/client_hello.rs index 2e3385b9..f09de7d2 100644 --- a/src/handshake/client_hello.rs +++ b/src/handshake/client_hello.rs @@ -80,6 +80,10 @@ where } .encode(buf)?; + if let Some(max_fragment_length) = self.config.max_fragment_length { + ClientExtension::MaxFragmentLength(max_fragment_length).encode(buf)?; + } + ClientExtension::SupportedGroups { supported_groups: self.config.named_groups.clone(), } diff --git a/src/max_fragment_length.rs b/src/max_fragment_length.rs index 40a03756..019204db 100644 --- a/src/max_fragment_length.rs +++ b/src/max_fragment_length.rs @@ -1,8 +1,13 @@ +/// Maximum plaintext fragment length #[derive(Debug, Copy, Clone)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum MaxFragmentLength { + /// 512 bytes Bits9 = 1, + /// 1024 bytes Bits10 = 2, + /// 2048 bytes Bits11 = 3, + /// 4096 bytes Bits12 = 4, } From 5970e142319bfc9631bf3e2a2440de47cb29b3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 22 Apr 2023 02:23:35 +0200 Subject: [PATCH 2/3] Replace loop-brake with while --- src/asynch.rs | 7 ++----- src/blocking.rs | 7 ++----- src/connection.rs | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/asynch.rs b/src/asynch.rs index bc682919..21a55ab5 100644 --- a/src/asynch.rs +++ b/src/asynch.rs @@ -86,7 +86,7 @@ where Handshake::new(Verifier::new(context.config.server_name)); let mut state = State::ClientHello; - loop { + while state != State::ApplicationData { let next_state = state .process( &mut self.delegate, @@ -100,11 +100,8 @@ where .await?; trace!("State {:?} -> {:?}", state, next_state); state = next_state; - if let State::ApplicationData = state { - self.opened = true; - break; - } } + self.opened = true; Ok(()) } diff --git a/src/blocking.rs b/src/blocking.rs index be00c838..7b0c780a 100644 --- a/src/blocking.rs +++ b/src/blocking.rs @@ -86,7 +86,7 @@ where Handshake::new(Verifier::new(context.config.server_name)); let mut state = State::ClientHello; - loop { + while state != State::ApplicationData { let next_state = state.process_blocking( &mut self.delegate, &mut handshake, @@ -98,11 +98,8 @@ where )?; trace!("State {:?} -> {:?}", state, next_state); state = next_state; - if let State::ApplicationData = state { - self.opened = true; - break; - } } + self.opened = true; Ok(()) } diff --git a/src/connection.rs b/src/connection.rs index fb0ddb6f..230cf665 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -196,7 +196,7 @@ where } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum State { ClientHello, From fb9409e226e64c365bac0ffaf648397386781ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 22 Apr 2023 00:12:44 +0200 Subject: [PATCH 3/3] Make handshake logs more useful --- src/connection.rs | 19 +++---------------- src/handshake/mod.rs | 2 +- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index 230cf665..89009753 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -59,15 +59,11 @@ where let server_key = key_schedule.get_key()?; let nonce = key_schedule.get_nonce()?; - // info!("decrypting {:x?} with {}", &header, app_data.len()); - //let crypto = Aes128Gcm::new(&self.key_schedule.get_server_key()); let crypto = ::new(&server_key); - // let nonce = &key_schedule.get_server_nonce(); - // info!("server write nonce {:x?}", nonce); crypto .decrypt_in_place(&nonce, header.data(), &mut app_data) .map_err(|_| TlsError::CryptoError)?; - // info!("decrypted with padding {:x?}", app_data.as_slice()); + let padding = app_data .as_slice() .iter() @@ -76,12 +72,11 @@ where if let Some((index, _)) = padding { app_data.truncate(index + 1); }; - //trace!("decrypted {:x?}", data); let content_type = ContentType::of(*app_data.as_slice().last().unwrap()).ok_or(TlsError::InvalidRecord)?; - trace!("Decrypting content type = {:?}", content_type); + trace!("Decrypting: content type = {:?}", content_type); // Remove the content type app_data.truncate(app_data.len() - 1); @@ -98,23 +93,18 @@ where let handshake_length = remaining - buf.remaining(); if let ServerHandshake::Finished(ref mut finished) = inner { - // trace!("Server finished hash: {:x?}", finished.hash); finished .hash .replace(key_schedule.transcript_hash().clone().finalize()); } - //info!("===> inner ==> {:?}", inner); - //if hash_later { key_schedule .transcript_hash() .update(&data[offset..offset + handshake_length]); offset += handshake_length; - // info!("hash {:02x?}", &data[..data.len()]); cb(key_schedule, ServerRecord::Handshake(inner))?; } - //} } ContentType::ApplicationData => { let inner = ApplicationData::new(app_data, header); @@ -127,10 +117,9 @@ where } _ => return Err(TlsError::Unimplemented), } - //debug!("decrypted {:?} --> {:x?}", content_type, data); key_schedule.increment_counter(); } else { - debug!("Not decrypting: Not encapsulated in app data"); + trace!("Not decrypting: content_type = {:?}", record.content_type()); cb(key_schedule, record)?; } Ok(()) @@ -481,10 +470,8 @@ where { let mut state = State::ServerVerify; decrypt_record(key_schedule.read_state(), record, |key_schedule, record| { - trace!("record = {:?}", record.content_type()); match record { ServerRecord::Handshake(server_handshake) => { - trace!("handshake = {:?}", server_handshake.handshake_type()); match server_handshake { ServerHandshake::EncryptedExtensions(_) => {} ServerHandshake::Certificate(certificate) => { diff --git a/src/handshake/mod.rs b/src/handshake/mod.rs index 9e317efc..ad4772c1 100644 --- a/src/handshake/mod.rs +++ b/src/handshake/mod.rs @@ -200,7 +200,7 @@ impl<'a, N: ArrayLength> ServerHandshake<'a, N> { HandshakeType::of(buf.read_u8().map_err(|_| TlsError::InvalidHandshake)?) .ok_or(TlsError::InvalidHandshake)?; - // info!("Handshake type {:?}", handshake_type); + trace!("handshake = {:?}", handshake_type); let content_len = buf.read_u24().map_err(|_| TlsError::InvalidHandshake)?;