Skip to content

attester: tdx-attester: extend RTMRs #1025

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mythi
Copy link
Contributor

@mythi mythi commented Jun 13, 2025

Linux 6.16 adds the RTMR ABI that lets userspace to extend TEE RTMRs. Initially the support is added for TDX and follows https://github.com/torvalds/linux/blob/master/Documentation/ABI/testing/sysfs-devices-virtual-misc-tdx_guest

Change tdx-attester to use the ABI defined by the spec to do RTMR extend.

TODO:

  • testing

@mythi mythi force-pushed the tdx-attester-rtmr branch from 3def66c to 7b31dd0 Compare June 13, 2025 10:53
@Xynnn007
Copy link
Member

Nice to see that we are using the new feature of kernel. One thing not clear to me is whether we have a plan to update the kernel version of kata-side? https://github.com/kata-containers/kata-containers/blob/main/versions.yaml#L199

@mythi
Copy link
Contributor Author

mythi commented Jun 16, 2025

I'm going to make the version bump once 6.16 is released

@mythi mythi force-pushed the tdx-attester-rtmr branch from 7b31dd0 to 82be898 Compare June 18, 2025 05:20
@mythi mythi marked this pull request as ready for review June 18, 2025 05:20
@mythi mythi requested a review from a team as a code owner June 18, 2025 05:20
fn runtime_measurement_extend_available() -> bool {
if Path::new("/sys/kernel/config/tsm/report").exists() {
if Path::new(TDX_RTMR_PATH).exists() {
Copy link
Member

Choose a reason for hiding this comment

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

Will this influence scenarios where we are using ioctl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I was not thinking the logic here very clearly. will re-work a bit.

Comment on lines 193 to 205
#[cfg(not(feature = "tdx-attest-dcap-ioctls"))]
std::fs::write(
Path::new(TDX_RTMR_PATH).join(format!("rtmr{rtmr_index}:sha384")),
extend_data,
)
.context("TDX Attester: failed to extend RTMR")?;

Copy link
Member

Choose a reason for hiding this comment

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

I found that we have multiple cfg conditions here for ioctl/non-ioctl. Could you help to make two separate same-name functions and just call it here but enable one of them with feature flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll re-think this along with the updated runtime_measurement_extend_available() logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Xynnn007 do you think it's OK as it is now?

Linux 6.16 adds the RTMR ABI that lets userspace to extend
TEE RTMRs. Initially the support is added for TDX and follows
https://github.com/torvalds/linux/blob/master/Documentation/ABI/testing/sysfs-devices-virtual-misc-tdx_guest

Change tdx-attester to use the ABI defined by the spec to do
RTMR extend.

Signed-off-by: Mikko Ylinen <[email protected]>
@mythi mythi force-pushed the tdx-attester-rtmr branch from 82be898 to a508662 Compare June 18, 2025 10:13
@fitzthum
Copy link
Member

I think we should also provide an option for people to use this for bind_init_data but that could potentially be future work.

@mythi
Copy link
Contributor Author

mythi commented Jun 18, 2025

I think we should also provide an option for people to use this for bind_init_data but that could potentially be future work.

I might wait with that a bit but yeah it's in the plans to move away from the TDREPORT ioctl to just read mrconfigid from this sysfs interface.

Comment on lines +197 to +205
let rtmr_path = Path::new(TDX_RTMR_PATH);
if rtmr_path.exists() {
std::fs::write(
rtmr_path.join(format!("rtmr{rtmr_index}:sha384")),
extend_data,
)
.context("TDX Attester: failed to extend RTMR")?;
}

Copy link
Member

Choose a reason for hiding this comment

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

If we are using tdx-attest-dcap-ioctls this code will also be executed, which is not expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is expected: I think it's harmless to check the path exists (kinda what we do with TSM reports too)

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 use also feature flag for this. A corner case is with io-ctl but coincidently the path exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, I thought about that corner base but that's almost impossible. Anyway, I'll rework on this further after my break...

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.

3 participants