Skip to content

Initial Implementation #1

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Initial Implementation #1

wants to merge 15 commits into from

Conversation

longtomjr
Copy link
Collaborator

@longtomjr longtomjr commented Nov 19, 2021

For testing, use: cargo test --no-default-features --features=test

@longtomjr longtomjr requested a review from PiDelport November 19, 2021 07:41
Copy link

@billguo99 billguo99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add rust-toolchain.toml in root for nightly rust toolchain for the cargo test command to work.

[package]
name = "dcap_attestation"
version = "0.1.0"
edition = "2018"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change edition = "2021" to the latest Rust edition. Can also change in the other Cargo.toml files.

let bindings: bool = !env::var("CARGO_FEATURE_BINDINGS").map_or(false, |val| val == "true");

if bindings {
// println!("cargo:rustc-link-lib=sgx_dcap_ql");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw you commented this out when you were adding the quote generations. Is this meant to stay commented out?

Copy link

@jdvlio jdvlio Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I am fairly certain this needs to be uncommented, since sgx_dcap_ql is listed under the package.links. Refer to the relevant cargo documentation.

Suggested change
// println!("cargo:rustc-link-lib=sgx_dcap_ql");
println!("cargo:rustc-link-lib=sgx_dcap_ql");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your "relevant cargo documentation" link leads to a 404 page. Could you please relink it? :)

We do want the Cargo.lock files for the enclave-test application.
This uses the file revision rather than the overall repository revision,
so that we don't have to update it for SGX SDK updates that don't touch
this file.
Copy link
Collaborator

@PiDelport PiDelport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I finished a first pass, and pushed some smaller changes. Questions so far:

  • Should libmock_dcap_quoteprov.so be removed?
  • Should we consider using matklad's crate / workspace layout? (This isn't a large project, but I'm growing to like the layout for Rust projects in general.)

Comment on lines 1 to 26
use sgx_types::{
c_char, sgx_ql_path_type_t, sgx_ql_request_policy_t, sgx_quote3_error_t, sgx_report_t,
sgx_target_info_t, uint32_t, uint8_t,
};

// FROM: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/d107bd0718f723221750a4f2973451b386cbf9d2/sgx_types/src/function.rs#L710
extern "C" {
//
// sgx_dcap_ql_wrapper.h
//
pub fn sgx_qe_set_enclave_load_policy(policy: sgx_ql_request_policy_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_target_info(p_qe_target_info: *mut sgx_target_info_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_quote_size(p_quote_size: *mut uint32_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_quote(
p_app_report: *const sgx_report_t,
quote_size: uint32_t,
p_quote: *mut uint8_t,
) -> sgx_quote3_error_t;
pub fn sgx_qe_cleanup_by_policy() -> sgx_quote3_error_t;

/* intel DCAP 1.6 */
pub fn sgx_ql_set_path(
path_type: sgx_ql_path_type_t,
p_path: *const c_char,
) -> sgx_quote3_error_t;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we replace these definitions with a pub use re-export?

I tried this locally, and it seems to work: is there any deeper or subtler reason not to do it this way?

Suggested change
use sgx_types::{
c_char, sgx_ql_path_type_t, sgx_ql_request_policy_t, sgx_quote3_error_t, sgx_report_t,
sgx_target_info_t, uint32_t, uint8_t,
};
// FROM: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/d107bd0718f723221750a4f2973451b386cbf9d2/sgx_types/src/function.rs#L710
extern "C" {
//
// sgx_dcap_ql_wrapper.h
//
pub fn sgx_qe_set_enclave_load_policy(policy: sgx_ql_request_policy_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_target_info(p_qe_target_info: *mut sgx_target_info_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_quote_size(p_quote_size: *mut uint32_t) -> sgx_quote3_error_t;
pub fn sgx_qe_get_quote(
p_app_report: *const sgx_report_t,
quote_size: uint32_t,
p_quote: *mut uint8_t,
) -> sgx_quote3_error_t;
pub fn sgx_qe_cleanup_by_policy() -> sgx_quote3_error_t;
/* intel DCAP 1.6 */
pub fn sgx_ql_set_path(
path_type: sgx_ql_path_type_t,
p_path: *const c_char,
) -> sgx_quote3_error_t;
}
// Re-export the `sgx_dcap_ql` symbols from `sgx_types`.
pub use sgx_types::{
// sgx_dcap_ql_wrapper.h
sgx_qe_cleanup_by_policy,
sgx_qe_get_quote,
sgx_qe_get_quote_size,
sgx_qe_get_target_info,
sgx_qe_set_enclave_load_policy,
// Intel DCAP 1.6
sgx_ql_set_path,
};

Copy link

@jdvlio jdvlio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready to be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants