gramineproject / gramine

A library OS for Linux multi-process applications, with Intel SGX support
GNU Lesser General Public License v3.0
606 stars 201 forks source link

Nginx test data not getting added as sgx.trusted_files in the manifest.sgx #2016

Closed vasanth-intel closed 1 month ago

vasanth-intel commented 1 month ago

Description of the problem

As part of trusted files commit [LibOS] Move trusted and allowed files logic to LibOS, Nginx Makefile was modified after which we are noticing that all the test data files (install/html/random) are not added as sgx.trusted_files in manifest.sgx file.

The order of components being built within the Makefile seems to be causing the issue, where the test data files are being generated after the manifest is generated. Updating the Makefile to generate the test data before manifest is generated is as shown in the below git diff screenshot. Also, attached are the Makefile outputs with the current master and after applying the below fix where test data is generated before calling the manifest PHONY.

image

Make_Output_After_Fix.txt Make_Output_Current_Master.txt

Steps to reproduce

  1. Git clone gramine repo git clone https://github.com/gramineproject/gramine.git.
  2. Go to CI-Examples/nginx folder.
  3. Execute make SGX=1 command.

Expected results

After issuing make SGX=1 command, the measurements of the test data files are added as part of resulting manifest file.

Actual results

After issuing make SGX=1 command, the measurements of the test data files are not getting added as part of resulting manifest file.

Gramine commit hash

aef087f9c844fbab176333cb76f9fc5d6e04c039

dimakuv commented 1 month ago

The bug is real, but the proposed fix is wrong.

You need a proper dependency: the target "building the .manifest" must depend on "testdata". The fix should be applied here: https://github.com/gramineproject/gramine/blob/988e6b8c82c5e317758f16034199eb13fdc9c2b3/CI-Examples/nginx/Makefile#L53

I think like this:

- nginx.manifest: nginx.manifest.template $(INSTALL_DIR)/sbin/nginx \
+ nginx.manifest: nginx.manifest.template $(INSTALL_DIR)/sbin/nginx testdata \

(But I haven't tested my proposal.)

Anyway, feel free to submit a PR fixing this.

mkow commented 1 month ago

@vasanth-intel: Please don't post screenshots of text, it doesn't make any sense. Just paste the text and use Markdown to format it.