Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added checks for AS and KBS policy setting #617

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions attestation-service/src/policy_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub enum PolicyError {
Base64DecodeFailed(#[from] base64::DecodeError),
#[error("Illegal policy id. Only support alphabet, numeric, `-` or `_`")]
InvalidPolicyId,
#[error("Illegal policy: {0}")]
InvalidPolicy(#[source] anyhow::Error),
#[error("Failed to load reference data: {0}")]
LoadReferenceDataFailed(#[source] anyhow::Error),
#[error("Failed to set input data: {0}")]
Expand Down
10 changes: 10 additions & 0 deletions attestation-service/src/policy_engine/opa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ impl PolicyEngine for OPA {
return Err(PolicyError::InvalidPolicyId);
}

// Check if the policy is valid
{
let policy_content = String::from_utf8(policy_bytes.clone())
.map_err(|e| PolicyError::InvalidPolicy(e.into()))?;
let mut engine = regorus::Engine::new();
engine
.add_policy(policy_id.clone(), policy_content)
.map_err(PolicyError::InvalidPolicy)?;
}

let mut policy_file_path = PathBuf::from(
&self
.policy_dir_path
Expand Down
6 changes: 4 additions & 2 deletions deps/verifier/src/snp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const LOADER_SPL_OID: Oid<'static> = oid!(1.3.6 .1 .4 .1 .3704 .1 .3 .1);
const KDS_CERT_SITE: &str = "https://kdsintf.amd.com";
const KDS_VCEK: &str = "/vcek/v1";

/// Attestation report versions supported
/// Attestation report versions supported
const REPORT_VERSION_MIN: u32 = 2;
const REPORT_VERSION_MAX: u32 = 3;

Expand Down Expand Up @@ -110,7 +110,9 @@ impl Verifier for Snp {

// See Trustee Issue#589 https://github.com/confidential-containers/trustee/issues/589
if report.version < REPORT_VERSION_MIN || report.version > REPORT_VERSION_MAX {
return Err(anyhow!("Unexpected attestation report version. Check SNP Firmware ABI specification"));
return Err(anyhow!(
"Unexpected attestation report version. Check SNP Firmware ABI specification"
));
}

if report.vmpl != 0 {
Expand Down
3 changes: 3 additions & 0 deletions kbs/src/policy_engine/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ pub enum KbsPolicyEngineError {

#[error("Set Policy request is illegal for {0}")]
IllegalSetPolicyRequest(&'static str),

#[error("Failed to set policy, illegal policy: {0}")]
InvalidPolicy(#[source] anyhow::Error),
}
24 changes: 17 additions & 7 deletions kbs/src/policy_engine/opa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ impl PolicyEngineInterface for Opa {
async fn set_policy(&mut self, policy: &str) -> Result<(), KbsPolicyEngineError> {
let policy_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(policy)?;

// Check if the policy is valid
{
let policy_content = String::from_utf8(policy_bytes.clone())
.map_err(|e| KbsPolicyEngineError::InvalidPolicy(e.into()))?;
let mut engine = regorus::Engine::new();
engine
.add_policy(String::from("default"), policy_content)
.map_err(KbsPolicyEngineError::InvalidPolicy)?;
}

tokio::fs::write(&self.policy_path, policy_bytes).await?;

Ok(())
Expand Down Expand Up @@ -153,6 +163,13 @@ mod tests {
res.err().unwrap(),
KbsPolicyEngineError::IOError(_)
));

// Illegal policy
let res = set_policy_from_file(&mut opa, "test/data/policy_invalid_1.rego").await;
assert!(matches!(
res.err().unwrap(),
KbsPolicyEngineError::InvalidPolicy(_)
));
}

#[rstest]
Expand All @@ -167,13 +184,6 @@ mod tests {
1,
Err(KbsPolicyEngineError::ResourcePathError)
)]
#[case(
"test/data/policy_invalid_1.rego",
"my_repo/Alice/key",
"Alice",
1,
Err(KbsPolicyEngineError::PolicyLoadError)
)]
#[case(
"test/data/policy_invalid_2.rego",
"my_repo/Alice/key",
Expand Down
Loading