Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tn/dcap rebased #1103

Merged
merged 34 commits into from
Dec 12, 2022
Merged

Tn/dcap rebased #1103

merged 34 commits into from
Dec 12, 2022

Conversation

Niederb
Copy link
Contributor

@Niederb Niederb commented Nov 22, 2022 •
edited
Loading

This is mostly a rebased version of #847. Please also check the comments there.

  • Add a new feature flag dcap that switches to dcap from ias
  • Additional changes to adapt it to the attestation-handler which was introduced later
  • Some of the code for the verification enclave we probably will not need in the end
  • I was thinking about having two versions (DCAP/IAS) of the AttestationHandler trait but decided against it (for now)
    • Would add additional state handling to the enclave
    • I'm not sure that both, DCAP and IAS, will fit nicely into a single API

@Niederb
Apply the changes of bh-dcap-squashed
d665ea7 
- Only change is that dcap is not active (see main.rs)
- Probably will not compile yet
@Niederb
Make the branch compile
ba7066d
@Niederb
Make clippy happy
953b080
@Niederb
Try with the updated docker image
9e24862
@Niederb
Try expanding the library path
87a4d7e
@Niederb
Try expanding the LD_LIBRARY_PATH in docker
e1357d1
@Niederb
Rename now_as_u64() to now_as_millis()
0dc20c7
@Niederb
Copy files over from builder
c51dc67
@Niederb
Move DCAP attestation to the attestation-handler
cd0cf10
@Niederb
Merge remote-tracking branch 'origin/tn/dcap-rebased' into tn/dcap-re…
0745ef5 
…based
@Niederb
Merge branch 'master' into tn/dcap-rebased
6057d72 
# Conflicts:
#	enclave-runtime/src/initialization/initialization.rs
#	sidechain/consensus/aura/src/slot_proposer.rs
@Niederb
Fix clippy issue
a9f1dae
@Niederb
Add dcap feature flag and make the dcap calls compile
c0725b6
@Niederb
Bring dcap api closer to the attestation handler
8e13caf
@Niederb
Move dump_dcap_ra_to_disk to attestation handler
82bca14
@Niederb
Clarify names for writing attestation material to disk
4d35c0c
@Niederb
Cleanup
ccab3ed
@Niederb
Fix clippy issues
b94aba0
@Niederb
Simplify code a bit
af317c2
@Niederb
Deactivate dcap
c0caec9
@Niederb
Incorporate review feedback
31cf879
@Niederb
Cancel report generation if sizes mismatch
a533b07
@Niederb
Check all parameters for null or 0
fb7b9c3
@Niederb
Move DCAP attestation to attestation_handler.rs
44d7e81
@Niederb
Rename IasAttestationHandler to IntelAttestationHandler as it also ha…
2c65a86 
…ndles dcap now
@Niederb
Add dcap to the AttestationHandler trait
10496f4
@Niederb
Make function names clearer
209dafb 
Distinguish between what generates the extrinsic and what generates the certificate
@Niederb
Mark more functions unsafe
018f024
@Niederb
Switch back to official docker image
7bae971
Niederb
Niederb commented Dec 1, 2022
View reviewed changes
build.Dockerfile
@@ -123,6 +123,7 @@ WORKDIR /usr/local/bin

COPY --from=builder /opt/sgxsdk/lib64 /opt/sgxsdk/lib64
COPY --from=builder /root/work/worker/bin/* ./
COPY --from=builder /lib/x86_64-linux-gnu /lib/x86_64-linux-gnu
Copy link
Contributor Author

@Niederb Niederb Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This copies some additional DCAP libraries, but also many libraries that we don't need. It makes our images quite a bit bigger.
Something I still want to understand and discuss with Felix. I created a follow up issue regarding this #1122

@Niederb
Copy link
Contributor Author

Niederb commented Dec 1, 2022

I think it is ready for a first review. @haerdib Maybe you also want to have a look of how I ruined your branch? 😰 😸

@Niederb Niederb requested a review from clangenb December 1, 2022 06:40
clangenb
clangenb reviewed Dec 6, 2022
View reviewed changes
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Cool, I really appreciate how nice and clean this looks, such that I can even understand it without digging too deep into the code. I only have some minor comments.

core-primitives/attestation-handler/src/attestation_handler.rs Outdated Show resolved Hide resolved
core-primitives/attestation-handler/src/attestation_handler.rs Show resolved Hide resolved
core-primitives/attestation-handler/src/cert.rs Show resolved Hide resolved
enclave-runtime/Enclave.edl Outdated Show resolved Hide resolved
enclave-runtime/src/ocall/attestation_ocall.rs Show resolved Hide resolved
enclave-runtime/src/ocall/attestation_ocall.rs Show resolved Hide resolved
clangenb
clangenb approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Very cool. From my point of view, this is ready to merge!

core-primitives/enclave-api/src/remote_attestation.rs Show resolved Hide resolved
enclave-runtime/Enclave.edl Outdated Show resolved Hide resolved
enclave-runtime/src/ocall/attestation_ocall.rs Show resolved Hide resolved
@Niederb
Merge branch 'master' into tn/dcap-rebased
1a59878 
# Conflicts:
#	Cargo.lock
#	enclave-runtime/Cargo.lock
#	enclave-runtime/Enclave.edl
@clangenb clangenb added A0-core Affects a core part B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" labels Dec 12, 2022
@Niederb Niederb marked this pull request as ready for review December 12, 2022 09:34
@clangenb clangenb added the E3-hardmerge PR introduces a lot changes in a lot of files. Requires some effort to merge or rebase to. label Dec 12, 2022
@clangenb clangenb merged commit bc0b67b into master Dec 12, 2022
This was referenced Dec 12, 2022
Closed
Closed
@Niederb Niederb deleted the tn/dcap-rebased branch December 13, 2022 06:13
@Niederb Niederb restored the tn/dcap-rebased branch December 13, 2022 06:29
@Niederb Niederb deleted the tn/dcap-rebased branch December 14, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clangenb clangenb clangenb approved these changes

Assignees
No one assigned
Labels
A0-core Affects a core part B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E3-hardmerge PR introduces a lot changes in a lot of files. Requires some effort to merge or rebase to.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Niederb @clangenb