-
Notifications
You must be signed in to change notification settings - Fork 91
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
Generate TLS credentials for user space #749
base: oe_port
Are you sure you want to change the base?
Conversation
Skip generating tls certificate in sw mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor change requested, other than that looks good to me. Christoph needs to review too.
done: | ||
|
||
if (private_key) | ||
oe_free(private_key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use oe_free_key instead
oe_free(private_key); | ||
|
||
if (public_key) | ||
oe_free(public_key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
This PR failed to generate certificate due to openenclave/openenclave#3378. We need to fix OE before merging this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically fine, but likely a layering problem.
|
||
if (!sgxlkl_in_sw_debug_mode()) | ||
{ | ||
enclave_generate_tls_credentials( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a layering problem here; we cant call oe_*
at this point in time anymore. If I'm not mistaken, the consensus was that we would generate this kind of data earlier and then pass it to the application via auxv. This would also mean that applications could pick it up directly from memory instead of a file. The current implementation would also fail on a read-only root file system. (CC @davidchisnall)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this routine is a part of the nominal userspace and will be refactored soon to make that distinction more explicit. We may not call OE APIs from here.
More generally, given that we've had so many meetings discussing the design and the mechanism for passing this via the ELF aux vector, I'd like to understand why a different approach is being pursued here.
#include <stdio.h> | ||
#include <stdlib.h> | ||
|
||
// This function and its caller should be moved to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to wait, we can do this now, can't we?
Generate TLS certificate and private key and expose them to user space through tmpfs. We need this for establishing attested TLS channel with external apps. Currently a few scenario tests also depend on this feature.