Merge pull request #14 from ionut-arm/ipc

Factor IPC handling out of the request handler

Author: ionut-arm

Cargo.toml

@@ -16,7 +16,7 @@ parsec-interface = "0.10.0"
 num = "0.2.1"
 rand = "0.7.3"
 log = "0.4.8"
-derivative = "2.0.2"
+derivative = "2.1.0"
 
 [dev-dependencies]
 mockstream = "0.0.3"

src/core/ipc_client/mod.rs

@@ -0,0 +1,21 @@
+// Copyright 2020 Contributors to the Parsec project.
+// SPDX-License-Identifier: Apache-2.0
+//! Types implementing an abstraction over IPC channels
+use crate::error::Result;
+use std::io::{Read, Write};
+
+pub mod unix_socket;
+
+/// This trait is created to allow the iterator returned by incoming to iterate over a trait object
+/// that implements both Read and Write.
+pub trait ReadWrite: Read + Write {}
+// Automatically implements ReadWrite for all types that implement Read and Write.
+impl<T: Read + Write> ReadWrite for T {}
+
+/// Trait that must be implemented by any IPC client
+///
+/// The trait is used by the request handler for obtaining a stream to the service.
+pub trait Connect {
+    /// Connect to underlying IPC and return a readable and writeable stream
+    fn connect(&self) -> Result<Box<dyn ReadWrite>>;
+}

src/core/ipc_client/unix_socket.rs

@@ -0,0 +1,51 @@
+// Copyright 2020 Contributors to the Parsec project.
+// SPDX-License-Identifier: Apache-2.0
+//! Handler for Unix domain sockets
+use super::{Connect, ReadWrite};
+use crate::error::{ClientErrorKind, Result};
+use std::os::unix::net::UnixStream;
+use std::path::PathBuf;
+use std::time::Duration;
+
+const DEFAULT_SOCKET_PATH: &str = "/tmp/security-daemon-socket";
+const DEFAULT_TIMEOUT: Duration = Duration::from_secs(1);
+
+/// IPC client for Unix domain sockets
+#[derive(Debug, Clone)]
+pub struct Client {
+    /// Path at which the socket can be found
+    path: PathBuf,
+    /// Timeout for reads and writes on the streams
+    timeout: Option<Duration>,
+}
+
+impl Connect for Client {
+    fn connect(&self) -> Result<Box<dyn ReadWrite>> {
+        let stream = UnixStream::connect(self.path.clone()).map_err(ClientErrorKind::Ipc)?;
+
+        stream
+            .set_read_timeout(self.timeout)
+            .map_err(ClientErrorKind::Ipc)?;
+        stream
+            .set_write_timeout(self.timeout)
+            .map_err(ClientErrorKind::Ipc)?;
+
+        Ok(Box::from(stream))
+    }
+}
+
+impl Client {
+    /// Create new client using given socket path and timeout duration
+    pub fn new(path: PathBuf, timeout: Option<Duration>) -> Self {
+        Client { path, timeout }
+    }
+}
+
+impl Default for Client {
+    fn default() -> Self {
+        Client {
+            path: DEFAULT_SOCKET_PATH.into(),
+            timeout: Some(DEFAULT_TIMEOUT),
+        }
+    }
+}

src/core/mod.rs

@@ -1,9 +1,10 @@
 // Copyright 2020 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
 //! Client library for integration with the Parsec service
-
+pub mod ipc_client;
 mod operation_handler;
 mod request_handler;
+mod testing;
 
 use crate::auth::AuthenticationData;
 use crate::error::{ClientErrorKind, Error, Result};

src/core/request_handler.rs

@@ -1,72 +1,40 @@
 // Copyright 2020 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
-#![allow(dead_code)]
-
+use super::ipc_client::{unix_socket, Connect};
 use crate::error::{ClientErrorKind, Result};
-#[cfg(test)]
-use mockstream::SyncMockStream;
+use derivative::Derivative;
 use parsec_interface::requests::{Request, Response};
-use std::io::{Read, Write};
-#[cfg(not(test))]
-use std::os::unix::net::UnixStream;
-use std::path::PathBuf;
-use std::time::Duration;
 
 const DEFAULT_MAX_BODY_SIZE: usize = usize::max_value();
