From e837aca1981553624ecd82133c93706b08741b35 Mon Sep 17 00:00:00 2001 From: Chris Connelly Date: Fri, 13 Aug 2021 22:32:28 +0100 Subject: [PATCH] refactor!: Separate `ConfigError` from `Error` Currently, the only configuration-time errors are related to certificate generation. Since we currently generate self-signed certificates, the only failures that could happen would be due to bugs. As such, the `ConfigError::CertificateGeneration` variant doesn't attempt to capture all the info, rather just an opaque `Error` (though the backtrace and source chain will be preserved). This removes a number of variants from `Error`. BREAKING CHANGE: There are several breaking changes: - `QuicP2p::from_config` now returns `Result`. - The `Error::CertificateParse` variant has been removed. - The `Error::CertificatePkParse` variant has been removed. - The `Error::CertificateGen` variant has been removed. - The `Error::Tls` variant has been removed. --- src/api.rs | 20 +++++++++--------- src/config.rs | 51 +++++++++++++++++++++++++++++++++++++++++----- src/connections.rs | 2 +- src/error.rs | 13 ------------ src/lib.rs | 2 +- 5 files changed, 58 insertions(+), 30 deletions(-) diff --git a/src/api.rs b/src/api.rs index d22ffaaf..39011dd6 100644 --- a/src/api.rs +++ b/src/api.rs @@ -8,7 +8,7 @@ // Software. use super::{ - config::{Config, InternalConfig}, + config::{Config, ConfigError, InternalConfig}, connection_pool::ConnId, connections::DisconnectionEvents, endpoint::{Endpoint, IncomingConnections, IncomingMessages}, @@ -48,7 +48,7 @@ impl QuicP2p { /// let quic_p2p = QuicP2p::::with_config(Config::default()) /// .expect("Error initializing QuicP2p"); /// ``` - pub fn with_config(cfg: Config) -> Result { + pub fn with_config(cfg: Config) -> Result { debug!("Config passed in to qp2p: {:?}", cfg); Ok(Self { @@ -65,20 +65,20 @@ impl QuicP2p { /// # Example /// /// ``` - /// use qp2p::{QuicP2p, Config, Error, ConnId}; - /// use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + /// use qp2p::{QuicP2p, Config, ConnId}; + /// use std::{error::Error, net::{IpAddr, Ipv4Addr, SocketAddr}}; /// /// # #[derive(Default, Ord, PartialEq, PartialOrd, Eq, Clone, Copy)] /// # struct XId(pub [u8; 32]); /// # /// # impl ConnId for XId { - /// # fn generate(_socket_addr: &SocketAddr) -> Result> { + /// # fn generate(_socket_addr: &SocketAddr) -> Result> { /// # Ok(XId(rand::random())) /// # } /// # } /// /// #[tokio::main] - /// async fn main() -> Result<(), Error> { + /// async fn main() -> Result<(), Box> { /// let local_addr = (IpAddr::V4(Ipv4Addr::LOCALHOST), 3000).into(); /// let quic_p2p = QuicP2p::::with_config(Config::default())?; /// let (mut endpoint, _, _, _) = quic_p2p.new_endpoint(local_addr).await?; @@ -122,20 +122,20 @@ impl QuicP2p { /// # Example /// /// ``` - /// use qp2p::{QuicP2p, Config, Error, ConnId}; - /// use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + /// use qp2p::{QuicP2p, Config, ConnId}; + /// use std::{error::Error, net::{IpAddr, Ipv4Addr, SocketAddr}}; /// /// # #[derive(Default, Ord, PartialEq, PartialOrd, Eq, Clone, Copy)] /// # struct XId(pub [u8; 32]); /// # /// # impl ConnId for XId { - /// # fn generate(_socket_addr: &SocketAddr) -> Result> { + /// # fn generate(_socket_addr: &SocketAddr) -> Result> { /// # Ok(XId(rand::random())) /// # } /// # } /// /// #[tokio::main] - /// async fn main() -> Result<(), Error> { + /// async fn main() -> Result<(), Box> { /// let local_addr = (IpAddr::V4(Ipv4Addr::LOCALHOST), 0).into(); /// let quic_p2p = QuicP2p::::with_config(Config::default())?; /// let (endpoint, incoming_connections, incoming_messages, disconnections) = quic_p2p.new_endpoint(local_addr).await?; diff --git a/src/config.rs b/src/config.rs index 6d84d353..0dba092b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,7 +9,6 @@ //! Configuration for `Endpoint`s. -use crate::error::{Error, Result}; use serde::{Deserialize, Serialize}; use std::{net::IpAddr, sync::Arc, time::Duration}; use structopt::StructOpt; @@ -31,6 +30,46 @@ pub const DEFAULT_MIN_RETRY_DURATION: Duration = Duration::from_secs(30); const MAIDSAFE_DOMAIN: &str = "maidsafe.net"; +// Convenience alias – not for export. +type Result = std::result::Result; + +/// Configuration errors. +#[derive(Debug, thiserror::Error)] +pub enum ConfigError { + /// An error occurred when generating the TLS certificate. + #[error("An error occurred when generating the TLS certificate")] + CertificateGeneration(#[from] CertificateGenerationError), +} + +impl From for ConfigError { + fn from(error: rcgen::RcgenError) -> Self { + Self::CertificateGeneration(CertificateGenerationError(error.into())) + } +} + +impl From for ConfigError { + fn from(error: quinn::ParseError) -> Self { + Self::CertificateGeneration(CertificateGenerationError(error.into())) + } +} + +impl From for ConfigError { + fn from(error: rustls::TLSError) -> Self { + Self::CertificateGeneration(CertificateGenerationError(error.into())) + } +} + +/// An error that occured when generating the TLS certificate. +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct CertificateGenerationError( + // Though there are multiple different errors that could occur by the code, since we are + // generating a certificate, they should only really occur due to buggy implementations. As + // such, we don't attempt to expose more detail than 'something went wrong', which will + // hopefully be enough for someone to file a bug report... + Box, +); + /// QuicP2p configurations #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq, StructOpt)] pub struct Config { @@ -177,11 +216,13 @@ impl InternalConfig { fn generate_cert() -> Result<(quinn::Certificate, quinn::PrivateKey)> { let cert = rcgen::generate_simple_self_signed(vec![MAIDSAFE_DOMAIN.to_string()])?; + + let cert_der = cert.serialize_der()?; + let key_der = cert.serialize_private_key_der(); + Ok(( - quinn::Certificate::from_der(&cert.serialize_der()?) - .map_err(|_| Error::CertificateParse)?, - quinn::PrivateKey::from_der(&cert.serialize_private_key_der()) - .map_err(|_| Error::CertificatePkParse)?, + quinn::Certificate::from_der(&cert_der)?, + quinn::PrivateKey::from_der(&key_der)?, )) } } diff --git a/src/connections.rs b/src/connections.rs index 778f6ff4..76bb9690 100644 --- a/src/connections.rs +++ b/src/connections.rs @@ -434,7 +434,7 @@ mod tests { use anyhow::anyhow; #[tokio::test(flavor = "multi_thread")] - async fn echo_service() -> Result<(), Error> { + async fn echo_service() -> Result<(), anyhow::Error> { let qp2p = QuicP2p::<[u8; 32]>::with_config(Config::default())?; // Create Endpoint diff --git a/src/error.rs b/src/error.rs index 83e3d950..c9095882 100644 --- a/src/error.rs +++ b/src/error.rs @@ -47,12 +47,6 @@ pub enum Error { /// Failed to set/get priority of stream. #[error("Unknown stream, cannot set/get priority.")] UnknownStream, - /// Certificate for secure communication couldn't be parsed. - #[error("Cannot parse certificate ")] - CertificateParse, - /// The certificate's private key for secure communication couldn't be parsed. - #[error("Cannot parse certificate's private key")] - CertificatePkParse, /// The contacts list was found empty when attempting /// to contact peers for the echo service. #[error("No peers found in the contacts list to send the echo request to")] @@ -64,9 +58,6 @@ pub enum Error { /// Failure occurred when sending an echo request. #[error("{0}")] EchoServiceFailure(String), - /// TLS error - #[error("TLS Error ")] - Tls(#[from] rustls::TLSError), /// Serialisation error, which can happen on different type of data. #[error("Serialisation error")] Serialisation(#[from] bincode::Error), @@ -97,10 +88,6 @@ pub enum Error { /// IGD is not supported on IPv6 #[error("IGD is not supported on IPv6")] IgdNotSupported, - /// An error was encountered when trying to either generate - /// or serialise a self-signed certificate. - #[error("Self-signed certificate generation error: {0}")] - CertificateGen(#[from] rcgen::RcgenError), /// Response message received contains an empty payload. #[error("Empty response message received from peer")] EmptyResponse, diff --git a/src/lib.rs b/src/lib.rs index de44c387..7505d533 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,7 +60,7 @@ mod utils; mod wire_msg; pub use api::QuicP2p; -pub use config::Config; +pub use config::{Config, ConfigError}; pub use connection_pool::ConnId; pub use connections::{DisconnectionEvents, RecvStream, SendStream}; pub use endpoint::{Endpoint, IncomingConnections, IncomingMessages};