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

api_server: add SGX support to Amber AS #159

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

mythi
Copy link
Contributor

@mythi mythi commented Sep 14, 2023

No description provided.

@mythi mythi marked this pull request as ready for review October 10, 2023 06:52
@mythi
Copy link
Contributor Author

mythi commented Oct 10, 2023

the SGX part tested with Amber, @Lu-Biao to help check TDX

@Lu-Biao
Copy link
Contributor

Lu-Biao commented Oct 10, 2023

It works well with TDX. @mythi

@mythi
Copy link
Contributor Author

mythi commented Oct 10, 2023

/cc @Xynnn007 @jialez0 @sameo

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch looks good to me, while we need continue the talk about the architecture of KBS.

}
// get quote
let attestation = serde_json::from_str::<Attestation>(attestation)
.map_err(|e| anyhow!("Deserialize Attestation failed: {:?}", e))?;
let tdx_evidence = serde_json::from_str::<TdxEvidence>(&attestation.tee_evidence)
.map_err(|e| anyhow!("Deserialize TDX Evidence failed: {:?}", e))?;
let evidence = serde_json::from_str::<AmberTeeEvidence>(&attestation.tee_evidence)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd better raise an info or warn here to tell the user that eventlog is ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With amber, event_log needs to be added as a separate field in the appraisal request but it's currently not done so there's no change in functionality with this PR.

@mythi
Copy link
Contributor Author

mythi commented Oct 26, 2023

The patch looks good to me, while we need continue the talk about the architecture of KBS.

can we get it for the release?

@Xynnn007 Xynnn007 merged commit c3bd225 into confidential-containers:main Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants