From 61fbca61c6c2012ec75a41016a00a0406b8a0b04 Mon Sep 17 00:00:00 2001 From: Arnaud Brousseau Date: Wed, 26 Jun 2024 10:31:44 -0500 Subject: [PATCH] Remove ConnectionClosed/EmptyRead errors: these are not errors, they should be propagated as empty, successful reads --- src/integration/src/bin/pivot_remote_tls.rs | 27 +++++++++--- src/integration/tests/remote_tls.rs | 14 ++++++ src/qos_net/src/error.rs | 5 --- src/qos_net/src/proxy.rs | 7 ++- src/qos_net/src/proxy_stream.rs | 47 +++++++++++++++------ 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/integration/src/bin/pivot_remote_tls.rs b/src/integration/src/bin/pivot_remote_tls.rs index e3c5ece2..a44af7e4 100644 --- a/src/integration/src/bin/pivot_remote_tls.rs +++ b/src/integration/src/bin/pivot_remote_tls.rs @@ -1,6 +1,6 @@ use core::panic; use std::{ - io::{Read, Write}, + io::{ErrorKind, Read, Write}, sync::Arc, }; @@ -63,18 +63,31 @@ impl RequestProcessor for Processor { ); tls.write_all(http_request.as_bytes()).unwrap(); - let _ciphersuite = tls.conn.negotiated_cipher_suite().unwrap(); let mut response_bytes = Vec::new(); - let read_to_end_result: usize = - tls.read_to_end(&mut response_bytes).unwrap(); - - // Ignore eof errors: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof + let read_to_end_result = tls.read_to_end(&mut response_bytes); + match read_to_end_result { + Ok(read_size) => { + assert!(read_size > 0); + // Close the connection + let closed = stream.close(); + closed.unwrap(); + } + Err(e) => { + // Only EOF errors are expected. This means the + // connection was closed by the remote server https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof + if e.kind() != ErrorKind::UnexpectedEof { + panic!( + "unexpected error trying to read_to_end: {e:?}" + ); + } + } + } let fetched_content = std::str::from_utf8(&response_bytes).unwrap(); PivotRemoteTlsMsg::RemoteTlsResponse(format!( - "Content fetched successfully ({read_to_end_result} bytes): {fetched_content}" + "Content fetched successfully: {fetched_content}" )) .try_to_vec() .expect("RemoteTlsResponse is valid borsh") diff --git a/src/integration/tests/remote_tls.rs b/src/integration/tests/remote_tls.rs index ec6e6c82..9513c0bb 100644 --- a/src/integration/tests/remote_tls.rs +++ b/src/integration/tests/remote_tls.rs @@ -47,4 +47,18 @@ fn fetch_remote_tls_content() { assert!(response_text.contains("Content fetched successfully")); assert!(response_text.contains("HTTP/1.1 200 OK")); assert!(response_text.contains("currentTime")); + + let app_request = PivotRemoteTlsMsg::RemoteTlsRequest { + host: "www.googleapis.com".to_string(), + path: "/oauth2/v3/certs".to_string(), + } + .try_to_vec() + .unwrap(); + + let response = enclave_client.send(&app_request).unwrap(); + let response_text = str::from_utf8(&response).unwrap(); + + assert!(response_text.contains("Content fetched successfully")); + assert!(response_text.contains("HTTP/1.1 200 OK")); + assert!(response_text.contains("keys")); } diff --git a/src/qos_net/src/error.rs b/src/qos_net/src/error.rs index ec2326b0..326437cc 100644 --- a/src/qos_net/src/error.rs +++ b/src/qos_net/src/error.rs @@ -27,11 +27,6 @@ pub enum QosNetError { DuplicateConnectionId(u32), /// Attempt to send a message to a remote connection, but ID isn't found ConnectionIdNotFound(u32), - /// Attempting to read on a closed remote connection (`.read` returned 0 - /// bytes) - ConnectionClosed, - /// Happens when a socket `read` results in no data - EmptyRead, /// Happens when a socket `read` returns too much data for the provided /// buffer and the data doesn't fit. The first `usize` is the size of the /// received data, the second `usize` is the size of the buffer. diff --git a/src/qos_net/src/proxy.rs b/src/qos_net/src/proxy.rs index edba0561..a9d65f74 100644 --- a/src/qos_net/src/proxy.rs +++ b/src/qos_net/src/proxy.rs @@ -151,7 +151,12 @@ impl Proxy { // connection close. So we can safely remove it. match self.remove_connection(connection_id) { Ok(_) => { - ProxyMsg::ProxyError(QosNetError::ConnectionClosed) + // Connection was successfully removed / closed + ProxyMsg::ReadResponse { + connection_id, + data: buf, + size: 0, + } } Err(e) => ProxyMsg::ProxyError(e), } diff --git a/src/qos_net/src/proxy_stream.rs b/src/qos_net/src/proxy_stream.rs index 5db878d0..a9f63a46 100644 --- a/src/qos_net/src/proxy_stream.rs +++ b/src/qos_net/src/proxy_stream.rs @@ -184,6 +184,10 @@ impl Read for ProxyStream { println!("READ {}: read {} bytes", buf.len(), size); Ok(size) } + ProxyMsg::ProxyError(e) => Err(std::io::Error::new( + ErrorKind::InvalidData, + format!("Proxy error: {e:?}"), + )), _ => Err(std::io::Error::new( ErrorKind::InvalidData, "unexpected response", @@ -310,8 +314,8 @@ mod test { #[test] fn can_fetch_tls_content_with_local_stream() { - let host = "api.turnkey.com"; - let path = "/health"; + let host = "www.googleapis.com"; + let path = "/oauth2/v3/certs"; let mut stream = LocalStream::new_by_name( host.to_string(), @@ -322,7 +326,10 @@ mod test { .unwrap(); assert_eq!(stream.num_connections(), 1); - assert_eq!(stream.remote_hostname, Some("api.turnkey.com".to_string())); + assert_eq!( + stream.remote_hostname, + Some("www.googleapis.com".to_string()) + ); let root_store = RootCertStore { roots: webpki_roots::TLS_SERVER_ROOTS.into() }; @@ -348,19 +355,27 @@ mod test { let mut response_bytes = Vec::new(); let read_to_end_result = tls.read_to_end(&mut response_bytes); - // Ignore eof errors: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof - assert!( - read_to_end_result.is_ok() - || (read_to_end_result - .is_err_and(|e| e.kind() == ErrorKind::UnexpectedEof)) - ); - let response_text = std::str::from_utf8(&response_bytes).unwrap(); - assert!(response_text.contains("HTTP/1.1 200 OK")); - assert!(response_text.contains("currentTime")); + match read_to_end_result { + Ok(read_size) => { + assert!(read_size > 0); + // Close the connection + let closed = stream.close(); + assert!(closed.is_ok()); + } + Err(e) => { + // Only EOF errors are expected. This means the connection was + // closed by the remote server https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof + assert_eq!(e.kind(), ErrorKind::UnexpectedEof) + } + } - let closed = stream.close(); - assert!(closed.is_ok()); + // We should be at 0 connections in our proxy: either the remote + // auto-closed (UnexpectedEof), or we did. assert_eq!(stream.num_connections(), 0); + + let response_text = std::str::from_utf8(&response_bytes).unwrap(); + assert!(response_text.contains("HTTP/1.1 200 OK")); + assert!(response_text.contains("keys")); } /// Struct representing a stream, with direct access to the proxy. @@ -447,6 +462,10 @@ mod test { println!("READ {}: read {} bytes", buf.len(), size); Ok(size) } + ProxyMsg::ProxyError(e) => Err(std::io::Error::new( + ErrorKind::InvalidData, + format!("Proxy error: {e:?}"), + )), _ => Err(std::io::Error::new( ErrorKind::InvalidData, "unexpected response",