From 2e608eafeefd7a7c2788e101d7e19e367a44a693 Mon Sep 17 00:00:00 2001 From: Anderson Toshiyuki Sasaki Date: Fri, 31 Jul 2026 16:40:29 +0200 Subject: [PATCH] Fix PCR 16 attestation failure on upgrade from pre-key-separation agent Backport: https://github.com/keylime/rust-keylime/pull/1263 Resolves: RHEL-213679 Signed-off-by: Anderson Toshiyuki Sasaki --- ...ure-on-upgrade-fall-back-to-mtls-key.patch | 448 ++++++++++++++++++ keylime-agent-rust.spec | 9 +- 2 files changed, 456 insertions(+), 1 deletion(-) create mode 100644 0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch diff --git a/0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch b/0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch new file mode 100644 index 0000000..49e0957 --- /dev/null +++ b/0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch @@ -0,0 +1,448 @@ +From 5b69275bd97c9c0788e6441cfb387b59b36c8a51 Mon Sep 17 00:00:00 2001 +From: Anderson Toshiyuki Sasaki +Date: Thu, 30 Jul 2026 16:10:43 +0200 +Subject: [PATCH 1/2] Accept any RSA key size for payload encryption + +The payload key validation was restricted to RSA-2048, rejecting any +other RSA key size loaded from disk. Before the key separation (commits +2091359 and bec5d94), the agent accepted any RSA key from file without +size validation, so users may have manually configured RSA-4096 keys +that worked with attestation. + +Relax the validation to accept any RSA key of at least 2048 bits for +backward compatibility, while still rejecting non-RSA and weak keys. +New key generation continues to use RSA-2048. + +Co-Authored-By: Claude Opus 4.6 +Signed-off-by: Anderson Toshiyuki Sasaki +--- + keylime-agent/src/main.rs | 23 +++++++++++++++++++++-- + 1 file changed, 21 insertions(+), 2 deletions(-) + +diff --git a/keylime-agent/src/main.rs b/keylime-agent/src/main.rs +index 280a1ac8..cf259737 100644 +--- a/keylime-agent/src/main.rs ++++ b/keylime-agent/src/main.rs +@@ -66,7 +66,7 @@ use keylime::{ + }; + use log::*; + use openssl::{ +- pkey::{PKey, Private, Public}, ++ pkey::{Id, PKey, Private, Public}, + x509::X509, + }; + use std::{ +@@ -504,7 +504,7 @@ async fn main() -> Result<()> { + key_path, + Some(config.payload_key_password.as_ref()), + keylime::algorithms::EncryptionAlgorithm::Rsa2048, +- true, // Validate that loaded keys are RSA 2048 ++ false, // Don't validate key size (accept any RSA for backward compatibility) + ) + .map_err(|e| { + error!( +@@ -517,6 +517,25 @@ async fn main() -> Result<()> { + ))) + })?; + ++ if payload_priv_key.id() != Id::RSA || payload_priv_key.bits() < 2048 { ++ error!( ++ "Payload key {} must be an RSA key of at least 2048 bits, \ ++ found {:?} with {} bits", ++ key_path.display(), ++ payload_priv_key.id(), ++ payload_priv_key.bits() ++ ); ++ return Err(Error::Configuration( ++ config::KeylimeConfigError::Generic(format!( ++ "Payload key {} must be an RSA key of at least 2048 \ ++ bits, found {:?} with {} bits", ++ key_path.display(), ++ payload_priv_key.id(), ++ payload_priv_key.bits() ++ )), ++ )); ++ } ++ + // Load or generate mTLS key pair (separate from payload keys) + // The mTLS key is always persistent, stored at the configured path. + // Uses ECC P-256 by default for better security and performance + +From f63fbdf576a8a34200f8eb3dd2c8cfc9d41863d1 Mon Sep 17 00:00:00 2001 +From: Anderson Toshiyuki Sasaki +Date: Thu, 30 Jul 2026 16:11:30 +0200 +Subject: [PATCH 2/2] Fix payload key upgrade regression by falling back to + mTLS key + +On upgrade from a pre-key-separation agent, the new agent generates a +fresh payload key because payload-private.pem does not exist. This +causes PCR 16 to be extended with the hash of the new key while the +verifier still has the old key cached, resulting in +pcr_validation.invalid_pcr_16. + +Add ensure_payload_key() that runs before load_or_generate_key(): if +payload-private.pem is missing but server-private.pem exists and +contains a supported RSA key (at least 2048 bits), it is copied as the +payload key. This preserves PCR 16 consistency during rolling upgrades. + +If the copy fails, the agent aborts rather than continuing with a +potentially corrupt or insecure file. If setting file permissions fails +(IOSetPermissionError), the file is removed so the next startup retries +the copy rather than loading a key with insecure permissions. + +Scenarios handled: +- Fresh install (no key files): generates separate keys (no change) +- Upgrade (supported RSA server key): copies to payload key with warning +- Upgrade (ECC or weak RSA server key): skips fallback, generates new key +- Both files already exist: no fallback logic triggers +- Server key unreadable: logs warning, falls through to generate new key +- Write failure: aborts execution with an actionable error message +- Permission-set failure: removes the file and aborts; next startup retries + +Resolves: keylime/rust-keylime#1262 + +Co-Authored-By: Claude Opus 5 +Signed-off-by: Anderson Toshiyuki Sasaki +--- + keylime-agent/src/main.rs | 316 ++++++++++++++++++++++++++++++++++++++ + 1 file changed, 316 insertions(+) + +diff --git a/keylime-agent/src/main.rs b/keylime-agent/src/main.rs +index cf259737..67a72e25 100644 +--- a/keylime-agent/src/main.rs ++++ b/keylime-agent/src/main.rs +@@ -118,6 +118,118 @@ fn get_retry_config(config: &config::AgentConfig) -> RetryConfig { + } + } + ++/// On upgrade from a pre-key-separation agent, the payload key file does not ++/// exist yet. If the mTLS server key is RSA, copy it as the payload key so ++/// that PCR 16 stays consistent with what the verifier has cached. ++/// ++/// Returns `Ok(())` when the copy succeeds or when a copy is not needed ++/// (key already exists, server key missing, or server key is not RSA). ++/// Returns `Err` if the server key is a valid RSA key but writing the ++/// payload key file fails — continuing in that state could leave a ++/// corrupt or insecure file behind. ++fn ensure_payload_key( ++ payload_key_path: &Path, ++ payload_key_password: &str, ++ server_key_path: &Path, ++ server_key_password: &str, ++) -> std::result::Result<(), Error> { ++ if payload_key_path.exists() { ++ return Ok(()); ++ } ++ if !server_key_path.exists() { ++ return Ok(()); ++ } ++ ++ match crypto::load_key_pair(server_key_path, Some(server_key_password)) { ++ Ok((_, priv_key)) ++ if priv_key.id() == Id::RSA && priv_key.bits() >= 2048 => ++ { ++ crypto::write_key_pair( ++ &priv_key, ++ payload_key_path, ++ Some(payload_key_password), ++ ) ++ .map_err(|e| { ++ if matches!(e, crypto::CryptoError::IOSetPermissionError(_)) { ++ // The key was written successfully but chmod failed. ++ // Remove the file so the next startup retries the copy ++ // rather than loading a key with insecure permissions. ++ // The file did not exist before this call (checked above), ++ // so removal cannot delete pre-existing user data. ++ match fs::remove_file(payload_key_path) { ++ Ok(()) => { ++ error!( ++ "Wrote mTLS key to {} but failed to set \ ++ file permissions: {e}. The file has been \ ++ removed; the copy will be retried on next \ ++ startup.", ++ payload_key_path.display() ++ ); ++ } ++ Err(rm_err) => { ++ error!( ++ "Wrote mTLS key to {} but failed to set \ ++ file permissions: {e}. Cleanup also failed: \ ++ {rm_err}. Remove the file manually before \ ++ restarting the agent to prevent loading a key \ ++ with insecure permissions.", ++ payload_key_path.display() ++ ); ++ } ++ } ++ } else { ++ error!( ++ "Failed to write mTLS key to {}: {e}. \ ++ If a partial file was left behind, remove \ ++ it manually before restarting the agent.", ++ payload_key_path.display() ++ ); ++ } ++ Error::Crypto(e) ++ })?; ++ ++ warn!( ++ "Payload key not found; wrote mTLS key from {} to {} \ ++ for backward compatibility. The same RSA key will \ ++ be used for both mTLS and payload encryption.", ++ server_key_path.display(), ++ payload_key_path.display() ++ ); ++ if !server_key_password.is_empty() ++ && payload_key_password.is_empty() ++ { ++ warn!( ++ "The mTLS key was encrypted at rest but the \ ++ payload key copy at {} is unencrypted. To \ ++ encrypt it, re-encrypt the file manually and \ ++ set 'payload_key_password' in the agent \ ++ configuration.", ++ payload_key_path.display() ++ ); ++ } ++ } ++ Ok(_) => { ++ warn!( ++ "mTLS key {} is not a supported RSA key (at least 2048 \ ++ bits); a new RSA payload key will be generated, which \ ++ will change PCR 16 and may cause attestation failures \ ++ until the agent is re-enrolled.", ++ server_key_path.display() ++ ); ++ } ++ Err(e) => { ++ warn!( ++ "Failed to load mTLS key {}: {e}. \ ++ A new payload key will be generated, which will change \ ++ PCR 16 and may cause attestation failures until the \ ++ agent is re-enrolled.", ++ server_key_path.display() ++ ); ++ } ++ } ++ Ok(()) ++} ++ + // This data is passed in to the actix httpserver threads that + // handle quotes. + #[derive(Debug)] +@@ -500,6 +612,13 @@ async fn main() -> Result<()> { + // by the Tenant and Cloud Verifier, respectively. + // The payload key is always persistent, stored at the configured path. + let key_path = Path::new(&config.payload_key); ++ ensure_payload_key( ++ key_path, ++ config.payload_key_password.as_ref(), ++ Path::new(&config.server_key), ++ config.server_key_password.as_ref(), ++ )?; ++ + let (payload_pub_key, payload_priv_key) = crypto::load_or_generate_key( + key_path, + Some(config.payload_key_password.as_ref()), +@@ -1205,4 +1324,201 @@ mod tests { + Some(config::DEFAULT_EXP_BACKOFF_MAX_DELAY as u64) + ); + } ++ ++ mod ensure_payload_key_tests { ++ use super::*; ++ use openssl::ec::{EcGroup, EcKey}; ++ use openssl::nid::Nid; ++ use openssl::pkey::PKey; ++ use openssl::rsa::Rsa; ++ use tempfile::TempDir; ++ ++ type Result = std::result::Result>; ++ ++ fn write_rsa_key( ++ path: &Path, ++ bits: u32, ++ password: &str, ++ ) -> Result<()> { ++ let rsa = Rsa::generate(bits)?; ++ let key = PKey::from_rsa(rsa)?; ++ crypto::write_key_pair(&key, path, Some(password))?; ++ Ok(()) ++ } ++ ++ fn write_ec_key(path: &Path, password: &str) -> Result<()> { ++ let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1)?; ++ let ec = EcKey::generate(&group)?; ++ let key = PKey::from_ec_key(ec)?; ++ crypto::write_key_pair(&key, path, Some(password))?; ++ Ok(()) ++ } ++ ++ #[test] ++ fn copies_rsa2048_server_key_as_payload_key() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "")?; ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ assert!(payload.exists()); ++ let (_, loaded) = crypto::load_key_pair(&payload, Some(""))?; ++ assert_eq!(loaded.id(), Id::RSA); ++ assert_eq!(loaded.bits(), 2048); ++ Ok(()) ++ } ++ ++ #[test] ++ fn copies_rsa4096_server_key_as_payload_key() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 4096, "")?; ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ assert!(payload.exists()); ++ let (_, loaded) = crypto::load_key_pair(&payload, Some(""))?; ++ assert_eq!(loaded.id(), Id::RSA); ++ assert_eq!(loaded.bits(), 4096); ++ Ok(()) ++ } ++ ++ #[test] ++ fn skips_when_payload_key_already_exists() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "")?; ++ write_rsa_key(&payload, 2048, "")?; ++ ++ let before = fs::read(&payload)?; ++ ensure_payload_key(&payload, "", &server, "")?; ++ let after = fs::read(&payload)?; ++ ++ assert_eq!(before, after); ++ Ok(()) ++ } ++ ++ #[test] ++ fn skips_when_no_server_key() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ assert!(!payload.exists()); ++ Ok(()) ++ } ++ ++ #[test] ++ fn skips_non_rsa_server_key() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_ec_key(&server, "")?; ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ assert!(!payload.exists()); ++ Ok(()) ++ } ++ ++ #[test] ++ fn skips_rsa_key_below_minimum_size() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 1024, "")?; ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ assert!(!payload.exists()); ++ Ok(()) ++ } ++ ++ #[test] ++ fn preserves_key_material_on_copy() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "")?; ++ let (_, original) = crypto::load_key_pair(&server, Some(""))?; ++ ++ ensure_payload_key(&payload, "", &server, "")?; ++ ++ let (_, copied) = crypto::load_key_pair(&payload, Some(""))?; ++ assert_eq!( ++ original.private_key_to_pem_pkcs8()?, ++ copied.private_key_to_pem_pkcs8()?, ++ ); ++ Ok(()) ++ } ++ ++ #[test] ++ fn handles_password_mismatch_gracefully() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "server-pass")?; ++ ++ ensure_payload_key(&payload, "payload-pass", &server, "wrong")?; ++ ++ assert!(!payload.exists()); ++ Ok(()) ++ } ++ ++ #[test] ++ fn copies_with_different_passwords() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ let payload = dir.path().join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "server-pass")?; ++ let (_, original) = ++ crypto::load_key_pair(&server, Some("server-pass"))?; ++ ++ ensure_payload_key( ++ &payload, ++ "payload-pass", ++ &server, ++ "server-pass", ++ )?; ++ ++ let (_, copied) = ++ crypto::load_key_pair(&payload, Some("payload-pass"))?; ++ assert_eq!( ++ original.private_key_to_pem_pkcs8()?, ++ copied.private_key_to_pem_pkcs8()?, ++ ); ++ Ok(()) ++ } ++ ++ #[test] ++ fn returns_err_when_write_fails() -> Result<()> { ++ let dir = TempDir::new()?; ++ let server = dir.path().join("server-private.pem"); ++ // Nonexistent parent directory forces FSCreateError in write_key_pair. ++ let payload = dir ++ .path() ++ .join("nonexistent-subdir") ++ .join("payload-private.pem"); ++ ++ write_rsa_key(&server, 2048, "")?; ++ ++ assert!(ensure_payload_key(&payload, "", &server, "").is_err()); ++ assert!(!payload.exists()); ++ Ok(()) ++ } ++ } + } diff --git a/keylime-agent-rust.spec b/keylime-agent-rust.spec index 15a4467..1886d23 100644 --- a/keylime-agent-rust.spec +++ b/keylime-agent-rust.spec @@ -10,7 +10,7 @@ Name: keylime-agent-rust Version: 0.2.10 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Rust agent for Keylime # Upstream license specification: Apache-2.0 @@ -58,6 +58,9 @@ Patch: 0002-rust-keylime-do-not-require-usr-libexec.patch # Push model is not supported in RHEL 9. # Downstream-only: disable push model in RHEL 9. Patch: 0003-Disable-push-model-agent-driven-attestation-for-RHEL.patch +# RHEL-213679 - Fix PCR 16 attestation failure when upgrading from pre-key-separation agent +# Backport: https://github.com/keylime/rust-keylime/pull/1263 +Patch: 0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch ExclusiveArch: %{rust_arches} @@ -405,6 +408,10 @@ chown -R keylime:keylime %{_sysconfdir}/keylime %endif %changelog +* Fri Jul 31 2026 Anderson Toshiyuki Sasaki - 0.2.10-2 +- Fix PCR 16 attestation failure when upgrading from pre-key-separation agent +- Resolves: RHEL-213679 + * Thu Jul 09 2026 Sergio Correia - 0.2.10-1 - Update to upstream release 0.2.10 - Resolves: RHEL-180619