-const DEFAULT_SOCKET_PATH: &str = "/tmp/security-daemon-socket";
 
 /// Low level client structure to send a `Request` and get a `Response`.
-#[derive(Clone)]
-#[cfg_attr(not(test), derive(Debug))]
+#[derive(Derivative)]
+#[derivative(Debug)]
 pub struct RequestHandler {
     pub max_body_size: usize,
-    pub timeout: Option<Duration>,
-    pub socket_path: PathBuf,
-    #[cfg(test)]
-    pub mock_stream: SyncMockStream,
+    #[derivative(Debug = "ignore")]
+    pub ipc_client: Box<dyn Connect>,
 }
 
 impl RequestHandler {
     /// Send a request and get a response.
     pub fn process_request(&self, request: Request) -> Result<Response> {
         // Try to connect once, wait for a timeout until trying again.
-        let mut stream = self.connect_to_socket()?;
+        let mut stream = self.ipc_client.connect()?;
 
         request
             .write_to_stream(&mut stream)
             .map_err(ClientErrorKind::Interface)?;
         Ok(Response::read_from_stream(&mut stream, self.max_body_size)
             .map_err(ClientErrorKind::Interface)?)
     }
-
-    #[cfg(test)]
-    fn connect_to_socket(&self) -> Result<impl Read + Write> {
-        Ok(self.mock_stream.clone())
-    }
-
-    #[cfg(not(test))]
-    fn connect_to_socket(&self) -> Result<impl Read + Write> {
-        let stream = UnixStream::connect(&self.socket_path).map_err(ClientErrorKind::Ipc)?;
-
-        stream
-            .set_read_timeout(self.timeout)
-            .map_err(ClientErrorKind::Ipc)?;
-        stream
-            .set_write_timeout(self.timeout)
-            .map_err(ClientErrorKind::Ipc)?;
-
-        Ok(stream)
-    }
 }
 
 impl Default for RequestHandler {
     fn default() -> Self {
         RequestHandler {
             max_body_size: DEFAULT_MAX_BODY_SIZE,
-            timeout: None,
-            socket_path: DEFAULT_SOCKET_PATH.into(),
-            #[cfg(test)]
-            mock_stream: SyncMockStream::new(),
+            ipc_client: Box::from(unix_socket::Client::default()),
         }
     }
 }
@@ -77,33 +45,8 @@ impl crate::CoreClient {
         self.op_handler.request_handler.max_body_size = max_body_size;
     }
 
-    /// Set the timeout allowed for operations on the IPC used for communicating with the service.
-    ///
-    /// A value of `None` represents "no timeout"
-    pub fn set_ipc_timeout(&mut self, timeout: Option<Duration>) {
-        self.op_handler.request_handler.timeout = timeout;
-    }
-
-    /// Set the location of the Unix Socket path where the service socket can be found
-    pub fn set_socket_path(&mut self, socket_path: PathBuf) {
-        self.op_handler.request_handler.socket_path = socket_path;
-    }
-
-    /// Test helper used to set the data to be returned from the mock stream
-    #[cfg(test)]
-    pub fn set_mock_read(&mut self, data: &[u8]) {
-        self.op_handler
-            .request_handler
-            .mock_stream
-            .push_bytes_to_read(data);
-    }
-
-    /// Test helper used to verify the data written to the mock stream
-    #[cfg(test)]
-    pub fn get_mock_write(&mut self) -> Vec<u8> {
-        self.op_handler
-            .request_handler
-            .mock_stream
-            .pop_bytes_written()
+    /// Set the IPC handler used for communication with the service
+    pub fn set_ipc_client(&mut self, ipc_client: Box<dyn Connect>) {
+        self.op_handler.request_handler.ipc_client = ipc_client;
     }
 }

src/tests/core_tests.rs renamed to src/core/testing/core_tests.rs

@@ -1,7 +1,7 @@
 // Copyright 2020 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
+use super::{TestCoreClient, DEFAULT_APP_NAME};
 use crate::error::{ClientErrorKind, Error};
-use crate::{auth::AuthenticationData, CoreClient};
 use mockstream::MockStream;
 use parsec_interface::operations;
 use parsec_interface::operations::list_providers::ProviderInfo;
@@ -27,7 +27,6 @@ const REQ_HEADER: RequestHeader = RequestHeader {
     auth_type: AuthType::NoAuth,
     opcode: Opcode::Ping,
 };
