From 26465e37d71e1a1c2661b3f2a4b09c6b2adf7444 Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Mon, 18 Nov 2024 14:50:17 +0100 Subject: [PATCH] rust/pvattest: Secret-store hash check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a check to verify the hash over the Secret Store in the guest UV storage. During 'create' the user can request that hash via a flag. During 'check' the user specifies the Add Secret requests and check whether the store is locked. If the calculated hash over this state matches the one reported by attestation, this check is successful. Reviewed-by: Jan Höppner Signed-off-by: Steffen Eiden --- rust/Cargo.lock | 1 + rust/pvattest/Cargo.toml | 1 + rust/pvattest/src/cli.rs | 31 +++++ rust/pvattest/src/cmd/check.rs | 15 ++- rust/pvattest/src/cmd/check/secret_store.rs | 129 ++++++++++++++++++++ rust/pvattest/src/cmd/create.rs | 1 + rust/pvattest/tests/assets/asrcb | 1 + 7 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 rust/pvattest/src/cmd/check/secret_store.rs create mode 120000 rust/pvattest/tests/assets/asrcb diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 8ec16c3e..6fbd8ade 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -459,6 +459,7 @@ dependencies = [ "clap", "clap_complete", "log", + "openssl", "s390_pv", "serde", "serde_yaml", diff --git a/rust/pvattest/Cargo.toml b/rust/pvattest/Cargo.toml index c46159ca..22ec5325 100644 --- a/rust/pvattest/Cargo.toml +++ b/rust/pvattest/Cargo.toml @@ -16,6 +16,7 @@ log = { version = "0.4.6", features = ["std", "release_max_level_debug"] } serde_yaml = "0.9" serde = { version = "1.0.139", features = ["derive"]} zerocopy = { version="0.7", features = ["derive"] } +openssl = "0.10.57" pv = { path = "../pv", package = "s390_pv" } utils = { path = "../utils" } diff --git a/rust/pvattest/src/cli.rs b/rust/pvattest/src/cli.rs index 96867156..f44b127d 100644 --- a/rust/pvattest/src/cli.rs +++ b/rust/pvattest/src/cli.rs @@ -99,6 +99,9 @@ pub enum AttAddFlags { /// Request the public host-key-hash of the key that decrypted the attestation request as /// additional-data. PhkhAtt, + + /// Request a hash over all successful Add-secret requests and the lock state as additional-data. + SecretStoreHash, } // all members s390x only @@ -255,6 +258,34 @@ pub struct CheckOpt { /// Check if the provided user data matches the data from the attestation response. #[arg(short, long, value_name = "FILE", value_hint = ValueHint::FilePath,)] pub user_data: Option, + + /// Use FILE to include as successful Add-secret request. + /// + /// Checks if the Attestation response contains the hash of all specified add secret + /// requests-tags. + /// The hash is sensible to the order in which the secrets where added. This means that if the + /// order of adding here different from the order the add-secret requests where sent to the UV + /// this check will fail even though the same secrets are included in the UV secret store. + /// Can be specified multiple times. + #[arg( + long, + value_name = "FILE", + value_hint = ValueHint::FilePath, + use_value_delimiter = true, + value_delimiter = ',', + requires("secret_store_locked"), + )] + pub secret: Vec, + + /// Check whether the guests secret store is locked or not. + /// + /// Compares the hash of the secret store state to the one calculated by this option and + /// optionally specified add-secret-requests. If the attestation response does not contain a + /// secret store hash, this check fails. + /// + /// Required if add-secret-requests are specified. + #[arg(long, value_name = "BOOL")] + pub secret_store_locked: Option, } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)] diff --git a/rust/pvattest/src/cmd/check.rs b/rust/pvattest/src/cmd/check.rs index 6f340af4..eb6b0663 100644 --- a/rust/pvattest/src/cmd/check.rs +++ b/rust/pvattest/src/cmd/check.rs @@ -3,9 +3,16 @@ // Copyright IBM Corp. 2024 mod host_key; +mod secret_store; -use self::host_key::{host_key_check, HostKeyCheck}; -use crate::{additional::AttestationResult, cli::CheckOpt, exchange::ExchangeFormatResponse}; +use self::{ + host_key::{host_key_check, HostKeyCheck}, + secret_store::SecretStoreCheck, +}; +use crate::{ + additional::AttestationResult, cli::CheckOpt, cmd::check::secret_store::secret_store_check, + exchange::ExchangeFormatResponse, +}; use anyhow::Result; use log::{debug, info, warn}; use pv::{ @@ -89,6 +96,8 @@ pub struct CheckResult<'a> { attest_host_key: HostKeyCheck<'a>, #[serde(skip_serializing_if = "Option::is_none")] user_data: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + secret_store: Option>, } /// Perform the policy checks @@ -107,6 +116,7 @@ pub fn check(opt: &CheckOpt) -> Result { .unwrap(); let user_data = user_data_check(opt, &att_res)?.check(&mut issues); + let secret_store = secret_store_check(opt, &att_res)?.check(&mut issues); let res = CheckResult { successful: !issues.is_empty(), @@ -114,6 +124,7 @@ pub fn check(opt: &CheckOpt) -> Result { image_host_key, attest_host_key, user_data, + secret_store, }; debug!("res {res:?}"); diff --git a/rust/pvattest/src/cmd/check/secret_store.rs b/rust/pvattest/src/cmd/check/secret_store.rs new file mode 100644 index 00000000..63519eab --- /dev/null +++ b/rust/pvattest/src/cmd/check/secret_store.rs @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MIT +// +// Copyright IBM Corp. 2024 + +use std::path::{Path, PathBuf}; + +use anyhow::Result; +use log::info; +use openssl::hash::DigestBytes; +use openssl::hash::{hash, MessageDigest}; +use pv::{misc::read_file, secret::AddSecretRequest}; +use serde::Serialize; + +use super::{bail_check, CheckState}; +use crate::{additional::AttestationResult, cli::CheckOpt}; + +#[derive(Debug, Serialize)] +pub struct SecretStoreCheck<'a> { + add_secret_requests: &'a [PathBuf], + locked: bool, +} + +const REQUEST_TAG_SIZE: usize = 16; + +fn secret_store_hash>(asrcbs: &[A], locked: bool) -> Result { + let mut requests = Vec::with_capacity(asrcbs.len() * REQUEST_TAG_SIZE + 1); + for asrcb in asrcbs { + let asrcb = read_file(asrcb, "Add-secret request")?; + let mut tag = AddSecretRequest::bin_tag(&asrcb)?; + requests.append(&mut tag); + } + requests.push(locked as u8); + Ok(hash(MessageDigest::sha512(), &requests)?) +} + +pub fn secret_store_check<'a>( + opt: &'a CheckOpt, + att_res: &AttestationResult, +) -> Result>> { + // The locked flag is the feature gate of this check + let locked = match opt.secret_store_locked { + None => return Ok(CheckState::None), + Some(state) => state, + }; + + let att_store_hash = match att_res + .add_fields + .as_ref() + .and_then(|add| add.secret_store_hash()) + { + Some(h) => h, + None => bail_check!( + "The Attestation response contains no secret-store-hash, but checking was enabled" + ), + }; + + if secret_store_hash(&opt.secret, locked)?.as_ref() != att_store_hash.as_ref() { + bail_check!("The calculated secret-store-hash does not match with the provided hash"); + } + info!("✓ Secret Store hash"); + + Ok(CheckState::Data(SecretStoreCheck { + add_secret_requests: &opt.secret, + locked, + })) +} +#[cfg(test)] +mod test { + use super::secret_store_hash; + + const ASRCB_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/assets/asrcb"); + + #[test] + fn hash() { + let asrcbs = [ + format!("{ASRCB_DIR}/assoc_derived_default_cuid_one"), + format!("{ASRCB_DIR}/assoc_simple_default_cuid_one"), + format!("{ASRCB_DIR}/null_none_default_cuid_one"), + format!("{ASRCB_DIR}/null_none_default_ncuid_one"), + format!("{ASRCB_DIR}/null_simple_default_cuid_one"), + format!("{ASRCB_DIR}/assoc_none_default_cuid_one"), + format!("{ASRCB_DIR}/null_derived_default_cuid_one"), + format!("{ASRCB_DIR}/null_none_default_cuid_seven"), + format!("{ASRCB_DIR}/null_none_dump_cuid_one"), + ]; + let hash = secret_store_hash(&asrcbs, true).unwrap(); + let exp = [ + 0xd0, 0x48, 0x70, 0x2b, 0x4a, 0x79, 0x47, 0x8b, 0x98, 0x5e, 0x92, 0xe7, 0xed, 0xff, + 0x45, 0x3f, 0x63, 0xf2, 0x4, 0x4e, 0x7d, 0x72, 0xfa, 0xf1, 0x2e, 0xfd, 0x2e, 0xae, + 0xa0, 0xcd, 0x5, 0x5, 0x55, 0x9e, 0xd6, 0x66, 0x5a, 0x6, 0xf6, 0xb4, 0xf9, 0xc6, 0xfc, + 0xf9, 0xf2, 0x96, 0xe, 0x6c, 0xd3, 0xc3, 0xcc, 0x8c, 0xaf, 0xe5, 0x7a, 0xc8, 0x40, + 0x6e, 0x61, 0x6b, 0xf9, 0x52, 0x95, 0x17, + ]; + assert_eq!(&exp, hash.as_ref()); + + let hash = secret_store_hash(&asrcbs, false).unwrap(); + let exp = [ + 0x51, 0xce, 0x62, 0xaf, 0x1f, 0x67, 0xb9, 0xe3, 0x25, 0x4b, 0x18, 0x4e, 0x33, 0xb2, + 0xaa, 0xd3, 0x10, 0x7, 0x58, 0x1a, 0x39, 0xe9, 0x9c, 0xde, 0xb0, 0x29, 0x98, 0xa3, + 0xb6, 0x7f, 0xf4, 0x56, 0xc4, 0x4a, 0x5, 0xee, 0x7d, 0x68, 0xe2, 0x4d, 0xfd, 0x43, + 0x6f, 0x2b, 0xe4, 0xc1, 0xe9, 0xf5, 0xc1, 0x1, 0x64, 0x68, 0xda, 0x64, 0x1, 0x5e, 0x9f, + 0x9f, 0xa3, 0x15, 0x6e, 0x11, 0xd, 0x6c, + ]; + assert_eq!(&exp, hash.as_ref()); + } + + #[test] + fn hash_empty() { + let hash = secret_store_hash::<&str>(&[], true).unwrap(); + let exp = [ + 0x7b, 0x54, 0xb6, 0x68, 0x36, 0xc1, 0xfb, 0xdd, 0x13, 0xd2, 0x44, 0x1d, 0x9e, 0x14, + 0x34, 0xdc, 0x62, 0xca, 0x67, 0x7f, 0xb6, 0x8f, 0x5f, 0xe6, 0x6a, 0x46, 0x4b, 0xaa, + 0xde, 0xcd, 0xbd, 0x0, 0x57, 0x6f, 0x8d, 0x6b, 0x5a, 0xc3, 0xbc, 0xc8, 0x8, 0x44, 0xb7, + 0xd5, 0xb, 0x1c, 0xc6, 0x60, 0x34, 0x44, 0xbb, 0xe7, 0xcf, 0xcf, 0x8f, 0xc0, 0xaa, + 0x1e, 0xe3, 0xc6, 0x36, 0xd9, 0xe3, 0x39, + ]; + assert_eq!(&exp, hash.as_ref()); + + let hash = secret_store_hash::<&str>(&[], false).unwrap(); + let exp = [ + 0xb8, 0x24, 0x4d, 0x2, 0x89, 0x81, 0xd6, 0x93, 0xaf, 0x7b, 0x45, 0x6a, 0xf8, 0xef, + 0xa4, 0xca, 0xd6, 0x3d, 0x28, 0x2e, 0x19, 0xff, 0x14, 0x94, 0x2c, 0x24, 0x6e, 0x50, + 0xd9, 0x35, 0x1d, 0x22, 0x70, 0x4a, 0x80, 0x2a, 0x71, 0xc3, 0x58, 0xb, 0x63, 0x70, + 0xde, 0x4c, 0xeb, 0x29, 0x3c, 0x32, 0x4a, 0x84, 0x23, 0x34, 0x25, 0x57, 0xd4, 0xe5, + 0xc3, 0x84, 0x38, 0xf0, 0xe3, 0x69, 0x10, 0xee, + ]; + assert_eq!(&exp, hash.as_ref()); + } +} diff --git a/rust/pvattest/src/cmd/create.rs b/rust/pvattest/src/cmd/create.rs index e32007d0..4e797530 100644 --- a/rust/pvattest/src/cmd/create.rs +++ b/rust/pvattest/src/cmd/create.rs @@ -21,6 +21,7 @@ fn flags(cli_flags: &[AttAddFlags]) -> AttestationFlags { match flag { AttAddFlags::PhkhImg => att_flags.set_image_phkh(), AttAddFlags::PhkhAtt => att_flags.set_attest_phkh(), + AttAddFlags::SecretStoreHash => att_flags.set_secret_store_hash(), } } att_flags diff --git a/rust/pvattest/tests/assets/asrcb b/rust/pvattest/tests/assets/asrcb new file mode 120000 index 00000000..bc7a2a86 --- /dev/null +++ b/rust/pvattest/tests/assets/asrcb @@ -0,0 +1 @@ +../../../pv/tests/assets/exp/asrcb \ No newline at end of file