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

Race condition with Attestors? #147

Open
hu55a1n1 opened this issue Aug 2, 2024 · 6 comments
Open

Race condition with Attestors? #147

hu55a1n1 opened this issue Aug 2, 2024 · 6 comments
Labels
question Further information is requested

Comments

@hu55a1n1
Copy link
Member

hu55a1n1 commented Aug 2, 2024

Summary

If so, what happens if some other thread also writes to /dev/attestation/user_report_data before we read from /dev/attestation/quote?

Originally posted by @thanethomson in #139 (comment)

@hu55a1n1 hu55a1n1 added the question Further information is requested label Aug 2, 2024
@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Aug 2, 2024

So, whatever is written to /dev/attestation/user_report_data is included (and attested to) in the quote read from /dev/attestation/quote (see REPORTDATA in https://api.trustedservices.intel.com/documents/sgx-attestation-api-spec.pdf). What could happen in theory is we could have an attestation created by a thread that is attesting to the report data from a different thread. Not sure if gramine provides any protection against this.
And gramine does allow you to have multiple threads (using sgx.max_threads in the manifest) although we're using the single-threaded runtime for tokio. So this doesn't seem to be a problem for now but definitely something to keep in mind.

Originally posted by @hu55a1n1 in #139 (comment)

@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Aug 2, 2024

cc: @amiller

@thanethomson
Copy link
Contributor

thanethomson commented Aug 2, 2024

Seems like a simple solution here, to be on the safe side, would be to deliberately serialize all attestation requests coming into the enclave?

@amiller
Copy link
Contributor

amiller commented Aug 2, 2024

This is a good point, i'm pretty sure this attestation flow is indeed not threadsafe. I would guess a lot of handling of sensitive data would need to be revisited if switching to use multiple threads

@dangush
Copy link
Contributor

dangush commented Aug 21, 2024

what's the status on this?

@hu55a1n1
Copy link
Member Author

I think this isn't a major problem now because we're using tokio's single-threaded runtime. So this is something we should revisit post v0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants