-
Notifications
You must be signed in to change notification settings - Fork 48
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
Fix broken sigining of EXT2 rootfs #1456
Conversation
713fd51
to
4f81bb6
Compare
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.
Thank you for fixing this!
@@ -440,10 +443,28 @@ int _sign(int argc, const char* argv[]) | |||
assert(myst_validate_file_path(program_file)); | |||
assert(myst_validate_file_path(temp_oeconfig_file)); | |||
|
|||
/* if not a CPIO archive, create a zero-filled file with one page */ | |||
if (myst_cpio_test(rootfs) == -ENOTSUP) |
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 we check roothashes_opt
for a quicker determination?
a39343f
to
c744062
Compare
@@ -0,0 +1,56 @@ | |||
TOP=$(abspath ../..) |
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.
This test should live in the tests/myst directory with the other signing tests
58b9dde
to
7ec1638
Compare
@@ -28,7 +28,9 @@ package.pem: | |||
|
|||
package: package.pem ext2rootfs | |||
echo "Generating a signed package" | |||
@myst package-sgx --roothash=roothash appdir package.pem ../config.json | |||
@myst package-sgx --roothash=roothash package.pem ../config.json |
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.
What does it mean when both roothash and appdir are present in args? Should this be prevented(we should not change what we have, just wondering..)
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.
The TEE_aware sample also takes appdir as a command line arg
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.
Thanks for the fix @mikbras
Signing of EXT2 images (using
myst sign
) was copying the entire EXT2 rootfs into the enclave image rather than a stub.Added
./tests/sign
to verify signing of EXT4-based images. Added negative test too.