Skip to content
This repository has been archived by the owner on May 9, 2022. It is now read-only.

Add implementation for execution token issuance #99

Merged
merged 14 commits into from
Jun 25, 2021
Merged

Conversation

longtomjr
Copy link
Collaborator

@longtomjr longtomjr commented Jun 12, 2021

No description provided.

@longtomjr longtomjr self-assigned this Jun 12, 2021
@longtomjr longtomjr force-pushed the he-issue-exec-token branch 2 times, most recently from 2535d60 to 16f302d Compare June 17, 2021 14:15
@longtomjr longtomjr marked this pull request as ready for review June 17, 2021 14:24
@longtomjr
Copy link
Collaborator Author

There is still some todos that need looking at, and the type names need to be refactored as well. The bulk of the functionality should be here for review though.

@longtomjr longtomjr force-pushed the he-issue-exec-token branch 2 times, most recently from 772c108 to ae43415 Compare June 17, 2021 14:47
@longtomjr longtomjr changed the title WIP: Exec token issuance Add implementation for execution token issuance Jun 17, 2021
@PiDelport PiDelport force-pushed the he-issue-exec-token branch from ae43415 to ac386d5 Compare June 17, 2021 15:43
@PiDelport
Copy link
Collaborator

(Rebased to refresh)

@PiDelport PiDelport force-pushed the he-issue-exec-token branch from ac386d5 to cf9f74d Compare June 18, 2021 09:50
@PiDelport PiDelport added M: auth enclave Module: Authorisation enclave (rtc_auth_enclave) feat New feature or request labels Jun 18, 2021
@longtomjr
Copy link
Collaborator Author

TODO: Add in the size of the dataset into the execution token.

@PiDelport PiDelport force-pushed the he-issue-exec-token branch 3 times, most recently from 5a38881 to 10a3d51 Compare June 21, 2021 16:13
@PiDelport
Copy link
Collaborator

I rebased to capture the enum naming changes, and tweaked the new imports to be cargo fmt compatible: diff

@PiDelport PiDelport force-pushed the he-issue-exec-token branch from 10a3d51 to 66151a8 Compare June 21, 2021 21:59
@PiDelport
Copy link
Collaborator

This was pulling in duplications of once_cell-sgx and serde-json-sgx, due to that Cargo issue.

I took a stab at documenting and fixing this a bit better:

rtc_auth_enclave/Cargo.toml Outdated Show resolved Hide resolved
rtc_auth_enclave/src/lib.rs Outdated Show resolved Hide resolved
rtc_types/src/exec_token.rs Outdated Show resolved Hide resolved
rtc_types/src/lib.rs Show resolved Hide resolved
rtc_auth_enclave/src/lib.rs Outdated Show resolved Hide resolved
rtc_auth_enclave/src/jwt.rs Show resolved Hide resolved
rtc_auth_enclave/src/token_store.rs Show resolved Hide resolved
rtc_auth_enclave/src/token_store.rs Outdated Show resolved Hide resolved
rtc_auth_enclave/src/token_store.rs Outdated Show resolved Hide resolved
rtc_uenclave/src/enclaves/rtc_auth.rs Outdated Show resolved Hide resolved
@PiDelport PiDelport linked an issue Jun 22, 2021 that may be closed by this pull request
6 tasks
@PiDelport
Copy link
Collaborator

(This closes #38, right?)

rtc_auth_enclave/src/lib.rs Show resolved Hide resolved
@longtomjr
Copy link
Collaborator Author

@PiDelport @Nghondzweni . Please have a look at the latest commit diff. I think this wraps up all the feedback, so just double check if there is anything else. Thanks for the reviews!
16084f7

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.

@longtomjr: This works, but it might be simpler to use an [out] variable for the buffer capacity used, instead of adding the IssueTokenResponse struct for that?

This seems to be the go-to pattern for variable-sized ECALL out buffers: for example here and here.

I took a stab at amending 16084f7 to use that approach, in 50fb146, and the result looks a bit more like a pattern we'd want to standardise on:

  1. EDL: rtc_auth_enclave/rtc_auth.edl
  2. Inside enclave: rtc_auth_enclave/src/lib.rs
  3. Outside enclave: rtc_uenclave/src/enclaves/rtc_auth.rs

Does that look preferable to you too?

@PiDelport
Copy link
Collaborator

Followed up with @longtomjr: happy with the out_token_used parameter. 🚀

…sues

Co-authored-by: Pi Delport <pi@registree.io>
@PiDelport PiDelport force-pushed the he-issue-exec-token branch from 16084f7 to bd03587 Compare June 25, 2021 12:19
@PiDelport PiDelport merged commit 3f9331b into main Jun 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat New feature or request M: auth enclave Module: Authorisation enclave (rtc_auth_enclave)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an ECALL for issuing execution tokens
3 participants