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

Makefile build flag is not passed inside enclave-runtime #3138

Merged
merged 3 commits into from
Oct 20, 2024

Conversation

BillyWooo
Copy link
Collaborator

Small, but very important PR.
We all overlooked this 😿

I would suggest to make a following PR to remove all IAS related code. There won't be different mode, but only dcap.

@BillyWooo BillyWooo requested review from kziemianek, Kailai-Wang and a team October 18, 2024 21:24
Copy link

linear bot commented Oct 18, 2024

@Kailai-Wang
Copy link
Collaborator

I don't fully understand this, can you please elaborate why this is the root cause for SGX crash?

It seems that we indeed haven't applied dcap feature for enclave compilation, but questions are:

  1. why did it always work locally in your case? Allegedly it was verified multiple times
  2. from the code - this should affect TLS server/client only - so primarily multi-worker situation. How is it related to the enclave registration and crash?

@Kailai-Wang
Copy link
Collaborator

cc @kziemianek as you once made some changes to enclave-runtime/Makefile too

@BillyWooo
Copy link
Collaborator Author

I don't fully understand this, can you please elaborate why this is the root cause for SGX crash?

It seems that we indeed haven't applied dcap feature for enclave compilation, but questions are:

  1. why did it always work locally in your case? Allegedly it was verified multiple times
  2. from the code - this should affect TLS server/client only - so primarily multi-worker situation. How is it related to the enclave registration and crash?

The crash will only be triggered from “create_ra_report_and_signature”, which won't be executed during initialization.

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thank you!

Shall we apply the same change to bc-worker too?

@Kailai-Wang
Copy link
Collaborator

@m1iktea please block external access to port 2001 and 3443 for identity-worker, and 4545 too if you don't use it for healthcheck.

It will help us not accept unneeded connections...

@BillyWooo
Copy link
Collaborator Author

Thank you!

Shall we apply the same change to bc-worker too?

Yes. We should. Let me make a separate PR for that.
I would like to merge and get new id-worker release out ASAP.

@BillyWooo BillyWooo merged commit f4731f6 into dev Oct 20, 2024
20 checks passed
@BillyWooo BillyWooo deleted the p-1105-sgxsgx_error_enclave_crashed branch October 20, 2024 09:00
@BillyWooo BillyWooo self-assigned this Oct 20, 2024
@m1iktea
Copy link
Collaborator

m1iktea commented Oct 21, 2024

@m1iktea please block external access to port 2001 and 3443 for identity-worker, and 4545 too if you don't use it for healthcheck.

It will help us not accept unneeded connections...

I have disabled public network access in the firewall, but I did not remove these two ports from the startup parameters.

BillyWooo added a commit that referenced this pull request Oct 21, 2024
* build flag is not passed inside enclave-runtime; add some log info

* typo
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