-const APP_NAME: &str = "test-app";
 
 fn get_response_bytes_from_result(result: NativeResult) -> Vec<u8> {
     let mut stream = MockStream::new();
@@ -52,13 +51,9 @@ fn get_operation_from_req_bytes(bytes: Vec<u8>) -> NativeOperation {
         .unwrap()
 }
 
-fn get_client() -> CoreClient {
-    CoreClient::new(AuthenticationData::AppIdentity(String::from(APP_NAME)))
-}
-
 #[test]
 fn ping_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(NativeResult::Ping(
         operations::ping::Result {
             wire_protocol_version_maj: 1,
@@ -74,7 +69,7 @@ fn ping_test() {
 
 #[test]
 fn list_provider_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let mut provider_info = Vec::new();
     provider_info.push(ProviderInfo {
         uuid: uuid::Uuid::nil(),
@@ -101,7 +96,7 @@ fn list_provider_test() {
 
 #[test]
 fn list_provider_operations_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let mut opcodes = HashSet::new();
     let _ = opcodes.insert(Opcode::PsaDestroyKey);
     let _ = opcodes.insert(Opcode::PsaGenerateKey);
@@ -121,7 +116,7 @@ fn list_provider_operations_test() {
 
 #[test]
 fn generate_key_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(
         NativeResult::PsaGenerateKey(operations::psa_generate_key::Result {}),
     ));
@@ -165,7 +160,7 @@ fn generate_key_test() {
 
 #[test]
 fn destroy_key_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(
         NativeResult::PsaDestroyKey(operations::psa_destroy_key::Result {}),
     ));
@@ -188,7 +183,7 @@ fn destroy_key_test() {
 
 #[test]
 fn import_key_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(NativeResult::PsaImportKey(
         operations::psa_import_key::Result {},
     )));
@@ -238,7 +233,7 @@ fn import_key_test() {
 
 #[test]
 fn export_public_key_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let key_data = vec![0xa5; 128];
     client.set_mock_read(&get_response_bytes_from_result(
         NativeResult::PsaExportPublicKey(operations::psa_export_public_key::Result {
@@ -266,7 +261,7 @@ fn export_public_key_test() {
 
 #[test]
 fn sign_hash_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let hash = vec![0x77_u8; 32];
     let key_name = String::from("key_name");
     let sign_algorithm = AsymmetricSignature::Ecdsa {
@@ -305,7 +300,7 @@ fn sign_hash_test() {
 
 #[test]
 fn verify_hash_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let hash = vec![0x77_u8; 32];
     let key_name = String::from("key_name");
     let sign_algorithm = AsymmetricSignature::Ecdsa {
@@ -343,7 +338,7 @@ fn verify_hash_test() {
 
 #[test]
 fn different_response_type_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(
         NativeResult::PsaVerifyHash(operations::psa_verify_hash::Result {}),
     ));
@@ -360,7 +355,7 @@ fn different_response_type_test() {
 
 #[test]
 fn response_status_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     let mut stream = MockStream::new();
     let status = ResponseStatus::PsaErrorDataCorrupt;
     let mut resp = Response::from_request_header(REQ_HEADER, ResponseStatus::Success);
@@ -374,7 +369,7 @@ fn response_status_test() {
 
 #[test]
 fn malformed_response_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&[0xcb_u8; 130]);
     let err = client.ping().expect_err("Error was expected");
 
@@ -386,7 +381,7 @@ fn malformed_response_test() {
 
 #[test]
 fn request_fields_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(NativeResult::Ping(
         operations::ping::Result {
             wire_protocol_version_maj: 1,
@@ -401,7 +396,7 @@ fn request_fields_test() {
 
 #[test]
 fn auth_value_test() {
-    let mut client = get_client();
+    let mut client: TestCoreClient = Default::default();
     client.set_mock_read(&get_response_bytes_from_result(
         NativeResult::PsaDestroyKey(operations::psa_destroy_key::Result {}),
     ));
@@ -413,6 +408,6 @@ fn auth_value_test() {
     let req = get_req_from_bytes(client.get_mock_write());
     assert_eq!(
         String::from_utf8(req.auth.bytes().to_owned()).unwrap(),
-        String::from(APP_NAME)
+        String::from(DEFAULT_APP_NAME)
     );
 }