Remove ConnectionClosed/EmptyRead errors: these are not errors, they should be propagated as empty, successful reads

Author: r-n-o

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")

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"));
 }

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.

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),
 					}

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",