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

tdx: sgx: Bump DCAP dependency #398

Merged
merged 1 commit into from
May 28, 2024

Conversation

fidencio
Copy link
Member

@fidencio fidencio commented May 28, 2024

Background motivation: Building Trustee on CentOS 9 stream will break, due to an incompatibility of rust-bindgen and clang, as shown by the error below:

  clang diag: warning: .: 'linker' input unused [-Wunused-command-line-argument]
  thread 'main' panicked at /home/cloud-user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.60.1/src/ir/context.rs:861:9:
  "_sgx_ql_qv_supplemental_t_union_(anonymous_at_/usr/x86_64-intel-sgx/include/sgx_qve_header_h_95_5)" is not a valid Ident
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
make: *** [Makefile:22: grpc-as] Error 101

The rust-bindgen version causing this issue was coming from the SGXDataCenterAttestationPrimitives repo. With that in mind, and considering that the DCAP version used so far is 1+ years old, let's bump it and solve those two issues at the same time.

Unfortunately, we're also adapting the code a little bit due to the API changes between 1.16 and 1.21.

Background motivation: Building Trustee on CentOS 9 stream will break,
due to an incompatibility of rust-bindgen and clang, as shown by the
error below:
```
  clang diag: warning: .: 'linker' input unused [-Wunused-command-line-argument]
  thread 'main' panicked at /home/cloud-user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.60.1/src/ir/context.rs:861:9:
  "_sgx_ql_qv_supplemental_t_union_(anonymous_at_/usr/x86_64-intel-sgx/include/sgx_qve_header_h_95_5)" is not a valid Ident
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
make: *** [Makefile:22: grpc-as] Error 101
```

The rust-bindgen version causing this issue was coming from the
SGXDataCenterAttestationPrimitives repo.  With that in mind, and
considering that the DCAP version used so far is 1+ years old, let's
bump it and solve those two issues at the same time.

Unfortunately, we're also adapting the code a little bit due to the API
changes between 1.16 and 1.21.

Signed-off-by: Fabiano Fidêncio <[email protected]>
@fidencio fidencio force-pushed the topic/update-dcap branch from da7041f to 63c02d6 Compare May 28, 2024 12:21
@fidencio fidencio added the test_e2e Authorize TEE e2e test run label May 28, 2024
@fidencio fidencio requested review from mkulke, mythi and Xynnn007 May 28, 2024 12:22
@fidencio fidencio marked this pull request as ready for review May 28, 2024 13:18
@fidencio fidencio requested a review from sameo as a code owner May 28, 2024 13:18
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.

lgtm

@fitzthum
Copy link
Member

We use tdx-attest-rs = { git = "https://github.com/intel/SGXDataCenterAttestationPrimitives", tag = "DCAP_1.20", optional = true } in the attester. I am assuming the newer DCAP will still be able to consume evidence from the older one.

Btw when you say updating the container image, which one are you referring to?

@fidencio
Copy link
Member Author

We use tdx-attest-rs = { git = "https://github.com/intel/SGXDataCenterAttestationPrimitives", tag = "DCAP_1.20", optional = true } in the attester. I am assuming the newer DCAP will still be able to consume evidence from the older one.

I hope so, but I think we should do the bump there as well and be consistent. WDYT?

Btw when you say updating the container image, which one are you referring to?

Forget about that, I thought that I would also need to update clang / libclang, but that was not needed (and I ended up forgetting to remove the message from the PR).

@mythi
Copy link
Contributor

mythi commented May 28, 2024

We use tdx-attest-rs = { git = "https://github.com/intel/SGXDataCenterAttestationPrimitives", tag = "DCAP_1.20", optional = true } in the attester

It's no longer used to generate the quote.

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

Guest-side could probably use fixup too.

@fidencio fidencio merged commit 4b0b47f into confidential-containers:main May 28, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_e2e Authorize TEE e2e test run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants