Improve tpm key generation - improve unix config for tpms. (#1782)

libs/crypto/src/lib.rs

@@ -1179,6 +1179,23 @@ impl Password {
             structures::{Digest, KeyedHashScheme, PublicBuilder, PublicKeyedHashParameters},
         };
 
+        // We generate a digest, which is really some unique small amount of data that
+        // we save into the key context that we are going to save/load. This allows us
+        // to have unique hmac keys compared to other users of the same tpm.
+
+        let digest = tpm_ctx
+            .get_random(16)
+            .map_err(|e| {
+                error!(tpm_err = ?e, "unable to proceed, tpm error");
+                CryptoError::Tpm2
+            })
+            .and_then(|rand| {
+                Digest::try_from(rand).map_err(|e| {
+                    error!(tpm_err = ?e, "unable to proceed, tpm error");
+                    CryptoError::Tpm2
+                })
+            })?;
+
         let object_attributes = ObjectAttributesBuilder::new()
             .with_sign_encrypt(true)
             .with_sensitive_data_origin(true)
@@ -1188,6 +1205,7 @@ impl Password {
                 error!(tpm_err = ?e, "unable to proceed, tpm error");
                 CryptoError::Tpm2
             })?;
+
         let key_pub = PublicBuilder::new()
             .with_public_algorithm(PublicAlgorithm::KeyedHash)
             .with_name_hashing_algorithm(HashingAlgorithm::Sha256)
