refactor(tls-harness): use single test pair IO to allow for decryption (#5648)

Author: jmayclin

bindings/rust/standard/Cargo.toml

@@ -5,3 +5,10 @@ members = [
     "s2n-tls-hyper",
     "tls-harness",
 ]
+
+# brass-aphid-wire has a dependency on s2n-tls. This patch is necessary to unify
+# the crates.io dependency that brass-aphid-wire declares with the path-based 
+# dependency that the integration tests have
+[patch.crates-io]
+s2n-tls-sys = { path = "../extended/s2n-tls-sys" }
+s2n-tls = { path = "../extended/s2n-tls" }

bindings/rust/standard/tls-harness/Cargo.toml

@@ -15,6 +15,9 @@ openssl = { version = "0.10.73", features = ["vendored"] }
 openssl-sys = "0.9.109"
 byteorder = "1.5.0"
 
+brass-aphid-wire-decryption = "0.0.1"
+brass-aphid-wire-messages = "0.0.1"
+
 [dev-dependencies]
 # env_logger and log are used to enable logging for rustls, which can help with
 # debugging interop failures

bindings/rust/standard/tls-harness/src/cohort/openssl.rs

@@ -13,6 +13,7 @@ use openssl::ssl::{
 use std::{
     error::Error,
     io::{Read, Write},
+    rc::Rc,
     sync::{Arc, Mutex},
 };
 
@@ -46,7 +47,7 @@ impl TlsConnection for OpenSslConnection {
     fn new_from_config(
         mode: harness::Mode,
         config: &Self::Config,
-        io: &harness::TestPairIO,
+        io: &Rc<harness::TestPairIO>,
     ) -> Result<Self, Box<dyn Error>> {
         // check if there is a session ticket available
         // a session ticket will only be available if the Config was created

bindings/rust/standard/tls-harness/src/cohort/rustls.rs

@@ -16,6 +16,7 @@ use rustls::{
 use std::{
     error::Error,
     io::{BufReader, Read, Write},
+    rc::Rc,
     sync::Arc,
 };
 
@@ -134,7 +135,7 @@ impl TlsConnection for RustlsConnection {
     fn new_from_config(
         mode: harness::Mode,
         config: &Self::Config,
-        io: &harness::TestPairIO,
+        io: &Rc<harness::TestPairIO>,
     ) -> Result<Self, Box<dyn Error>> {
         let connection = match config {
             RustlsConfig::Client(config) => Connection::Client(ClientConnection::new(

bindings/rust/standard/tls-harness/src/cohort/s2n_tls.rs

@@ -18,6 +18,7 @@ use std::{
     io::ErrorKind,
     os::raw::c_int,
     pin::Pin,
+    rc::Rc,
     sync::{Arc, Mutex},
     task::Poll,
 };
@@ -137,7 +138,7 @@ impl TlsConnection for S2NConnection {
     fn new_from_config(
         mode: harness::Mode,
         config: &Self::Config,
-        io: &harness::TestPairIO,
+        io: &Rc<harness::TestPairIO>,
     ) -> Result<Self, Box<dyn Error>> {
         let s2n_mode = match mode {
             Mode::Client => s2n_tls::enums::Mode::Client,

bindings/rust/standard/tls-harness/src/harness/io.rs

@@ -9,51 +9,66 @@ use std::{
     sync::atomic::{AtomicBool, Ordering},
 };
 
+use brass_aphid_wire_decryption::decryption::stream_decrypter::StreamDecrypter;
 use byteorder::{BigEndian, ReadBytesExt};
 
+use crate::Mode;
+
 pub type LocalDataBuffer = RefCell<VecDeque<u8>>;
 
 #[derive(Debug, Default)]
 pub struct TestPairIO {
     /// a data buffer that the server writes to and the client reads from
-    pub server_tx_stream: Rc<LocalDataBuffer>,
+    pub server_tx_stream: LocalDataBuffer,
     /// a data buffer that the client writes to and the server reads from
-    pub client_tx_stream: Rc<LocalDataBuffer>,
-
-    pub recording: Rc<AtomicBool>,
-    pub client_tx_transcript: Rc<RefCell<Vec<u8>>>,
-    pub server_tx_transcript: Rc<RefCell<Vec<u8>>>,
+    pub client_tx_stream: LocalDataBuffer,
+
+    /// indicates whether all client/server writes should be stored to the
+    /// transcript fields
+    pub recording: AtomicBool,
+    pub client_tx_transcript: RefCell<Vec<u8>>,
+    pub server_tx_transcript: RefCell<Vec<u8>>,
+    /// [`Self::enable_decryption`] will initialize the stream decrypter, which
+    /// allows tests to make assertions on the decrypted TLS transcript.
+    ///
+    /// This is especially useful for TLS 1.3 where much of the handshake is encrypted.
+    pub decrypter: RefCell<Option<StreamDecrypter>>,
 }
 
 impl TestPairIO {
-    pub fn client_view(&self) -> ViewIO {
+    pub fn client_view(self: &Rc<Self>) -> ViewIO {
         ViewIO {
-            send_ctx: self.client_tx_stream.clone(),
-            recv_ctx: self.server_tx_stream.clone(),
-            recording: self.recording.clone(),
-            send_transcript: self.client_tx_transcript.clone(),
+            identity: Mode::Client,
+            io: Rc::clone(self),
         }
     }
 
-    pub fn server_view(&self) -> ViewIO {
+    pub fn server_view(self: &Rc<Self>) -> ViewIO {
         ViewIO {
-            send_ctx: self.server_tx_stream.clone(),
-            recv_ctx: self.client_tx_stream.clone(),
-            recording: self.recording.clone(),
-            send_transcript: self.server_tx_transcript.clone(),
+            identity: Mode::Server,
+            io: Rc::clone(self),
         }
     }
 
-    pub fn enable_recording(&mut self) {
+    pub fn enable_recording(&self) {
         self.recording.store(true, Ordering::Relaxed);
     }
 
+    /// Note: this is only available for TLS 1.3
+    pub fn enable_decryption(
+        &self,
+        keys: brass_aphid_wire_decryption::decryption::key_manager::KeyManager,
+    ) {
+        let stream_decrypter = StreamDecrypter::new(keys);
+        *self.decrypter.borrow_mut() = Some(stream_decrypter);
+    }
+
     pub fn client_record_sizes(&self) -> Vec<u16> {
-        Self::record_sizes(self.client_tx_transcript.as_ref().borrow().as_slice()).unwrap()
+        Self::record_sizes(self.client_tx_transcript.borrow().as_slice()).unwrap()
     }
 
     pub fn server_record_sizes(&self) -> Vec<u16> {
-        Self::record_sizes(self.server_tx_transcript.as_ref().borrow().as_slice()).unwrap()
+        Self::record_sizes(self.server_tx_transcript.borrow().as_slice()).unwrap()
     }
 
     /// Return a list of the record sizes contained in `buffer`.
@@ -80,20 +95,37 @@ impl TestPairIO {
 ///
 /// This view is client/server specific, and notably implements the read and write
 /// traits.
-///
-// This struct is used by Openssl and Rustls which both rely on a "stream" abstraction
-// which implements read and write. This is not used by s2n-tls, which relies on
-// lower level callbacks.
 pub struct ViewIO {
-    pub send_ctx: Rc<LocalDataBuffer>,
-    pub recv_ctx: Rc<LocalDataBuffer>,
-    pub recording: Rc<AtomicBool>,
-    pub send_transcript: Rc<RefCell<Vec<u8>>>,
+    pub identity: Mode,
+    pub io: Rc<TestPairIO>,
+}
+
+impl ViewIO {
+    fn recv_ctx(&self) -> &LocalDataBuffer {
+        match self.identity {
+            Mode::Client => &self.io.server_tx_stream,
+            Mode::Server => &self.io.client_tx_stream,
+        }
+    }
+
+    fn send_ctx(&self) -> &LocalDataBuffer {
+        match self.identity {
+            Mode::Client => &self.io.client_tx_stream,
+            Mode::Server => &self.io.server_tx_stream,
+        }
+    }
+
+    fn send_transcript(&self) -> &RefCell<Vec<u8>> {
+        match self.identity {
+            Mode::Client => &self.io.client_tx_transcript,
+            Mode::Server => &self.io.server_tx_transcript,
+        }
+    }
 }
 
 impl std::io::Read for ViewIO {
     fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
-        let res = self.recv_ctx.borrow_mut().read(buf);
+        let res = self.recv_ctx().borrow_mut().read(buf);
         if let Ok(0) = res {
             // We are "faking" a TcpStream, where a read of length 0 indicates
             // EoF. That is incorrect for this scenario. Instead we return WouldBlock
@@ -107,15 +139,29 @@ impl std::io::Read for ViewIO {
 
 impl std::io::Write for ViewIO {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
-        let write_result = self.send_ctx.borrow_mut().write(buf);
-
-        if self.recording.load(Ordering::Relaxed) {
-            if let Ok(written) = write_result {
-                self.send_transcript
+        let write_result = self.send_ctx().borrow_mut().write(buf);
+
+        // if we successfully wrote data, we need to record it in the various test
+        // utilities.
+        if let Ok(written) = write_result {
+            // recorder
+            if self.io.recording.load(Ordering::Relaxed) {
+                self.send_transcript()
                     .borrow_mut()
                     .write_all(&buf[0..written])
                     .unwrap();
             }
+
+            // decrypter
+            let mut decrypter = self.io.decrypter.borrow_mut();
+            if let Some(decrypter) = decrypter.as_mut() {
+                let wire_mode = match self.identity {
+                    Mode::Client => brass_aphid_wire_decryption::decryption::Mode::Client,
+                    Mode::Server => brass_aphid_wire_decryption::decryption::Mode::Server,
+                };
+                decrypter.record_tx(&buf[0..written], wire_mode);
+                decrypter.decrypt_records(wire_mode).unwrap();
+            }
         }
 
         write_result

bindings/rust/standard/tls-harness/src/harness/mod.rs

@@ -3,7 +3,7 @@
 mod io;
 pub use io::{LocalDataBuffer, TestPairIO, ViewIO};
 
-use std::{error::Error, fmt::Debug, fs::read_to_string};
+use std::{error::Error, fmt::Debug, fs::read_to_string, rc::Rc};
 use strum::EnumIter;
 
 #[derive(Clone, Copy, EnumIter)]
@@ -86,7 +86,7 @@ pub trait TlsConnection: Sized {
     fn new_from_config(
         mode: Mode,
         config: &Self::Config,
-        io: &TestPairIO,
+        io: &Rc<TestPairIO>,
     ) -> Result<Self, Box<dyn Error>>;
 
     /// Run one handshake step: receive msgs from other connection, process, and send new msgs
@@ -159,7 +159,7 @@ pub trait TlsConfigBuilder {
 pub struct TlsConnPair<C, S> {
     pub client: C,
     pub server: S,
-    pub io: TestPairIO,
+    pub io: Rc<TestPairIO>,
 }
 
 impl<C, S> TlsConnPair<C, S>
@@ -168,7 +168,7 @@ where
     S: TlsConnection,
 {
     pub fn from_configs(client_config: &C::Config, server_config: &S::Config) -> Self {
-        let io = TestPairIO::default();
+        let io = Rc::new(TestPairIO::default());
         let client = C::new_from_config(Mode::Client, client_config, &io).unwrap();
         let server = S::new_from_config(Mode::Server, server_config, &io).unwrap();
         Self { client, server, io }
@@ -199,7 +199,6 @@ where
             self.client.handshake()?;
             self.server.handshake()?;
         }
-
         Ok(())
     }
 

bindings/rust/standard/tls-harness/src/test_utilities.rs

@@ -36,3 +36,60 @@ where
     conn_pair.round_trip_transfer(&mut data).unwrap();
     conn_pair.shutdown().unwrap();
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use brass_aphid_wire_decryption::decryption::key_manager::KeyManager;
+    use brass_aphid_wire_messages::iana;
+    use openssl::ssl::SslContextBuilder;
+
+    use crate::{
+        cohort::{OpenSslConnection, S2NConnection},
+        TlsConnPair,
+    };
+
+    /// make sure that the brass-aphid-wire integration is able to correctly decrypt
+    /// TLS 1.3 conversations
+    #[test]
+    fn tls13_decryption() {
+        let key_manager = KeyManager::new();
+        let mut pair: TlsConnPair<OpenSslConnection, S2NConnection> = {
+            let mut configs =
+                TlsConfigBuilderPair::<SslContextBuilder, s2n_tls::config::Builder>::default();
+            configs
+                .server
+                .set_security_policy(&s2n_tls::security::DEFAULT)
+                .unwrap();
+            key_manager.enable_s2n_logging(&mut configs.server);
+            configs.client.set_groups_list("x448:secp256r1").unwrap();
+            configs.connection_pair()
+        };
+        pair.io.enable_recording();
+        pair.io.enable_decryption(key_manager.clone());
+
+        pair.handshake().unwrap();
+        pair.shutdown().unwrap();
+
+        let transcript = pair.io.decrypter.borrow();
+        let transcript = transcript.as_ref().unwrap().transcript();
+        let ch = transcript.client_hellos().first().unwrap().clone();
+        let client_key_shares = ch.key_share().unwrap();
+        let client_supported_groups = ch.supported_groups().unwrap();
+
+        // openssl sends the most preferred group as a key share
+        assert_eq!(client_key_shares, vec![iana::constants::x448]);
+        assert_eq!(
+            client_supported_groups,
+            vec![iana::constants::x448, iana::constants::secp256r1]
+        );
+
+        // s2n-tls selects secp256r1
+        let sh = transcript.server_hello();
+        let selected_group = sh.selected_group().unwrap().unwrap();
+        assert_eq!(selected_group, iana::constants::secp256r1);
+
+        // there was an HRR
+        assert!(transcript.hello_retry_request().is_some());
+    }
+}