Support EC private keys

We didn't support elliptic curve private keys. See
bolcom/unFTP#126 .

I took the opportunity to also improve the error handling in the TLS
code.

Cargo.toml

@@ -30,32 +30,32 @@ members = [
 ]
 
 [dependencies]
-async-trait = "0.1.56"
+async-trait = "0.1.57"
 bitflags = "1.3.2"
-bytes = "1.2.0"
-chrono = { version = "0.4.19", default-features = false, features = ["std"] }
+bytes = "1.2.1"
+chrono = { version = "0.4.22", default-features = false, features = ["std"] }
 derive_more = { version = "0.99.17", features = ["display"] }
-futures-util = { version = "0.3.21", default-features = false, features = ["alloc", "sink"] }
+futures-util = { version = "0.3.24", default-features = false, features = ["alloc", "sink"] }
 getrandom = "0.2.7"
 lazy_static = "1.4.0"
-md-5 = "0.10.1"
-moka = "0.9.2"
-prometheus = { version = "0.13.1", default-features = false }
+md-5 = "0.10.4"
+moka = "0.9.4"
+prometheus = { version = "0.13.2", default-features = false }
 proxy-protocol = "0.5.0"
 rustls = "0.20.6"
-rustls-pemfile = "1.0.0"
+rustls-pemfile = "1.0.1"
 slog = { version = "2.7.0", features = ["max_level_trace", "release_max_level_info"] }
 slog-stdlog = "4.1.1"
-thiserror = "1.0.31"
-tokio = { version = "1.20.0", features = ["macros", "rt", "net", "sync", "io-util", "time"] }
+thiserror = "1.0.35"
+tokio = { version = "1.21.1", features = ["macros", "rt", "net", "sync", "io-util", "time"] }
 tokio-rustls = "0.23.4"
-tokio-util = { version = "0.7.3", features = ["codec"] }
-tracing = { version = "0.1.35", default-features = false }
+tokio-util = { version = "0.7.4", features = ["codec"] }
+tracing = { version = "0.1.36", default-features = false }
 tracing-attributes = "0.1.22"
 uuid = { version = "1.1.2", features = ["v4"] }
 x509-parser = "0.14.0"
 
 [dev-dependencies]
-pretty_assertions = "1.2.1"
-tokio = { version = "1.20.0", features = ["macros", "rt-multi-thread"] }
+pretty_assertions = "1.3.0"
+tokio = { version = "1.21.1", features = ["macros", "rt-multi-thread"] }
 unftp-sbe-fs = { path = "../libunftp/crates/unftp-sbe-fs"}

src/server/ftpserver/error.rs

@@ -23,7 +23,7 @@ impl ServerError {
     }
 }
 