@@ -1195,7 +1213,7 @@ impl Password {
             .with_keyed_hash_parameters(PublicKeyedHashParameters::new(
                 KeyedHashScheme::HMAC_SHA_256,
             ))
-            .with_keyed_hash_unique_identifier(Digest::default())
+            .with_keyed_hash_unique_identifier(digest)
             .build()
             .map_err(|e| {
                 error!(tpm_err = ?e, "unable to proceed, tpm error");

unix_integration/src/cache.rs

@@ -12,7 +12,7 @@ use reqwest::StatusCode;
 use tokio::sync::{Mutex, RwLock};
 
 use crate::db::Db;
-use crate::unix_config::{HomeAttr, UidAttr};
+use crate::unix_config::{HomeAttr, TpmPolicy, UidAttr};
 use crate::unix_proto::{HomeDirectoryInfo, NssGroup, NssUser};
 
 // use crate::unix_passwd::{EtcUser, EtcGroup};
@@ -77,9 +77,9 @@ impl CacheLayer {
         uid_attr_map: UidAttr,
         gid_attr_map: UidAttr,
         allow_id_overrides: Vec<String>,
-        require_tpm: Option<&str>,
+        tpm_policy: &TpmPolicy,
     ) -> Result<Self, ()> {
-        let db = Db::new(path, require_tpm)?;
+        let db = Db::new(path, tpm_policy)?;
 
         // setup and do a migrate.
         {

unix_integration/src/constants.rs

@@ -14,3 +14,4 @@ pub const DEFAULT_USE_ETC_SKEL: bool = false;
 pub const DEFAULT_UID_ATTR_MAP: UidAttr = UidAttr::Spn;
 pub const DEFAULT_GID_ATTR_MAP: UidAttr = UidAttr::Spn;
 pub const DEFAULT_SELINUX: bool = true;
+pub const DEFAULT_TPM_TCTI_NAME: &str = "device:/dev/tpmrm0";

unix_integration/src/daemon.rs

@@ -673,7 +673,7 @@ async fn main() -> ExitCode {
                 cfg.uid_attr_map,
                 cfg.gid_attr_map,
                 cfg.allow_local_account_override.clone(),
-                cfg.require_tpm.as_deref(),
+                &cfg.tpm_policy,
             )
             .await
             {

unix_integration/src/db.rs

@@ -2,6 +2,7 @@ use std::convert::TryFrom;
 use std::fmt;
 use std::time::Duration;
 
+use crate::unix_config::TpmPolicy;
 use kanidm_lib_crypto::CryptoPolicy;
 use kanidm_lib_crypto::DbPasswordV1;
 use kanidm_lib_crypto::Password;
@@ -26,7 +27,7 @@ pub struct DbTxn<'a> {
 }
 
 impl Db {
-    pub fn new(path: &str, require_tpm: Option<&str>) -> Result<Self, ()> {
+    pub fn new(path: &str, tpm_policy: &TpmPolicy) -> Result<Self, ()> {
         let before = unsafe { umask(0o0027) };
         let conn = Connection::open(path).map_err(|e| {
             error!(err = ?e, "rusqulite error");
@@ -38,23 +39,12 @@ impl Db {
 
         debug!("Configured {:?}", crypto_policy);
 
-        // Test a tpm context.
-        #[allow(unused_variables)]
-        let require_tpm = if let Some(tcti_str) = require_tpm {
-            #[cfg(feature = "tpm")]
-            let r = Db::tpm_setup_context(
-                tcti_str,
-                pool.get().expect("Unable to get connection from pool!!!"),
-            )?;
+        // Test if we have a tpm context.
 
-            #[cfg(not(feature = "tpm"))]
-            warn!("require_tpm is set, but tpm was not built in. This instance will NOT cache passwords!");
-            #[cfg(not(feature = "tpm"))]
-            let r = tpm::TpmConfig {};
-
-            Some(r)
-        } else {
-            None
+        let require_tpm = match tpm_policy {
+            TpmPolicy::Ignore => None,
+            TpmPolicy::IfPossible(tcti_str) => Db::tpm_setup_context(tcti_str, &conn).ok(),
+            TpmPolicy::Required(tcti_str) => Some(Db::tpm_setup_context(tcti_str, &conn)?),
         };
 
         Ok(Db {
@@ -771,15 +761,25 @@ impl<'a> Drop for DbTxn<'a> {
 
 #[cfg(not(feature = "tpm"))]
 pub(crate) mod tpm {
+    use super::Db;
+
+    use rusqlite::Connection;
+
     pub struct TpmConfig {}
+
+    impl Db {
+        pub fn tpm_setup_context(_tcti_str: &str, _conn: &Connection) -> Result<TpmConfig, ()> {
+            warn!("tpm feature is not available in this build");
+            Err(())
+        }
+    }
 }
 
 #[cfg(feature = "tpm")]
 pub(crate) mod tpm {
     use super::Db;
 
-    use r2d2_sqlite::SqliteConnectionManager;
-    use rusqlite::OptionalExtension;
+    use rusqlite::{Connection, OptionalExtension};
 
     use kanidm_lib_crypto::{CryptoError, CryptoPolicy, Password, TpmError};
     use tss_esapi::{utils::TpmsContext, Context, TctiNameConf};
@@ -792,10 +792,7 @@ pub(crate) mod tpm {
     }
 
     impl Db {
-        pub fn tpm_setup_context(
-            tcti_str: &str,
-            conn: r2d2::PooledConnection<SqliteConnectionManager>,
-        ) -> Result<TpmConfig, ()> {
+        pub fn tpm_setup_context(tcti_str: &str, conn: &Connection) -> Result<TpmConfig, ()> {
             let tcti = TctiNameConf::from_str(tcti_str).map_err(|e| {
                 error!(tpm_err = ?e, "Failed to parse tcti name");
             })?;
@@ -939,6 +936,7 @@ mod tests {
 
     use super::Db;
     use crate::cache::Id;
+    use crate::unix_config::TpmPolicy;
 
     const TESTACCOUNT1_PASSWORD_A: &str = "password a for account1 test";
     const TESTACCOUNT1_PASSWORD_B: &str = "password b for account1 test";
@@ -946,7 +944,7 @@ mod tests {
     #[tokio::test]
     async fn test_cache_db_account_basic() {
         sketching::test_init();
-        let db = Db::new("", None).expect("failed to create.");
+        let db = Db::new("", &TpmPolicy::default()).expect("failed to create.");
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
 
@@ -1030,7 +1028,7 @@ mod tests {
     #[tokio::test]
     async fn test_cache_db_group_basic() {
         sketching::test_init();
-        let db = Db::new("", None).expect("failed to create.");
+        let db = Db::new("", &TpmPolicy::default()).expect("failed to create.");
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
 
@@ -1105,7 +1103,7 @@ mod tests {
     #[tokio::test]
     async fn test_cache_db_account_group_update() {
         sketching::test_init();
-        let db = Db::new("", None).expect("failed to create.");
+        let db = Db::new("", &TpmPolicy::default()).expect("failed to create.");
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
 
@@ -1175,12 +1173,12 @@ mod tests {
         sketching::test_init();
 
         #[cfg(feature = "tpm")]
-        let tcti_str = Some("device:/dev/tpmrm0");
+        let tpm_policy = TpmPolicy::Required("device:/dev/tpmrm0".to_string());
 
         #[cfg(not(feature = "tpm"))]
-        let tcti_str = None;
+        let tpm_policy = TpmPolicy::default();
 
-        let db = Db::new("", tcti_str).expect("failed to create.");
+        let db = Db::new("", &tpm_policy).expect("failed to create.");
 
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
@@ -1230,7 +1228,7 @@ mod tests {
     #[tokio::test]
     async fn test_cache_db_group_rename_duplicate() {
         sketching::test_init();
-        let db = Db::new("", None).expect("failed to create.");
+        let db = Db::new("", &TpmPolicy::default()).expect("failed to create.");
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
 
@@ -1285,7 +1283,7 @@ mod tests {
     #[tokio::test]
     async fn test_cache_db_account_rename_duplicate() {
         sketching::test_init();
-        let db = Db::new("", None).expect("failed to create.");
+        let db = Db::new("", &TpmPolicy::default()).expect("failed to create.");
         let dbtxn = db.write().await;
         assert!(dbtxn.migrate().is_ok());
 

unix_integration/src/unix_config.rs

@@ -12,7 +12,8 @@ use serde::Deserialize;
 use crate::constants::{
     DEFAULT_CACHE_TIMEOUT, DEFAULT_CONN_TIMEOUT, DEFAULT_DB_PATH, DEFAULT_GID_ATTR_MAP,
     DEFAULT_HOME_ALIAS, DEFAULT_HOME_ATTR, DEFAULT_HOME_PREFIX, DEFAULT_SELINUX, DEFAULT_SHELL,
-    DEFAULT_SOCK_PATH, DEFAULT_TASK_SOCK_PATH, DEFAULT_UID_ATTR_MAP, DEFAULT_USE_ETC_SKEL,
+    DEFAULT_SOCK_PATH, DEFAULT_TASK_SOCK_PATH, DEFAULT_TPM_TCTI_NAME, DEFAULT_UID_ATTR_MAP,
+    DEFAULT_USE_ETC_SKEL,
 };
 
 #[derive(Debug, Deserialize)]
@@ -33,7 +34,8 @@ struct ConfigInt {
     selinux: Option<bool>,
     #[serde(default)]
     allow_local_account_override: Vec<String>,
-    require_tpm: Option<String>,
+    tpm_tcti_name: Option<String>,
+    tpm_policy: Option<String>,
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -76,6 +78,28 @@ impl Display for UidAttr {
     }
 }
 
+#[derive(Debug, Clone, Default)]
+pub enum TpmPolicy {
+    #[default]
+    Ignore,
+    IfPossible(String),
+    Required(String),
+}
+
+impl Display for TpmPolicy {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            TpmPolicy::Ignore => write!(f, "Ignore"),
+            TpmPolicy::IfPossible(p) => {
+                write!(f, "IfPossible ({})", p)
+            }
+            TpmPolicy::Required(p) => {
+                write!(f, "Required ({})", p)
+            }
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct KanidmUnixdConfig {
     pub db_path: String,
@@ -93,7 +117,7 @@ pub struct KanidmUnixdConfig {
     pub uid_attr_map: UidAttr,
     pub gid_attr_map: UidAttr,
     pub selinux: bool,
-    pub require_tpm: Option<String>,
+    pub tpm_policy: TpmPolicy,
     pub allow_local_account_override: Vec<String>,
 }
 
@@ -128,11 +152,7 @@ impl Display for KanidmUnixdConfig {
         writeln!(f, "gid_attr_map: {}", self.gid_attr_map)?;
 
         writeln!(f, "selinux: {}", self.selinux)?;
-        writeln!(
-            f,
-            "require_tpm: {}",
-            self.require_tpm.as_deref().unwrap_or("-")
-        )?;
+        writeln!(f, "tpm_policy: {}", self.tpm_policy)?;
         writeln!(
             f,
             "allow_local_account_override: {:#?}",
@@ -163,7 +183,7 @@ impl KanidmUnixdConfig {
             uid_attr_map: DEFAULT_UID_ATTR_MAP,
             gid_attr_map: DEFAULT_GID_ATTR_MAP,
             selinux: DEFAULT_SELINUX,
-            require_tpm: None,
+            tpm_policy: TpmPolicy::default(),
             allow_local_account_override: Vec::default(),
         }
     }
@@ -276,7 +296,23 @@ impl KanidmUnixdConfig {
                 true => selinux_util::supported(),
                 _ => false,
             },
-            require_tpm: config.require_tpm.or(self.require_tpm),
+            tpm_policy: config
+                .tpm_policy
+                .and_then(|v| {
+                    let tpm_tcti_name = config
+                        .tpm_tcti_name
+                        .unwrap_or(DEFAULT_TPM_TCTI_NAME.to_string());
+                    match v.as_str() {
+                        "ignore" => Some(TpmPolicy::Ignore),
+                        "if_possible" => Some(TpmPolicy::IfPossible(tpm_tcti_name)),
+                        "required" => Some(TpmPolicy::Required(tpm_tcti_name)),
+                        _ => {
+                            warn!("Invalid tpm_policy configured, using default ...");
+                            None
+                        }
+                    }
+                })
+                .unwrap_or(self.tpm_policy),
             allow_local_account_override: config.allow_local_account_override,
         })
     }

unix_integration/tests/cache_layer_test.rs

@@ -11,6 +11,7 @@ use kanidm_unix_common::constants::{
     DEFAULT_GID_ATTR_MAP, DEFAULT_HOME_ALIAS, DEFAULT_HOME_ATTR, DEFAULT_HOME_PREFIX,
     DEFAULT_SHELL, DEFAULT_UID_ATTR_MAP,
 };
+use kanidm_unix_common::unix_config::TpmPolicy;
 use kanidmd_core::config::{Configuration, IntegrationTestConfig, ServerRole};
 use kanidmd_core::create_server_core;
 use tokio::task;
@@ -110,7 +111,7 @@ async fn setup_test(fix_fn: Fixture) -> (CacheLayer, KanidmClient) {
         DEFAULT_UID_ATTR_MAP,
         DEFAULT_GID_ATTR_MAP,
         vec!["masked_group".to_string()],
-        None,
+        &TpmPolicy::default(),
     )
     .await
     .expect("Failed to build cache layer.");