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

In production, generate TDX quote using configfs-tsm #1041

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

ameba23
Copy link
Contributor

@ameba23 ameba23 commented Sep 7, 2024

This adds TDX quote generation using configfs-tsm, a linux filesystem interface to generate TDX quotes in production.

If we want to run the TS server on non-production hardware, we have to use either test mode or enable the unsafe feature flag, or attestation will fail.

This uses the configfs-tsm crate which is only a few lines and has no dependencies. There is still some work to be done on it but the api used here should stay the same. I am considering moving that code into the tdx-quote crate, to avoid needing to manage yet another crate.

Copy link
Collaborator

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Looks fine. I took a quick look through the crate and it does look simple, so maybe worth rolling into tdx-quote until we feel like it needs to be its own thing

@@ -72,6 +72,7 @@ sha2 ="0.10.8"
hkdf ="0.12.4"
project-root ={ version="0.2.2", optional=true }
tdx-quote ={ git="https://github.com/entropyxyz/tdx-quote", optional=true, features=["mock"] }
configfs-tsm ={ git="https://github.com/entropyxyz/configfs-tsm" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would pin this to a specific commit hash or tag

Copy link
Member

@JesseAbram JesseAbram left a comment

Choose a reason for hiding this comment

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

looks good but can we make a new flag and put all the tdx stuff behind it?

@ameba23
Copy link
Contributor Author

ameba23 commented Sep 12, 2024

looks good but can we make a new flag and put all the tdx stuff behind it?

Personally, i think the default build should be a production build, and if you want unsafe testing features, such as making mock quotes, you should have to specify that, which is currently the case.

I'm willing to do it the other way around - meaning default build makes mock quotes and production build needs a production feature to be enabled. But i want a thumbs up from both of you on that. We also need to think about our release scripts - probably we want a production release and a testnet version release.

@HCastano
Copy link
Collaborator

@ameba23

Personally, i think the default build should be a production build, and if you want unsafe testing features, such as making mock quotes, you should have to specify that, which is currently the case.

I'm willing to do it the other way around - meaning default build makes mock quotes and production build needs a production feature to be enabled. But i want a thumbs up from both of you on that. We also need to think about our release scripts - probably we want a production release and a testnet version release.

Until we get a good testing and release pipeline with TDX hardware we should have the default be non-TDX.

@ameba23
Copy link
Contributor Author

ameba23 commented Sep 17, 2024

@ameba23

Personally, i think the default build should be a production build, and if you want unsafe testing features, such as making mock quotes, you should have to specify that, which is currently the case.
I'm willing to do it the other way around - meaning default build makes mock quotes and production build needs a production feature to be enabled. But i want a thumbs up from both of you on that. We also need to think about our release scripts - probably we want a production release and a testnet version release.

Until we get a good testing and release pipeline with TDX hardware we should have the default be non-TDX.

Ok i will make a production feature flag. But will wait until my other open PR is done to avoid conflicts.

@ameba23
Copy link
Contributor Author

ameba23 commented Sep 20, 2024

I have added a production flag, and now if that flag is not present mock quotes will always be used. CI is passing so i am gonna go ahead and hit merge. Lemme know if you(s) are not happy with how that flag is set up.

@ameba23 ameba23 merged commit 590305f into master Sep 20, 2024
8 checks passed
@ameba23 ameba23 deleted the peg/tdx-quote-generation branch September 20, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants