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.

Sign up for GitHub

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

Jump to bottom

AA | Add Eventlog Recording for Attestation Agent #548

Merged
merged 5 commits into from
Jul 10, 2024
Merged

AA | Add Eventlog Recording for Attestation Agent #548

merged 5 commits into from
Jul 10, 2024

Conversation

Xynnn007
Copy link
Member

@Xynnn007 Xynnn007 commented Apr 23, 2024 •
edited
Loading

Related to/Close #495

This PR

This PR is still a draft. TODOs

  • A specification document to define CoCo event types
  • logic to convert NELR to CELR
  • logic to deliver eventlogs to the verifiers

Needs some comments to ensure we are on the same way.

cc @binxing @mythi

btw @mkulke I am not sure if vTPM crates are ready for extending PCRs now?

@Xynnn007 Xynnn007 force-pushed the aa-eventlog branch 2 times, most recently from b8eb5e6 to 3da8d73 Compare April 23, 2024 09:49
@mkulke
Copy link
Contributor

mkulke commented Apr 23, 2024

btw @mkulke I am not sure if vTPM crates are ready for extending PCRs now?

Not yet, we'll have to add this to the API, since we also need it for Peerpod's initdata

@zvonkok
Copy link
Member

zvonkok commented Apr 29, 2024 •
edited
Loading

@Xynnn007 @fitzthum Are we looking to integrate coconut-svsm? vTPM support was recently merged (SNP). There is some work going on to add TDX-partioning as well.

@fitzthum
Copy link
Member

fitzthum commented Apr 29, 2024 •
edited
Loading

Are we looking to integrate coconut-svsm? vTPM support was recently merged (SNP). There is some work going on to add TDX-partioning as well.

I am open to this. One of my close colleagues has been working on adding vTPM support there. I think the biggest challenge might be supporting all the forks that they currently rely on to get things running. We will also probably need a new vTPM attester/verifier or possibly one for each platform coconut runs on.

arronwy
arronwy reviewed Jun 6, 2024
View reviewed changes
Copy link
Member

@arronwy arronwy left a comment

Choose a reason for hiding this comment

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

Thanks @Xynnn007 , verified with this PR, this feature works well.

attestation-agent/attestation-agent/config.example.json Outdated Show resolved Hide resolved
attestation-agent/attestation-agent/config.example.toml Outdated Show resolved Hide resolved
attestation-agent/attestation-agent/src/config/mod.rs Outdated Show resolved Hide resolved
attestation-agent/attestation-agent/src/config/mod.rs Outdated Show resolved Hide resolved
attestation-agent/attestation-agent/src/lib.rs Show resolved Hide resolved
@Xynnn007 Xynnn007 force-pushed the aa-eventlog branch 2 times, most recently from dd44679 to 2b2502d Compare June 7, 2024 02:03
arronwy
arronwy approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@arronwy arronwy left a comment

Choose a reason for hiding this comment

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

Thanks @Xynnn007 LGTM!

@Xynnn007 Xynnn007 marked this pull request as ready for review June 7, 2024 02:11
@Xynnn007
Copy link
Member Author

Xynnn007 commented Jun 7, 2024

With great help from @arronwy , the whole stream from AA -> AS passed. I will put the AS side PR then.

mythi
mythi reviewed Jun 12, 2024
View reviewed changes
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

I did not do a thorough review yet, just commenting on the TdxEvidence mainly.

attestation-agent/attester/src/tdx/mod.rs Show resolved Hide resolved
attestation-agent/attester/src/tdx/rtmr.rs Outdated Show resolved Hide resolved
@Xynnn007 Xynnn007 mentioned this pull request Jun 12, 2024
Merged
@Xynnn007 Xynnn007 force-pushed the aa-eventlog branch 2 times, most recently from 048580f to 0f9f538 Compare June 20, 2024 08:12
@Xynnn007
Copy link
Member Author

Xynnn007 commented Jul 8, 2024

Rebased to resolve the conflicts. Does this PR now look good?

mkulke
mkulke approved these changes Jul 9, 2024
View reviewed changes
Copy link
Contributor

@mkulke mkulke left a comment

Choose a reason for hiding this comment

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

small nit, otherwise lgtm

attestation-agent/attestation-agent/src/lib.rs Outdated Show resolved Hide resolved
@mkulke mkulke mentioned this pull request Jul 9, 2024
Closed
@mkulke
Copy link
Contributor

mkulke commented Jul 10, 2024

@Xynnn007 can we merge this one? I would start working on a PR for vTPM runtime measurements based on this

Xynnn007 and others added 5 commits July 10, 2024 17:48
@Xynnn007
AA: deprecate unused proto
8062d06 
Signed-off-by: Xynnn007 <[email protected]>
@mkulke @Xynnn007
attestation-agent: init attester in constructor
8fd187a 
Instead of running heuristics every time we invoke an AA function, the
heuristics are invoked initially when the AA instance is created.

This has the upside that we can define per-instance configuration that
will be applied when AA interacts with the TEE.

Signed-off-by: Magnus Kulke <[email protected]>
@Xynnn007
AA: Update extend_runtime_measurement to align with NELR
ade57ef 
Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
AA/attester: fix TDX's RTMR extension
7736977 
Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
Attester/TDX: add AA eventlog support
41a31d1 
Signed-off-by: Xynnn007 <[email protected]>
@mkulke mkulke merged commit 15d85b6 into confidential-containers:main Jul 10, 2024
20 checks passed
@Xynnn007 Xynnn007 deleted the aa-eventlog branch July 10, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@binxing binxing binxing left review comments

@mythi mythi mythi left review comments

@arronwy arronwy arronwy approved these changes

@mkulke mkulke mkulke approved these changes

@jialez0 jialez0 Awaiting requested review from jialez0

@sameo sameo Awaiting requested review from sameo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AA: Measurement Event Log Format
7 participants
@Xynnn007 @mkulke @zvonkok @fitzthum @mythi @arronwy @binxing