Review Feedback

unix_integration/common/src/unix_config.rs

@@ -1,3 +1,12 @@
+//! This is configuration definitions and parser for the various unix integration
+//! tools and services. This needs to support a number of use cases like pam/nss
+//! modules parsing the config quickly and the unix daemon which has to connect to
+//! various backend sources.
+//!
+//! To achieve this the configuration has two main sections - the configuration
+//! specification which will be parsed by the tools, then the configuration as
+//! relevant to that tool.
+
 use std::env;
 use std::fmt::{Display, Formatter};
 use std::fs::File;
@@ -51,6 +60,25 @@ impl Display for UidAttr {
     }
 }
 
+#[derive(Debug, Clone, Default)]
+pub enum HsmType {
+    #[cfg_attr(not(feature = "tpm"), default)]
+    Soft,
+    #[cfg_attr(feature = "tpm", default)]
+    TpmIfPossible,
+    Tpm,
+}
+
+impl Display for HsmType {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            HsmType::Soft => write!(f, "Soft"),
+            HsmType::TpmIfPossible => write!(f, "Tpm if possible"),
+            HsmType::Tpm => write!(f, "Tpm"),
+        }
+    }
+}
+
 // Allowed as the large enum is only short lived at startup to the true config
 #[allow(clippy::large_enum_variant)]
 // This bit of magic lets us deserialise the old config and the new versions.
