feat: In-place crypto (mozilla#2385)

* feat: In-place crypto Only in-place encryption so far, and only for the main data path. Fixes mozilla#2246 (eventually) * aead_null * WIP decrypt * More * fix(transport/packet): don't (mutably) borrow data multiple times * Fixes * Minimize * Some suggestions from @martinthomson * More * More * Fix `AeadNull` * More suggestions from @martinthomson * clippy * fixme * Update neqo-crypto/src/aead.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-crypto/src/aead.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-crypto/src/aead.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-crypto/src/aead_null.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Minimize diff * Fix --------- Signed-off-by: Lars Eggert <[email protected]> Co-authored-by: Max Leonard Inden <[email protected]> Co-authored-by: Martin Thomson <[email protected]>
larseggert · Feb 4, 2025 · 2406bfa · 2406bfa
1 parent 5cb82c7
commit 2406bfa
Show file tree

Hide file tree

Showing 18 changed files with 347 additions and 177 deletions.
diff --git a/fuzz/fuzz_targets/packet.rs b/fuzz/fuzz_targets/packet.rs
@@ -14,7 +14,7 @@ fuzz_target!(|data: &[u8]| {
     neqo_crypto::init().unwrap();
 
     // Run the fuzzer
-    _ = PublicPacket::decode(data, decoder);
+    _ = PublicPacket::decode(&mut data.to_vec(), decoder);
 });
 
 #[cfg(any(not(fuzzing), windows))]

diff --git a/neqo-bin/src/client/http09.rs b/neqo-bin/src/client/http09.rs
@@ -195,7 +195,7 @@ impl super::Client for Connection {
 
     fn process_multiple_input<'a>(
         &mut self,
-        dgrams: impl IntoIterator<Item = Datagram<&'a [u8]>>,
+        dgrams: impl IntoIterator<Item = Datagram<&'a mut [u8]>>,
         now: Instant,
     ) {
         self.process_multiple_input(dgrams, now);

diff --git a/neqo-bin/src/client/http3.rs b/neqo-bin/src/client/http3.rs
@@ -139,7 +139,7 @@ impl super::Client for Http3Client {
 
     fn process_multiple_input<'a>(
         &mut self,
-        dgrams: impl IntoIterator<Item = Datagram<&'a [u8]>>,
+        dgrams: impl IntoIterator<Item = Datagram<&'a mut [u8]>>,
         now: Instant,
     ) {
         self.process_multiple_input(dgrams, now);

diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs
@@ -384,7 +384,7 @@ trait Client {
     fn process_output(&mut self, now: Instant) -> Output;
     fn process_multiple_input<'a>(
         &mut self,
-        dgrams: impl IntoIterator<Item = Datagram<&'a [u8]>>,
+        dgrams: impl IntoIterator<Item = Datagram<&'a mut [u8]>>,
         now: Instant,
     );
     fn has_events(&self) -> bool;

diff --git a/neqo-bin/src/server/http09.rs b/neqo-bin/src/server/http09.rs
@@ -187,7 +187,7 @@ impl HttpServer {
 }
 
 impl super::HttpServer for HttpServer {
-    fn process(&mut self, dgram: Option<Datagram<&[u8]>>, now: Instant) -> Output {
+    fn process(&mut self, dgram: Option<Datagram<&mut [u8]>>, now: Instant) -> Output {
         self.server.process(dgram, now)
     }
 

diff --git a/neqo-bin/src/server/http3.rs b/neqo-bin/src/server/http3.rs
@@ -82,7 +82,7 @@ impl Display for HttpServer {
 }
 
 impl super::HttpServer for HttpServer {
-    fn process(&mut self, dgram: Option<Datagram<&[u8]>>, now: Instant) -> neqo_http3::Output {
+    fn process(&mut self, dgram: Option<Datagram<&mut [u8]>>, now: Instant) -> neqo_http3::Output {
         self.server.process(dgram, now)
     }
 

diff --git a/neqo-bin/src/server/mod.rs b/neqo-bin/src/server/mod.rs
@@ -193,7 +193,7 @@ fn qns_read_response(filename: &str) -> Result<Vec<u8>, io::Error> {
 
 #[allow(clippy::module_name_repetitions)]
 pub trait HttpServer: Display {
-    fn process(&mut self, dgram: Option<Datagram<&[u8]>>, now: Instant) -> Output;
+    fn process(&mut self, dgram: Option<Datagram<&mut [u8]>>, now: Instant) -> Output;
     fn process_events(&mut self, now: Instant);
     fn has_events(&self) -> bool;
 }
@@ -243,7 +243,7 @@ impl ServerRunner {
         timeout: &mut Option<Pin<Box<Sleep>>>,
         sockets: &mut [(SocketAddr, crate::udp::Socket)],
         now: &dyn Fn() -> Instant,
-        mut input_dgram: Option<Datagram<&[u8]>>,
+        mut input_dgram: Option<Datagram<&mut [u8]>>,
     ) -> Result<(), io::Error> {
         loop {
             match server.process(input_dgram.take(), now()) {

diff --git a/neqo-common/src/datagram.rs b/neqo-common/src/datagram.rs
@@ -4,7 +4,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{net::SocketAddr, ops::Deref};
+use std::{
+    net::SocketAddr,
+    ops::{Deref, DerefMut},
+};
 
 use crate::{hex_with_len, IpTos};
 
@@ -47,7 +50,6 @@ impl<D: AsRef<[u8]>> Datagram<D> {
     }
 }
 
-#[cfg(test)]
 impl<D: AsMut<[u8]> + AsRef<[u8]>> AsMut<[u8]> for Datagram<D> {
     fn as_mut(&mut self) -> &mut [u8] {
         self.d.as_mut()
@@ -65,6 +67,12 @@ impl Datagram<Vec<u8>> {
     }
 }
 
+impl<D: AsRef<[u8]> + AsMut<[u8]>> DerefMut for Datagram<D> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        AsMut::<[u8]>::as_mut(self)
+    }
+}
+
 impl<D: AsRef<[u8]>> Deref for Datagram<D> {
     type Target = [u8];
     #[must_use]
@@ -86,9 +94,9 @@ impl<D: AsRef<[u8]>> std::fmt::Debug for Datagram<D> {
     }
 }
 
-impl<'a> Datagram<&'a [u8]> {
+impl<'a> Datagram<&'a mut [u8]> {
     #[must_use]
-    pub const fn from_slice(src: SocketAddr, dst: SocketAddr, tos: IpTos, d: &'a [u8]) -> Self {
+    pub fn from_slice(src: SocketAddr, dst: SocketAddr, tos: IpTos, d: &'a mut [u8]) -> Self {
         Self { src, dst, tos, d }
     }
 

diff --git a/neqo-crypto/src/aead.rs b/neqo-crypto/src/aead.rs
@@ -123,14 +123,45 @@ impl RealAead {
                 c_uint::try_from(output.len())?,
             )
         }?;
-        Ok(&output[0..(l.try_into()?)])
+        Ok(&output[..l.try_into()?])
     }
 
-    /// Decrypt a ciphertext.
+    /// Encrypt `data` consisting of `aad` and plaintext `data` in place.
+    ///
+    /// The last `Aead::expansion` of `data` is overwritten by the AEAD tag by this function.
+    /// Therefore, a buffer should be provided that is that much larger than the plaintext.
+    ///
+    /// # Panics
+    ///
+    /// If `data` is shorter than `<self as Aead>::expansion()`.
+    ///
+    /// # Errors
     ///
-    /// Note that NSS insists upon having extra space available for decryption, so
-    /// the buffer for `output` should be the same length as `input`, even though
-    /// the final result will be shorter.
+    /// If the input can't be protected or any input is too large for NSS.
+    pub fn encrypt_in_place<'a>(
+        &self,
+        count: u64,
+        aad: &[u8],
+        data: &'a mut [u8],
+    ) -> Res<&'a mut [u8]> {
+        let mut l: c_uint = 0;
+        unsafe {
+            SSL_AeadEncrypt(
+                *self.ctx,
+                count,
+                aad.as_ptr(),
+                c_uint::try_from(aad.len())?,
+                data.as_ptr(),
+                c_uint::try_from(data.len() - self.expansion())?,
+                data.as_ptr(),
+                &mut l,
+                c_uint::try_from(data.len())?,
+            )
+        }?;
+        Ok(&mut data[..l.try_into()?])
+    }
+
+    /// Decrypt a ciphertext.
     ///
     /// # Errors
     ///
@@ -144,6 +175,9 @@ impl RealAead {
     ) -> Res<&'a [u8]> {
         let mut l: c_uint = 0;
         unsafe {
+            // Note that NSS insists upon having extra space available for decryption, so
+            // the buffer for `output` should be the same length as `input`, even though
+            // the final result will be shorter.
             SSL_AeadDecrypt(
                 *self.ctx,
                 count,
@@ -156,7 +190,41 @@ impl RealAead {
                 c_uint::try_from(output.len())?,
             )
         }?;
-        Ok(&output[0..(l.try_into()?)])
+        Ok(&output[..l.try_into()?])
+    }
+
+    /// Decrypt a ciphertext in place.
+    /// Returns a subslice of `data` (without the last `<self as Aead>::expansion()` bytes),
+    /// that has been decrypted in place.
+    ///
+    /// # Errors
+    ///
+    /// If the input isn't authenticated or any input is too large for NSS.
+    pub fn decrypt_in_place<'a>(
+        &self,
+        count: u64,
+        aad: &[u8],
+        data: &'a mut [u8],
+    ) -> Res<&'a mut [u8]> {
+        let mut l: c_uint = 0;
+        unsafe {
+            // Note that NSS insists upon having extra space available for decryption, so
+            // the buffer for `output` should be the same length as `input`, even though
+            // the final result will be shorter.
+            SSL_AeadDecrypt(
+                *self.ctx,
+                count,
+                aad.as_ptr(),
+                c_uint::try_from(aad.len())?,
+                data.as_ptr(),
+                c_uint::try_from(data.len())?,
+                data.as_ptr(),
+                &mut l,
+                c_uint::try_from(data.len())?,
+            )
+        }?;
+        debug_assert_eq!(usize::try_from(l)?, data.len() - self.expansion());
+        Ok(&mut data[..l.try_into()?])
     }
 }
 

diff --git a/neqo-crypto/src/aead_null.rs b/neqo-crypto/src/aead_null.rs
@@ -42,36 +42,68 @@ impl AeadNull {
     ) -> Res<&'a [u8]> {
         let l = input.len();
         output[..l].copy_from_slice(input);
-        output[l..l + 16].copy_from_slice(AEAD_NULL_TAG);
-        Ok(&output[..l + 16])
+        output[l..l + self.expansion()].copy_from_slice(AEAD_NULL_TAG);
+        Ok(&output[..l + self.expansion()])
     }
 
     #[allow(clippy::missing_errors_doc)]
-    pub fn decrypt<'a>(
+    pub fn encrypt_in_place<'a>(
         &self,
         _count: u64,
         _aad: &[u8],
-        input: &[u8],
-        output: &'a mut [u8],
-    ) -> Res<&'a [u8]> {
-        if input.len() < AEAD_NULL_TAG.len() {
+        data: &'a mut [u8],
+    ) -> Res<&'a mut [u8]> {
+        let pos = data.len() - self.expansion();
+        data[pos..].copy_from_slice(AEAD_NULL_TAG);
+        Ok(data)
+    }
+
+    fn decrypt_check(&self, _count: u64, _aad: &[u8], input: &[u8]) -> Res<usize> {
+        if input.len() < self.expansion() {
             return Err(Error::from(SEC_ERROR_BAD_DATA));
         }
 
-        let len_encrypted = input.len() - AEAD_NULL_TAG.len();
+        let len_encrypted = input
+            .len()
+            .checked_sub(self.expansion())
+            .ok_or_else(|| Error::from(SEC_ERROR_BAD_DATA))?;
         // Check that:
         // 1) expansion is all zeros and
         // 2) if the encrypted data is also supplied that at least some values are no zero
         //    (otherwise padding will be interpreted as a valid packet)
         if &input[len_encrypted..] == AEAD_NULL_TAG
             && (len_encrypted == 0 || input[..len_encrypted].iter().any(|x| *x != 0x0))
         {
-            output[..len_encrypted].copy_from_slice(&input[..len_encrypted]);
-            Ok(&output[..len_encrypted])
+            Ok(len_encrypted)
         } else {
             Err(Error::from(SEC_ERROR_BAD_DATA))
         }
     }
+
+    #[allow(clippy::missing_errors_doc)]
+    pub fn decrypt<'a>(
+        &self,
+        count: u64,
+        aad: &[u8],
+        input: &[u8],
+        output: &'a mut [u8],
+    ) -> Res<&'a [u8]> {
+        self.decrypt_check(count, aad, input).map(|len| {
+            output[..len].copy_from_slice(&input[..len]);
+            &output[..len]
+        })
+    }
+
+    #[allow(clippy::missing_errors_doc)]
+    pub fn decrypt_in_place<'a>(
+        &self,
+        count: u64,
+        aad: &[u8],
+        data: &'a mut [u8],
+    ) -> Res<&'a mut [u8]> {
+        self.decrypt_check(count, aad, data)
+            .map(move |len| &mut data[..len])
+    }
 }
 
 impl fmt::Debug for AeadNull {

diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs
@@ -842,7 +842,11 @@ impl Http3Client {
     }
 
     /// This function combines  `process_input` and `process_output` function.
-    pub fn process(&mut self, dgram: Option<Datagram<impl AsRef<[u8]>>>, now: Instant) -> Output {
+    pub fn process(
+        &mut self,
+        dgram: Option<Datagram<impl AsRef<[u8]> + AsMut<[u8]>>>,
+        now: Instant,
+    ) -> Output {
         qtrace!("[{self}] Process");
         if let Some(d) = dgram {
             self.process_input(d, now);
@@ -860,13 +864,13 @@ impl Http3Client {
     /// packets need to be sent or if a timer needs to be updated.
     ///
     /// [1]: ../neqo_transport/enum.ConnectionEvent.html
-    pub fn process_input(&mut self, dgram: Datagram<impl AsRef<[u8]>>, now: Instant) {
+    pub fn process_input(&mut self, dgram: Datagram<impl AsRef<[u8]> + AsMut<[u8]>>, now: Instant) {
         self.process_multiple_input(iter::once(dgram), now);
     }
 
     pub fn process_multiple_input(
         &mut self,
-        dgrams: impl IntoIterator<Item = Datagram<impl AsRef<[u8]>>>,
+        dgrams: impl IntoIterator<Item = Datagram<impl AsRef<[u8]> + AsMut<[u8]>>>,
         now: Instant,
     ) {
         let mut dgrams = dgrams.into_iter().peekable();

diff --git a/neqo-http3/src/server.rs b/neqo-http3/src/server.rs
@@ -117,7 +117,11 @@ impl Http3Server {
         self.process(None::<Datagram>, now)
     }
 
-    pub fn process(&mut self, dgram: Option<Datagram<impl AsRef<[u8]>>>, now: Instant) -> Output {
+    pub fn process(
+        &mut self,
+        dgram: Option<Datagram<impl AsRef<[u8]> + AsMut<[u8]>>>,
+        now: Instant,
+    ) -> Output {
         qtrace!("[{self}] Process");
         let out = self.server.process(dgram, now);
         self.process_http3(now);