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

Add SignedQeIdentityVerifier #105

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Add SignedQeIdentityVerifier #105

merged 2 commits into from
Jun 23, 2023

Conversation

nick-mobilecoin
Copy link
Collaborator

@nick-mobilecoin nick-mobilecoin commented Jun 16, 2023 •
edited
Loading

Add a verifier which verifies the signature and time validity of the qe
identity json data.

Be sure and look at #105 (comment) as it may push for a different implementation

@nick-mobilecoin
Add SignedQeIdentityVerifier
1da0b7b 
Add a verifier which verifies the signature and time validity of the qe
identity json data.
@github-actions github-actions bot added the size/L Large PRs label Jun 16, 2023
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jun 16, 2023
Merged
@meowblecoinbot meowblecoinbot requested a review from a team June 16, 2023 15:37
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jun 16, 2023
Merged
@nick-mobilecoin
Copy link
Collaborator Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@github-actions
Copy link

github-actions bot commented Jun 16, 2023 •
edited
Loading

❌ Unreviewed dependencies found

Crate Version Reviews (N/2) LoC Left-Pad Index Geiger Flags
yare 1.0.2 0 89 11 0 ____
mc-sgx-dcap-sys-types 0.6.1 0 97 3 0 CB__
assert_matches 1.5.0 0 209 84 0 ____
mc-sgx-core-sys-types 0.6.1 0 224 5 0 CB__
subtle 2.5.0 0 440 146 9 ____
hex 0.4.3 0 474 144 0 ____
displaydoc 0.2.4 0 600 58 20 ____
x509-cert 0.2.3 0 1963 6 0 ____
mc-sgx-dcap-types 0.6.1 0 1990 0 0 ____
mc-sgx-core-types 0.6.1 0 2053 1 0 ____
textwrap 0.16.0 0 2234 74 0 ____
p256 0.13.2 0 2545 25 0 ____
der 0.7.6 0 7820 23 0 ____
serde 1.0.164 0 10262 49 5 CB__
serde_json 1.0.96 0 12157 41 7 CB__

@nick-mobilecoin nick-mobilecoin requested review from eranrund and cbeck88 and removed request for a team June 16, 2023 15:56
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jun 16, 2023
Closed
3 tasks
eranrund
eranrund approved these changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@eranrund eranrund left a comment

Choose a reason for hiding this comment

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

LGTM, I don't think there's a huge win in going with SignedJson and the alias trick

verifier/src/quote_enclave_identity.rs Show resolved Hide resolved
verifier/src/quote_enclave_identity.rs Outdated Show resolved Hide resolved
@nick-mobilecoin
Simplify Verifiers that use Option<Error>
1b2c051 
Previously `Verifier`s that had a `Value` of `Option<Error>` had logic
to manually get the error out of a `Result<>` and convert it to the
`Option<Error>`. This logic has been simplified by utilizing the
[`err()`](https://doc.rust-lang.org/std/result/enum.Result.html#method.err)
method of the `Result<>` type.
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #105 (1b2c051) into nick/qe_identity (af9fb2b) will decrease coverage by 0.05%.
The diff coverage is 97.89%.

@@                 Coverage Diff                  @@
##           nick/qe_identity     #105      +/-   ##
====================================================
- Coverage             98.41%   98.37%   -0.05%     
====================================================
  Files                    10       10              
  Lines                  2460     2645     +185     
====================================================
+ Hits                   2421     2602     +181     
- Misses                   39       43       +4
Impacted Files Coverage Δ
verifier/src/error.rs 70.00% <ø> (ø)
verifier/src/quote_enclave_identity.rs 98.22% <97.88%> (-0.44%) ⬇️
verifier/src/tcb.rs 99.81% <100.00%> (-0.01%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Base automatically changed from nick/qe_identity to main June 23, 2023 22:24
@nick-mobilecoin nick-mobilecoin merged commit dd681f7 into main Jun 23, 2023
18 of 20 checks passed
@nick-mobilecoin nick-mobilecoin deleted the nick/qe_identity_verify branch June 23, 2023 22:25
@nick-mobilecoin nick-mobilecoin linked an issue Jul 10, 2023 that may be closed by this pull request
Quoting enclave ID verifier #101
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eranrund eranrund eranrund approved these changes

@cbeck88 cbeck88 Awaiting requested review from cbeck88

Assignees

@nick-mobilecoin nick-mobilecoin

Labels
size/L Large PRs
Projects
Core PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Quoting enclave ID verifier
2 participants
@nick-mobilecoin @eranrund