rpms/keylime-agent-rust
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix PCR 16 attestation failure on upgrade from pre-key-separation agent

Browse Source 
Backport: https://github.com/keylime/rust-keylime/pull/1263
Resolves: RHEL-213679

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
This commit is contained in:
Anderson Toshiyuki Sasaki 2026-07-31 16:40:29 +02:00
parent 3e1ff1d862
commit 2e608eafee
No known key found for this signature in database
2 changed files with 456 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

448
0004-fix-pcr16-failure-on-upgrade-fall-back-to-mtls-key.patch Normal file
View File

@ -0,0 +1,448 @@
From 5b69275bd97c9c0788e6441cfb387b59b36c8a51 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
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 <noreply@anthropic.com>
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
---
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 <ansasaki@redhat.com>
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 <noreply@anthropic.com>
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
---
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<T> = std::result::Result<T, Box<dyn std::error::Error>>;
+
+ 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(())
+ }
+ }
}

9
keylime-agent-rust.spec
View File

@ -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 <ansasaki@redhat.com> - 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 <scorreia@redhat.com> - 0.2.10-1
- Update to upstream release 0.2.10
- Resolves: RHEL-180619