From 4619b4be7681960786de459f74c101ff03c8692e Mon Sep 17 00:00:00 2001 From: Magnus Kulke Date: Tue, 16 Jul 2024 14:39:14 +0200 Subject: [PATCH] AA: handle multiline content in log events The NELR spec doesn't prescribe a format for the `content` payload of an event. However, since a line break is semantic in the log file, the content cannot be a multi-line string: ``` content = "one two\nthree four five" | v my-domain my-operation one two three four five ``` The result would be ambiguous. This change enforces this assertion. Also added tests for the event log and moved the init log line generation to the eventlog module (as the digest calc is already there) Signed-off-by: Magnus Kulke --- .../attestation-agent/src/eventlog.rs | 137 ++++++++++++++---- .../attestation-agent/src/lib.rs | 69 +++++---- 2 files changed, 145 insertions(+), 61 deletions(-) diff --git a/attestation-agent/attestation-agent/src/eventlog.rs b/attestation-agent/attestation-agent/src/eventlog.rs index 47e73e4a1..905b89d35 100644 --- a/attestation-agent/attestation-agent/src/eventlog.rs +++ b/attestation-agent/attestation-agent/src/eventlog.rs @@ -5,7 +5,7 @@ use std::{fmt::Display, fs::File, io::Write}; -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use const_format::concatcp; use crate::config::HashAlgorithm; @@ -17,60 +17,143 @@ pub const EVENTLOG_PARENT_DIR_PATH: &str = "/run/attestation-agent"; pub const EVENTLOG_PATH: &str = concatcp!(EVENTLOG_PARENT_DIR_PATH, "/eventlog"); pub struct EventLog { + writer: Box, +} + +trait Writer: Sync + Send { + fn append(&mut self, entry: &LogEntry) -> Result<()>; +} + +pub struct FileWriter { file: File, } +impl Writer for FileWriter { + fn append(&mut self, entry: &LogEntry) -> Result<()> { + writeln!(self.file, "{entry}").context("failed to write log")?; + self.file + .flush() + .context("failed to flush log to I/O media")?; + Ok(()) + } +} + impl EventLog { pub fn new() -> Result { std::fs::create_dir_all(EVENTLOG_PARENT_DIR_PATH).context("create eventlog parent dir")?; let file = File::create(EVENTLOG_PATH).context("create eventlog")?; - Ok(Self { file }) + let writer = Box::new(FileWriter { file }); + Ok(Self { writer }) } - pub fn write_log(&mut self, log: &str) -> Result<()> { - writeln!(self.file, "{log}").context("failed to write log")?; - self.file - .flush() - .context("failed to flush log to I/O media")?; - Ok(()) + pub fn write_log(&mut self, entry: &LogEntry) -> Result<()> { + self.writer.append(entry) } } -pub struct EventEntry<'a> { - domain: &'a str, - operation: &'a str, - content: &'a str, -} +pub struct Content<'a>(&'a str); -impl<'a> EventEntry<'a> { - pub fn new(domain: &'a str, operation: &'a str, content: &'a str) -> Self { - Self { - domain, - operation, - content, +impl<'a> TryFrom<&'a str> for Content<'a> { + type Error = anyhow::Error; + + fn try_from(value: &'a str) -> Result { + if value.chars().any(|c| c == '\n') { + bail!("content contains newline"); } + Ok(Content(value)) } +} + +pub enum LogEntry<'a> { + Event { + domain: &'a str, + operation: &'a str, + content: Content<'a>, + }, + Init(HashAlgorithm), +} - /// Calculate the EventEntry's digest with the given [`HashAlgorithm`] +impl<'a> LogEntry<'a> { + /// Calculate the LogEntry's digest with the given [`HashAlgorithm`] pub fn digest_with(&self, hash_alg: HashAlgorithm) -> Vec { let log_entry = self.to_string(); hash_alg.digest(log_entry.as_bytes()) } } -impl<'a> Display for EventEntry<'a> { +impl<'a> Display for LogEntry<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{} {} {}", self.domain, self.operation, self.content) + match self { + LogEntry::Event { + domain, + operation, + content, + } => { + write!(f, "{} {} {}", domain, operation, content.0) + } + LogEntry::Init(hash_alg) => { + // TODO: We should get the current platform's evidence to + // see the RTMR value. Here we assume RTMR is not polluted + // thus all be set `\0` + let (sha, zeroes) = match hash_alg { + HashAlgorithm::Sha256 => ("sha256", "0".repeat(64)), + HashAlgorithm::Sha384 => ("sha384", "0".repeat(96)), + HashAlgorithm::Sha512 => ("sha512", "0".repeat(128)), + }; + write!(f, "INIT {}/{}", sha, zeroes) + } + } } } #[cfg(test)] mod tests { + use super::*; + use crate::config::HashAlgorithm; use rstest::rstest; + use std::sync::{Arc, Mutex}; - use crate::config::HashAlgorithm; + struct TestWriter(Arc>>); - use super::EventEntry; + impl Writer for TestWriter { + fn append(&mut self, entry: &LogEntry) -> Result<()> { + self.0.lock().unwrap().push(entry.to_string()); + Ok(()) + } + } + + #[test] + fn test_content() { + let a_str = "hello"; + let _: Content = a_str.try_into().unwrap(); + let b_str = "hello\nworld"; + let content: Result = b_str.try_into(); + assert!(content.is_err()); + } + + #[test] + fn test_log_events() { + let lines = Arc::new(Mutex::new(vec![])); + let tw = TestWriter(lines.clone()); + let mut el = EventLog { + writer: Box::new(tw), + }; + let i = LogEntry::Init(HashAlgorithm::Sha256); + el.write_log(&i).unwrap(); + let i_line = concat!( + "INIT sha256/00000000000000000000000000", + "00000000000000000000000000000000000000" + ); + assert_eq!(lines.lock().unwrap().join("\n"), i_line); + let ev = LogEntry::Event { + domain: "one", + operation: "two", + content: "three".try_into().unwrap(), + }; + el.write_log(&ev).unwrap(); + let e_line = "one two three"; + assert_eq!(lines.lock().unwrap()[1], e_line); + } #[rstest] #[case( @@ -89,7 +172,11 @@ mod tests { #[case] digest: &str, #[case] hash_alg: HashAlgorithm, ) { - let event = EventEntry::new(domain, operation, content); + let event = LogEntry::Event { + domain, + operation, + content: content.try_into().unwrap(), + }; let dig = event.digest_with(hash_alg); let dig_hex = dig.iter().map(|c| format!("{c:02x}")).collect::(); assert_eq!(dig_hex, digest); diff --git a/attestation-agent/attestation-agent/src/lib.rs b/attestation-agent/attestation-agent/src/lib.rs index 9af778bec..0f6c35155 100644 --- a/attestation-agent/attestation-agent/src/lib.rs +++ b/attestation-agent/attestation-agent/src/lib.rs @@ -3,11 +3,10 @@ // SPDX-License-Identifier: Apache-2.0 // -use std::{io::Write, str::FromStr}; - use anyhow::{Context, Result}; use async_trait::async_trait; use attester::{detect_tee_type, BoxedAttester}; +use std::{io::Write, str::FromStr}; use tokio::sync::Mutex; pub use attester::InitdataResult; @@ -16,8 +15,7 @@ pub mod config; mod eventlog; pub mod token; -use config::HashAlgorithm; -use eventlog::{EventEntry, EventLog}; +use eventlog::{Content, EventLog, LogEntry}; use log::{debug, info, warn}; use token::*; @@ -83,28 +81,20 @@ pub struct AttestationAgent { impl AttestationAgent { pub async fn init(&mut self) -> Result<()> { - // We should get the current platform's evidence to see the RTMR value. - // Here we assume RTMR is not polluted thus all be set `\0` - let init_entry = match self.config.eventlog_config.eventlog_algorithm { - HashAlgorithm::Sha256 => "INIT sha256/0000000000000000000000000000000000000000000000000000000000000000", - HashAlgorithm::Sha384 => "INIT sha384/000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - HashAlgorithm::Sha512 => "INIT sha512/00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + let alg = self.config.eventlog_config.eventlog_algorithm; + let pcr = self.config.eventlog_config.init_pcr; + let init_entry = LogEntry::Init(alg); + let digest = init_entry.digest_with(alg); + { + // perform atomicly in this block + let mut eventlog = self.eventlog.lock().await; + self.attester + .extend_runtime_measurement(digest, pcr) + .await + .context("write INIT entry")?; + + eventlog.write_log(&init_entry).context("write INIT log")?; }; - - let event_digest = self - .config - .eventlog_config - .eventlog_algorithm - .digest(init_entry.as_bytes()); - - let mut eventlog = self.eventlog.lock().await; - - self.attester - .extend_runtime_measurement(event_digest, self.config.eventlog_config.init_pcr) - .await - .context("write INIT entry")?; - eventlog.write_log(init_entry).context("write INIT log")?; - Ok(()) } @@ -194,23 +184,30 @@ impl AttestationAPIs for AttestationAgent { content: &str, register_index: Option, ) -> Result<()> { - let register_index = register_index.unwrap_or_else(|| { + let pcr = register_index.unwrap_or_else(|| { let pcr = self.config.eventlog_config.init_pcr; debug!("No PCR index provided, use default {pcr}"); pcr }); - let log_entry = EventEntry::new(domain, operation, content); - let event_digest = log_entry.digest_with(self.config.eventlog_config.eventlog_algorithm); - - let mut eventlog = self.eventlog.lock().await; - - self.attester - .extend_runtime_measurement(event_digest, register_index) - .await?; - - eventlog.write_log(&log_entry.to_string())?; + let content: Content = content.try_into()?; + let log_entry = LogEntry::Event { + domain, + operation, + content, + }; + let alg = self.config.eventlog_config.eventlog_algorithm; + let digest = log_entry.digest_with(alg); + { + // perform atomicly in this block + let mut eventlog = self.eventlog.lock().await; + self.attester + .extend_runtime_measurement(digest, pcr) + .await?; + + eventlog.write_log(&log_entry)?; + } Ok(()) }