From 23eb4283e9e316297670ee85bd67a3c2d218be13 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Tue, 27 Jun 2023 10:09:19 +1000 Subject: [PATCH] Improve tpm key generation - improve unix config for tpms. (#1782) --- libs/crypto/src/lib.rs | 20 +++++++- unix_integration/src/cache.rs | 6 +-- unix_integration/src/constants.rs | 1 + unix_integration/src/daemon.rs | 2 +- unix_integration/src/db.rs | 60 +++++++++++----------- unix_integration/src/unix_config.rs | 56 ++++++++++++++++---- unix_integration/tests/cache_layer_test.rs | 3 +- 7 files changed, 101 insertions(+), 47 deletions(-) diff --git a/libs/crypto/src/lib.rs b/libs/crypto/src/lib.rs index 0f7e396c6..0cbf5e71c 100644 --- a/libs/crypto/src/lib.rs +++ b/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"); diff --git a/unix_integration/src/cache.rs b/unix_integration/src/cache.rs index b314a6f3c..4cbc0667d 100644 --- a/unix_integration/src/cache.rs +++ b/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, - require_tpm: Option<&str>, + tpm_policy: &TpmPolicy, ) -> Result { - let db = Db::new(path, require_tpm)?; + let db = Db::new(path, tpm_policy)?; // setup and do a migrate. { diff --git a/unix_integration/src/constants.rs b/unix_integration/src/constants.rs index ab6b8e644..47f0b0323 100644 --- a/unix_integration/src/constants.rs +++ b/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"; diff --git a/unix_integration/src/daemon.rs b/unix_integration/src/daemon.rs index db538b26e..4fbee2520 100644 --- a/unix_integration/src/daemon.rs +++ b/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 { diff --git a/unix_integration/src/db.rs b/unix_integration/src/db.rs index c6d0dd346..7761a5bff 100644 --- a/unix_integration/src/db.rs +++ b/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 { + pub fn new(path: &str, tpm_policy: &TpmPolicy) -> Result { 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 { + 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, - ) -> Result { + pub fn tpm_setup_context(tcti_str: &str, conn: &Connection) -> Result { 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()); diff --git a/unix_integration/src/unix_config.rs b/unix_integration/src/unix_config.rs index f1bf0b1c1..144101aa3 100644 --- a/unix_integration/src/unix_config.rs +++ b/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, #[serde(default)] allow_local_account_override: Vec, - require_tpm: Option, + tpm_tcti_name: Option, + tpm_policy: Option, } #[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, + pub tpm_policy: TpmPolicy, pub allow_local_account_override: Vec, } @@ -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, }) } diff --git a/unix_integration/tests/cache_layer_test.rs b/unix_integration/tests/cache_layer_test.rs index d812c5879..b61fac69e 100644 --- a/unix_integration/tests/cache_layer_test.rs +++ b/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.");