@@ -73,6 +101,7 @@ enum ConfigVersion {
 
 #[derive(Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
+/// This is the version 2 of the JSON configuration specification for the unixd suite.
 struct ConfigV2 {
     cache_db_path: Option<String>,
     sock_path: Option<String>,
@@ -113,6 +142,7 @@ struct KanidmConfigV2 {
 }
 
 #[derive(Debug, Deserialize)]
+/// This is the version 1 of the JSON configuration specification for the unixd suite.
 struct ConfigInt {
     db_path: Option<String>,
     sock_path: Option<String>,
@@ -137,33 +167,28 @@ struct ConfigInt {
     hsm_type: Option<String>,
     tpm_tcti_name: Option<String>,
 
-    // Detect and warn on values in these places.
+    // Detect and warn on values in these places - this is to catch
+    // when someone is using a v2 value on a v1 config.
     #[serde(default)]
     cache_db_path: Option<toml::value::Value>,
     #[serde(default)]
     kanidm: Option<toml::value::Value>,
 }
 
-#[derive(Debug, Clone, Default)]
+// ========================================================================
-pub enum HsmType {
-    #[cfg_attr(not(feature = "tpm"), default)]
-    Soft,
-    #[cfg_attr(feature = "tpm", default)]
-    TpmIfPossible,
-    Tpm,
-}
 
-impl Display for HsmType {
+#[derive(Debug)]
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+/// This is the parsed Kanidm provider configuration that the Unixd resolver
-        match self {
+/// will use to connect to Kanidm.
-            HsmType::Soft => write!(f, "Soft"),
+pub struct KanidmConfig {
-            HsmType::TpmIfPossible => write!(f, "Tpm if possible"),
+    pub conn_timeout: u64,
-            HsmType::Tpm => write!(f, "Tpm"),
+    pub request_timeout: u64,
-        }
+    pub pam_allowed_login_groups: Vec<String>,
-    }
+    pub map_group: Vec<GroupMap>,
 }
 
 #[derive(Debug)]
+/// This is the parsed configuration for the Unixd resolver.
 pub struct UnixdConfig {
     pub cache_db_path: String,
     pub sock_path: String,
@@ -182,18 +207,9 @@ pub struct UnixdConfig {
     pub hsm_type: HsmType,
     pub hsm_pin_path: String,
     pub tpm_tcti_name: String,
-
     pub kanidm_config: Option<KanidmConfig>,
 }
 
-#[derive(Debug)]
-pub struct KanidmConfig {
-    pub conn_timeout: u64,
-    pub request_timeout: u64,
-    pub pam_allowed_login_groups: Vec<String>,
-    pub map_group: Vec<GroupMap>,
-}
-
 impl Default for UnixdConfig {
     fn default() -> Self {
         UnixdConfig::new()
@@ -540,28 +556,30 @@ impl UnixdConfig {
 }
 
 #[derive(Debug)]
-pub struct KanidmUnixdConfig {
+/// This is the parsed configuration that will be used by pam/nss tools that need fast access to
+/// only the socket and timeout information related to the resolver.
+pub struct PamNssConfig {
     pub sock_path: String,
     // pub conn_timeout: u64,
     pub unix_sock_timeout: u64,
 }
 
-impl Default for KanidmUnixdConfig {
+impl Default for PamNssConfig {
     fn default() -> Self {
-        KanidmUnixdConfig::new()
+        PamNssConfig::new()
     }
 }
 
-impl Display for KanidmUnixdConfig {
+impl Display for PamNssConfig {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         writeln!(f, "sock_path: {}", self.sock_path)?;
         writeln!(f, "unix_sock_timeout: {}", self.unix_sock_timeout)
     }
 }
 
-impl KanidmUnixdConfig {
+impl PamNssConfig {
     pub fn new() -> Self {
-        KanidmUnixdConfig {
+        PamNssConfig {
             sock_path: DEFAULT_SOCK_PATH.to_string(),
             unix_sock_timeout: DEFAULT_CONN_TIMEOUT * 2,
         }
@@ -628,7 +646,7 @@ impl KanidmUnixdConfig {
             .unwrap_or(self.unix_sock_timeout);
 
         // Now map the values into our config.
-        Ok(KanidmUnixdConfig {
+        Ok(PamNssConfig {
             sock_path: config.sock_path.unwrap_or(self.sock_path),
             unix_sock_timeout,
         })
@@ -642,7 +660,7 @@ impl KanidmUnixdConfig {
             .map(|timeout| timeout * 2);
 
         // Now map the values into our config.
-        Ok(KanidmUnixdConfig {
+        Ok(PamNssConfig {
             sock_path: config.sock_path.unwrap_or(self.sock_path),
             unix_sock_timeout: kanidm_conn_timeout.unwrap_or(self.unix_sock_timeout),
         })

unix_integration/nss_kanidm/src/core.rs

@@ -1,5 +1,5 @@
 use kanidm_unix_common::client_sync::DaemonClientBlocking;
-use kanidm_unix_common::unix_config::KanidmUnixdConfig;
+use kanidm_unix_common::unix_config::PamNssConfig;
 use kanidm_unix_common::unix_passwd::{
     read_etc_group_file, read_etc_passwd_file, EtcGroup, EtcUser,
 };
@@ -36,7 +36,7 @@ impl RequestOptions {
     fn connect_to_daemon(self) -> Source {
         match self {
             RequestOptions::Main { config_path } => {
-                let maybe_client = KanidmUnixdConfig::new()
+                let maybe_client = PamNssConfig::new()
                     .read_options_from_optional_config(config_path)
                     .ok()
                     .and_then(|cfg| {

unix_integration/pam_kanidm/src/core.rs

@@ -2,7 +2,7 @@ use crate::constants::PamResultCode;
 use crate::module::PamResult;
 use crate::pam::ModuleOptions;
 use kanidm_unix_common::client_sync::DaemonClientBlocking;
-use kanidm_unix_common::unix_config::KanidmUnixdConfig;
+use kanidm_unix_common::unix_config::PamNssConfig;
 use kanidm_unix_common::unix_passwd::{
     read_etc_passwd_file, read_etc_shadow_file, EtcShadow, EtcUser,
 };
@@ -44,7 +44,7 @@ impl RequestOptions {
     fn connect_to_daemon(self) -> Source {
         match self {
             RequestOptions::Main { config_path } => {
-                let maybe_client = KanidmUnixdConfig::new()
+                let maybe_client = PamNssConfig::new()
                     .read_options_from_optional_config(config_path)
                     .ok()
                     .and_then(|cfg| {

unix_integration/pam_kanidm/src/pam/mod.rs

@@ -36,7 +36,7 @@ use std::convert::TryFrom;
 use std::ffi::CStr;
 
 use kanidm_unix_common::constants::DEFAULT_CONFIG_PATH;
-use kanidm_unix_common::unix_config::KanidmUnixdConfig;
+use kanidm_unix_common::unix_config::PamNssConfig;
 
 use crate::core::{self, RequestOptions};
 use crate::pam::constants::*;
@@ -50,8 +50,8 @@ use tracing_subscriber::filter::LevelFilter;
 use tracing_subscriber::fmt;
 use tracing_subscriber::prelude::*;
 
-pub fn get_cfg() -> Result<KanidmUnixdConfig, PamResultCode> {
+pub fn get_cfg() -> Result<PamNssConfig, PamResultCode> {
-    KanidmUnixdConfig::new()
+    PamNssConfig::new()
         .read_options_from_optional_config(DEFAULT_CONFIG_PATH)
         .map_err(|_| PamResultCode::PAM_SERVICE_ERR)
 }

unix_integration/resolver/src/bin/kanidm-unix.rs

@@ -18,7 +18,7 @@ use std::process::ExitCode;
 use clap::Parser;
 use kanidm_unix_common::client::DaemonClient;
 use kanidm_unix_common::constants::DEFAULT_CONFIG_PATH;
-use kanidm_unix_common::unix_config::KanidmUnixdConfig;
+use kanidm_unix_common::unix_config::PamNssConfig;
 use kanidm_unix_common::unix_proto::{
     ClientRequest, ClientResponse, PamAuthRequest, PamAuthResponse, PamServiceInfo,
 };
@@ -28,8 +28,7 @@ include!("../opt/tool.rs");
 
 macro_rules! setup_client {
     () => {{
-        let Ok(cfg) =
+        let Ok(cfg) = PamNssConfig::new().read_options_from_optional_config(DEFAULT_CONFIG_PATH)
-            KanidmUnixdConfig::new().read_options_from_optional_config(DEFAULT_CONFIG_PATH)
         else {
             error!("Failed to parse {}", DEFAULT_CONFIG_PATH);
             return ExitCode::FAILURE;

unix_integration/resolver/src/bin/kanidm_ssh_authorizedkeys.rs

@@ -19,7 +19,7 @@ use std::process::ExitCode;
 use clap::Parser;
 use kanidm_unix_common::client::DaemonClient;
 use kanidm_unix_common::constants::DEFAULT_CONFIG_PATH;
-use kanidm_unix_common::unix_config::KanidmUnixdConfig;
+use kanidm_unix_common::unix_config::PamNssConfig;
 use kanidm_unix_common::unix_proto::{ClientRequest, ClientResponse};
 
 include!("../opt/ssh_authorizedkeys.rs");
@@ -44,8 +44,7 @@ async fn main() -> ExitCode {
 
     debug!("Starting authorized keys tool ...");
 
-    let cfg = match KanidmUnixdConfig::new().read_options_from_optional_config(DEFAULT_CONFIG_PATH)
+    let cfg = match PamNssConfig::new().read_options_from_optional_config(DEFAULT_CONFIG_PATH) {
-    {
         Ok(c) => c,
         Err(e) => {
             error!("Failed to parse {}: {:?}", DEFAULT_CONFIG_PATH, e);