-
Notifications
You must be signed in to change notification settings - Fork 0
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
[RISCV] Added TPM DRoT support with Tychools verification #96
base: main
Are you sure you want to change the base?
Conversation
Hello Alexandre (@alex-douk) I left some comments on specific files, can you please take a look? :) Further, two things: (I am not sure what the QEMU test here is - the one that's failing, maybe @CharlyCst can provide more insights.) Once these are all resolved, I believe it's ready for merging. :) |
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 guess this one hasn't been modified since being fetched from simple enclave. ^ ^
Maybe it could be edited to replace the simple enclave explanation with rot enclave explanation and same for the sample output too?
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.
Can you also add the sample output for when TPM fields are set to zero?
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.
We generally don't include the binaries and they are rebuilt locally by everyone so we may not end up facing any environment configuration/portability issues ^ ^
Could we please remove these? :)
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.
Line 28: can we restore the path as "qemu/" so that we have the general build command?
Line 154: Similarly, the bear command, can we restore the original one and you can add a comment of what problems you observed with the bear setup and put the existing line 154 in the comment as which command to use instead when bear doesn't work as expected?
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.
For the TPM device option to QEMU, can we define a tpm-dev (similar to dev-riscv}?
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 also see some lines removed during the testing. Is that intentional?
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.
Can we just put a todo here before line 128 to specify the impl. using registers to retrieve the report and that ideally we would do it with 1 call via some shared memory? (Or if this description already exists in the monitor or somewhere else, can you please add a reference to that?)
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.
Since the enclave_attestation_size call is only used by the enclave_rot_example, it doesn't have the support to be called using the tyche driver (so basically it's not exposed as part of the API through the driver).
Can we add a note here to specify its limited purpose and implementation to avoid confusion?
Our RC frame were never de-allocated, causing memory leaks. With this change I run the creation micro-benchmark with 20000 enclaves without running out of memory.
The test that fails is due to our server not responding, it needs to be fixed when someone gets some free time. |
Summary
Tyche can now share with the application (and by extent, Tychools) an attestation report dynamically generated by a platform TPM. A modifiedOpenSBI version is responsible for the DRTM operations, and passes the report to Tyche through the manifest. We require an augmented version of QEMU to be able to leverage this feature.
OpenSBI changes and QEMU changes are documented in the tyche-stage1-opensbi repo. OpenSBI changes are on branch
alex-rot
, and QEMU changes are on branchqemu_modified_files
. The QEMU branch only contains the modified files from QEMU 8.1.Tyche changes
We modified the following :
crates/attestation/src/signature.rs
: We added fields to capture the TPM-related information in theEnclaveReport
data structure. These fields are : RSA-3072 modulus (384 bytes), RSA-3072 signature (384 bytes), and the TPM attestation per se (129 bytes).monitor/tyche/src/calls.rs
: Added aENCLAVE_ATTESTATION_SIZE
call. This was added because the TPM attestation parameters (and therefore size) can be reconfigured.monitor/tyche/src/riscv/riscv_tpm_attestation.rs
: New file that contains theENCLAVE_ATTESTATION
Tyche call handler. As the TPM attestation can change, we thought it best to abstract away how it is parsed and passed to the application.monitor/tyche/src/riscv/guest.rs
: See above. We also added a handler for theENCLAVE_ATTESTATION_SIZE
that just passes the size of the augmentedEnclaveReport
struct.monitor/tyche/src/attestation_domain.rs
: Added the TPM-related fields in the enclave report creation.The default behavior in case of absence of a TPM on the platform is that the TPM-related fields are still passed to the application, but are all initialized to 0.
New example
In order to demonstrate the above capabilities, we added an example, based on
simple-enclave
. We named this new example applicationrot_enclave
, implemented inexample/enclave_rot_riscv
. The core application did not change. We modified the number of calls to Tyche (with theENCLAVE_ATTESTATION
hypercall) to match the size of the augmented report, and matched the data written to Tychools.There is a sanity check using the
ENCLAVE_ATTESTATION_SIZE
hypercall, so the example is bound to the current configuration of OpenSBI's TPM parameters.Tychools changes
We added in
crates/tychools/src/attestation.rs
a way to read from the application output file the entire augmented report. We also added a hardcoded PCR value that the report should be matched against. In order to verify the attestation signature, we had to import thering
crate. The Tychools changes are also RISC-V specific (wrapped in a block checking for theriscv_enabled
flag). In case of absence of a TPM on the platform, the verification fails as the hardcoded PCR value and the attestation PCR value mismatch.Note that currently, on its own,
simple-enclave
is broken and the message is not verified, and as such, our example follows the same pattern.Note that the hardcoded PCR value has to be changed everytime a change to Tyche is made for the PCR digest check to pass. Signature verification will still go through.