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

attester: tdx: make libtdx-attest optional #905

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mythi
Copy link
Contributor

@mythi mythi commented Feb 12, 2025

TDX Guest driver support is problematic: configfs-tsm currently
does not allow to request bare TD reports. It's always a signed
TD quote that needs exits to QGS for signing. However, for the
initdata checks, we only need the TD report.

Linux still supports the /dev/tdx_guest ioctl() for getting the
report. Since the early days, we have used libtdx-attest wrappers
for the ioctls(). However, getting the C libraries and headers
installed in various places has turned out to be problematic.
Furthermore, it isn't even necessary for uses cases like
kbs-client (where only the configfs-tsm GetQuote is needed).

@mythi mythi force-pushed the tdx-attester-3 branch 2 times, most recently from cc1dbf7 to 1795fe9 Compare February 12, 2025 12:56
TDX Guest driver support is problematic: configfs-tsm currently
does not allow to request bare TD reports. It's always a signed
TD quote that needs exits to QGS for signing. However, for the
initdata checks, we only need the TD report.

Linux still supports the /dev/tdx_guest ioctl() for getting the
report. Since the early days, we have used libtdx-attest wrappers
for the ioctls(). However, getting the C libraries and headers
installed in various places has turned out to be problematic.
Furthermore, it isn't even necessary for uses cases like
kbs-client (where only the configfs-tsm GetQuote is needed).

Move the get_report() method to use raw ioctls() using iocuddle.
This follows the sev crate implementation and which turns out to
be very simple to adopt here too.

Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
For the use-cases needed by CoCo and supported by upstream Linux,
we don't need libtdx-attest so make the dependency a build-time
opt-in feature.

Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
@mythi mythi marked this pull request as ready for review February 14, 2025 07:45
@mythi mythi requested a review from a team as a code owner February 14, 2025 07:45
For configurations where the out-of-tree RTMR extend ioctl()
is available and/or the libtdx-attest supported way to get the
TD quote is needed, add a customized tdx-attester feature that
supports this.

Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

LGTM but one nit about the name of the feature

@@ -76,6 +76,7 @@ all-attesters = [
"cca-attester",
]
tdx-attester = ["kbs_protocol?/tdx-attester", "attester/tdx-attester"]
tdx-attester-oot = ["kbs_protocol?/tdx-attester-oot", "attester/tdx-attester", "attester/tdx-attest-dcap-ioctls"]
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment about what this is. Is oot the best name for the feature? This is referring to a non-upstream kernel? Maybe we should just call it -libtdx or something.

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