-impl From<std::net::AddrParseError> for ServerError {
+impl From<AddrParseError> for ServerError {
     fn from(e: AddrParseError) -> Self {
         ServerError::new("could not parse address", e)
     }
@@ -35,6 +35,12 @@ impl From<std::io::Error> for ServerError {
     }
 }
 
+impl From<super::tls::ConfigError> for ServerError {
+    fn from(e: super::tls::ConfigError) -> Self {
+        ServerError::new("error with TLS configuration", e)
+    }
+}
+
 #[derive(Error, Debug)]
 #[error("shutdown error: {msg}")]
 pub struct ShutdownError {

src/server/tls.rs

@@ -1,20 +1,19 @@
 use crate::options::{FtpsClientAuth, TlsFlags};
-use rustls::server::StoresServerSessions;
 use rustls::{
-    server::{AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient, NoClientAuth, NoServerSessionStorage},
+    server::{AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient, NoClientAuth, NoServerSessionStorage, StoresServerSessions},
     version::{TLS12, TLS13},
     Certificate, NoKeyLog, PrivateKey, RootCertStore, ServerConfig, SupportedProtocolVersion, Ticketer,
 };
 use std::{
-    error::Error,
-    fmt,
-    fmt::Formatter,
+    fmt::{self, Display, Formatter},
     fs::File,
-    io::BufReader,
+    io::{self, BufReader},
     path::{Path, PathBuf},
     sync::Arc,
     time::Duration,
 };
+use thiserror::Error;
+use tokio_rustls::webpki;
 
 // FTPSConfig shows how TLS security is configured for the server or a particular channel.
 #[derive(Clone)]
@@ -37,21 +36,37 @@ impl fmt::Debug for FtpsConfig {
 #[derive(Debug, Copy, Clone)]
 pub struct FtpsNotAvailable;
 
-impl fmt::Display for FtpsNotAvailable {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+impl Display for FtpsNotAvailable {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         write!(f, "FTPS not configured/available")
     }
 }
 
-impl Error for FtpsNotAvailable {}
+impl std::error::Error for FtpsNotAvailable {}
+
+// The error returned by new_config
+#[derive(Error, Debug)]
+pub enum ConfigError {
+    #[error("found no private key")]
+    NoPrivateKey,
+
+    #[error("error reading key/cert input")]
+    Load(#[from] io::Error),
+
+    #[error("error building root certs")]
+    RootCerts(#[from] webpki::Error),
+
+    #[error("error initialising Rustls")]
+    RustlsInit(#[from] rustls::Error),
+}
 
 pub fn new_config<P: AsRef<Path>>(
     certs_file: P,
     key_file: P,
     flags: TlsFlags,
     client_auth: FtpsClientAuth,
     trust_store: P,
-) -> std::io::Result<Arc<ServerConfig>> {
+) -> Result<Arc<ServerConfig>, ConfigError> {
     let certs: Vec<Certificate> = load_certs(certs_file)?;
     let privkey: PrivateKey = load_private_key(key_file)?;
 
@@ -78,10 +93,10 @@ pub fn new_config<P: AsRef<Path>>(
     let mut config = ServerConfig::builder()
         .with_safe_default_cipher_suites()
         .with_safe_default_kx_groups()
-        .with_protocol_versions(&versions).map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?
+        .with_protocol_versions(&versions).map_err(ConfigError::RustlsInit)?
         .with_client_cert_verifier(client_auther)
         // No SNI, single certificate 
-        .with_single_cert(certs, privkey).map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
+        .with_single_cert(certs, privkey).map_err(ConfigError::RustlsInit)?;
 
     // Support session resumption with server side state (Session IDs)
     config.session_storage = if flags.contains(TlsFlags::RESUMPTION_SESS_ID) {
@@ -91,61 +106,53 @@ pub fn new_config<P: AsRef<Path>>(
     };
     // Support session resumption with tickets. See https://tools.ietf.org/html/rfc5077
     if flags.contains(TlsFlags::RESUMPTION_TICKETS) {
-        config.ticketer = Ticketer::new().map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
+        config.ticketer = Ticketer::new().map_err(ConfigError::RustlsInit)?;
     };
     // Don't allow dumping session keys
     config.key_log = Arc::new(NoKeyLog {});
 
     Ok(Arc::new(config))
 }
 
-fn root_cert_store<P: AsRef<Path>>(trust_pem: P) -> std::io::Result<RootCertStore> {
+fn root_cert_store<P: AsRef<Path>>(trust_pem: P) -> Result<RootCertStore, ConfigError> {
     let mut store = RootCertStore::empty();
     let certs = load_certs(trust_pem)?;
     for cert in certs.iter() {
-        store.add(cert).map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))?
+        store.add(cert).map_err(ConfigError::RootCerts)?
     }
     Ok(store)
 }
 
-fn load_certs<P: AsRef<Path>>(filename: P) -> std::io::Result<Vec<Certificate>> {
+fn load_certs<P: AsRef<Path>>(filename: P) -> Result<Vec<Certificate>, ConfigError> {
     let certfile: File = File::open(filename)?;
     let mut reader: BufReader<File> = BufReader::new(certfile);
-    rustls_pemfile::certs(&mut reader)
-        .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))
-        .map(|v| {
-            let mut res = Vec::with_capacity(v.len());
-            for e in v {
-                res.push(Certificate(e));
-            }
-            res
-        })
+    rustls_pemfile::certs(&mut reader).map_err(ConfigError::Load).map(|v| {
+        let mut res = Vec::with_capacity(v.len());
+        for e in v {
+            res.push(Certificate(e));
+        }
+        res
+    })
 }
 
-fn load_private_key<P: AsRef<Path>>(filename: P) -> std::io::Result<PrivateKey> {
-    let rsa_keys = {
-        let keyfile = File::open(&filename)?;
-        let mut reader = BufReader::new(keyfile);
-        rustls_pemfile::rsa_private_keys(&mut reader).map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))?
-    };
+fn load_private_key<P: AsRef<Path>>(filename: P) -> Result<PrivateKey, ConfigError> {
+    use rustls_pemfile::{read_one, Item};
+    use std::iter;
 
-    let pkcs8_keys = {
-        let keyfile = File::open(&filename)?;
-        let mut reader = BufReader::new(keyfile);
-        rustls_pemfile::pkcs8_private_keys(&mut reader).map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))?
-    };
+    let keyfile = File::open(&filename)?;
+    let mut reader = BufReader::new(keyfile);
 
-    // prefer to load pkcs8 keys
-    let key = if !pkcs8_keys.is_empty() {
-        pkcs8_keys[0].clone()
-    } else {
-        if rsa_keys.is_empty() {
-            return Err(std::io::Error::from(std::io::ErrorKind::Other));
+    for item in iter::from_fn(|| read_one(&mut reader).transpose()) {
+        match item {
+            Ok(Item::RSAKey(key)) => return Ok(PrivateKey(key)),
+            Ok(Item::PKCS8Key(key)) => return Ok(PrivateKey(key)),
+            Ok(Item::ECKey(key)) => return Ok(PrivateKey(key)),
+            Err(e) => return Err(ConfigError::Load(e)),
+            _ => {}
         }
-        rsa_keys[0].clone()
-    };
+    }
 
-    Ok(PrivateKey(key))
+    Err(ConfigError::NoPrivateKey)
 }
 
 /// Stores the session IDs server side.