-
Notifications
You must be signed in to change notification settings - Fork 93
Changes to verify MR enclave value in client side #658
base: main
Are you sure you want to change the base?
Conversation
return quote_status; | ||
} | ||
|
||
bool verify_mr_enclave_value(const std::string& enclave_quote_body, |
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.
Please add function comments
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.
Done
v_report["isvEnclaveQuoteBody"], | ||
mr_enclave) | ||
if mr_check is False: | ||
logger.info("MR enclave value check failed") |
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.
Please change log level to error.
pkg-config \ | ||
wget \ | ||
cmake \ | ||
libprotobuf-dev \ |
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 dont need protobuf. please remove
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.
It is required otherwise below dpkg complain while installing packages.
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.
Ok
1. verify IAS report utility which verifies MR enclave 2. generic client shows the usage. Signed-off-by: Ramakrishna Srinivasamurthy <[email protected]>
################################################################################ | ||
|
||
SET(SGX_SDK "$ENV{SGX_SDK}") | ||
SET(SGX_SDK_INCLUDE ${SGX_SDK}/include) |
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.
Do we need SGX SDK dependency for untrusted part.? SGX SDK dependency previously was based on the condition if(NOT UNTRUSTED_ONLY)
.
sgx_quote_t* quote_body = reinterpret_cast<sgx_quote_t*>( | ||
quote_bytes.data()); | ||
sgx_report_body_t* report_body = "e_body->report_body; | ||
sgx_measurement_t mr_enclave_from_report = *(&report_body->mr_enclave); |
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 issue with above approach is it brings SGX SDK dependency on common/cpp module. So we need to install SGX SDK in client Dockerfile ? All this is done just to extract the mrencalve value from quote.
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 would suggest to do mrenclave verification at python layer either using
- Python struct - similar to
https://github.com/hyperledger/sawtooth-poet/blob/master/common/sawtooth_poet_common/sgx_structs/_sgx_quote.py
https://github.com/hyperledger/sawtooth-poet/blob/master/common/sawtooth_poet_common/sgx_structs/_sgx_report_body.py
https://github.com/hyperledger/sawtooth-poet/blob/master/common/tests/test_sgx_structs/tests.py
or
- Extract the mrenclave value as bytes from a fixed offset in the IAS report
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 we are using python client for doing to MR enclave check and we are depend on C++ sgx sdk. Is there problem if we bring SGX sdk dependency in common/cpp? Anyway we are using sgx sdk for verifying signature and quote verification. Porting C++ structure to python is not right thing to do. It may lead to inconsistencies. I am not clear on option 2.
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.
One of design goals of Avalon is to hide SGX complexities from Avalon clients. If we make Avalon clients install SGX SDK we are moving away from that goal. Sawtooth PoET for example solves this by doing AVR verification at Python layer and it uses the python struct. Another option is used a fixed offset in AVR quote (based on IAS attestation API spec)
Also bringing in SGX SDK dependency on Avalon client just to do MREnclave verification is not necessary.
sgx_quote_t* quote_body = reinterpret_cast<sgx_quote_t*>( | ||
quote_bytes.data()); | ||
sgx_report_body_t* report_body = "e_body->report_body; | ||
sgx_measurement_t mr_enclave_from_report = *(&report_body->mr_enclave); |
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 would suggest to do mrenclave verification at python layer either using
- Python struct - similar to
https://github.com/hyperledger/sawtooth-poet/blob/master/common/sawtooth_poet_common/sgx_structs/_sgx_quote.py
https://github.com/hyperledger/sawtooth-poet/blob/master/common/sawtooth_poet_common/sgx_structs/_sgx_report_body.py
https://github.com/hyperledger/sawtooth-poet/blob/master/common/tests/test_sgx_structs/tests.py
or
- Extract the mrenclave value as bytes from a fixed offset in the IAS report
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.
Use @return in your block comments
* @param signing_cert_pem signing certificate | ||
* @param ias_report attestion report | ||
* @param ias_signature attestation report signature | ||
* Returns true if signature verification success |
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.
Change Return to @return
* report and compare with the value passed | ||
* @param enclave_quote_body Enclave quote body | ||
* @param mr_enclave MR enclave value in hex format | ||
* Return true if comparision matches otherwise false |
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.
Change Return to @return
Signed-off-by: Ramakrishna Srinivasamurthy [email protected]