Improve TLS configuration errors (#2447)

This improves the errors during TLS configuration to localise them to
the error site, as well as calling our file path diagnostics tool
to assist with permission errors.
This commit is contained in:
Firstyear 2024-01-23 16:13:14 +10:00 committed by GitHub
parent 3698d65982
commit 967bc7c9df
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 49 additions and 31 deletions

View file

@ -7,7 +7,7 @@
use std::fmt;
use std::fs::File;
use std::io::Read;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use kanidm_proto::constants::DEFAULT_SERVER_ADDRESS;
@ -75,9 +75,9 @@ fn default_online_backup_versions() -> usize {
#[derive(Deserialize, Debug, Clone)]
pub struct TlsConfiguration {
pub chain: String,
pub key: String,
pub client_ca: Option<String>,
pub chain: PathBuf,
pub key: PathBuf,
pub client_ca: Option<PathBuf>,
}
/// This is the Server Configuration as read from `server.toml`.
@ -648,9 +648,9 @@ impl Configuration {
match (chain, key) {
(None, None) => {}
(Some(chainp), Some(keyp)) => {
let chain = chainp.to_string();
let key = keyp.to_string();
let client_ca = client_ca.clone();
let chain = PathBuf::from(keyp.clone());
let key = PathBuf::from(chainp.clone());
let client_ca = client_ca.clone().map(PathBuf::from);
self.tls_config = Some(TlsConfiguration {
chain,
key,

View file

@ -93,30 +93,49 @@ pub fn check_privkey_minimums(privkey: &PKeyRef<Private>) -> Result<(), String>
/// From the server configuration, generate an OpenSSL acceptor that we can use
/// to build our sockets for HTTPS/LDAPS.
pub fn setup_tls(config: &Configuration) -> Result<Option<SslAcceptor>, ErrorStack> {
pub fn setup_tls(config: &Configuration) -> Result<Option<SslAcceptor>, ()> {
match &config.tls_config {
Some(tls_config) => {
// Signing algorithm minimums are enforced by the SSLAcceptor - it won't start up with a sha1-signed cert.
let mut ssl_builder = SslAcceptor::mozilla_modern(SslMethod::tls())?;
ssl_builder.set_certificate_chain_file(&tls_config.chain)?;
// https://wiki.mozilla.org/Security/Server_Side_TLS
let mut ssl_builder =
SslAcceptor::mozilla_intermediate_v5(SslMethod::tls()).map_err(|openssl_err| {
error!("Failed to start TLS builder");
error!(?openssl_err);
})?;
ssl_builder.set_private_key_file(&tls_config.key, SslFiletype::PEM)?;
ssl_builder.check_private_key()?;
ssl_builder
.set_certificate_chain_file(&tls_config.chain)
.map_err(|openssl_err| {
error!("Failed to access certificate chain file");
error!(?openssl_err);
let diag = kanidm_lib_file_permissions::diagnose_path(&tls_config.chain);
info!(%diag);
})?;
ssl_builder
.set_private_key_file(&tls_config.key, SslFiletype::PEM)
.map_err(|openssl_err| {
error!("Failed to access private key file");
error!(?openssl_err);
let diag = kanidm_lib_file_permissions::diagnose_path(&tls_config.chain);
info!(%diag);
})?;
ssl_builder.check_private_key().map_err(|openssl_err| {
error!("Failed to validate private key");
error!(?openssl_err);
})?;
let acceptor = ssl_builder.build();
// let's enforce some TLS minimums!
#[allow(clippy::expect_used)]
let privkey = acceptor
.context()
.private_key()
.expect("Couldn't pull TLS key after configuring one!");
let privkey = acceptor.context().private_key().ok_or_else(|| {
error!("Failed to access acceptor private key");
})?;
check_privkey_minimums(privkey).map_err(|err| {
#[cfg(any(test, debug_assertions))]
println!("{}", err);
admin_error!("{}", err);
ErrorStack::get() // this probably should be a real errorstack but... how?
error!("{}", err);
})?;
Ok(Some(acceptor))

View file

@ -397,7 +397,10 @@ async fn server_loop(
// If configured, setup TLS client authentication.
if let Some(client_ca) = tls_param.client_ca.as_ref() {
debug!("Configuring client certificates from {}", client_ca);
debug!(
"Configuring client certificates from {}",
client_ca.display()
);
let verify = SslVerifyMode::PEER;
// In future we may add a "require mTLS option" which would necesitate this.

View file

@ -36,7 +36,6 @@ mod repl;
mod utils;
use std::fmt::{Display, Formatter};
use std::path::Path;
use std::sync::Arc;
use crate::utils::touch_file_or_quit;
@ -628,10 +627,7 @@ pub fn cert_generate_core(config: &Configuration) {
// Get the cert root
let (tls_key_path, tls_chain_path) = match &config.tls_config {
Some(tls_config) => (
Path::new(tls_config.key.as_str()),
Path::new(tls_config.chain.as_str()),
),
Some(tls_config) => (tls_config.key.as_path(), tls_config.chain.as_path()),
None => {
error!("Unable to find tls configuration");
std::process::exit(1);
@ -820,8 +816,8 @@ pub async fn create_server_core(
// Setup TLS (if any)
let _opt_tls_params = match crypto::setup_tls(&config) {
Ok(opt_tls_params) => opt_tls_params,
Err(e) => {
error!("Failed to configure TLS parameters -> {:?}", e);
Err(()) => {
error!("Failed to configure TLS parameters");
return Err(());
}
};
@ -1017,8 +1013,8 @@ pub async fn create_server_core(
Some(la) => {
let opt_ldap_ssl_acceptor = match crypto::setup_tls(&config) {
Ok(t) => t,
Err(e) => {
error!("Failed to configure LDAP TLS parameters -> {:?}", e);
Err(()) => {
error!("Failed to configure LDAP TLS parameters");
return Err(());
}
};