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.

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

ConstantTimeEq proc macro derive #302

Closed
wants to merge 1 commit into from
Closed

ConstantTimeEq proc macro derive #302

wants to merge 1 commit into from

Conversation

varsha888
Copy link
Contributor

@varsha888 varsha888 commented Mar 24, 2023

Motivation

Fixes #280
Using Subtle crate
Implemented ConstantTimeEq using proc macro derive
Implemented ConstantTimeGreater and ConstantTimeLesser

Future Work

Move proc macro into subtle crate

@github-actions github-actions bot added the size/XL PRs with more than 750 lines of changes label Mar 24, 2023
@github-actions github-actions bot added the rust Pull requests that update Rust code label Mar 24, 2023
@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Merging #302 (4586c32) into main (8767447) will increase coverage by 2.25%.
The diff coverage is 98.49%.

@@            Coverage Diff             @@
##             main     #302      +/-   ##
==========================================
+ Coverage   88.24%   90.50%   +2.25%     
==========================================
  Files          47       48       +1     
  Lines        4466     5611    +1145     
==========================================
+ Hits         3941     5078    +1137     
- Misses        525      533       +8     
Impacted Files Coverage Δ
core/build/src/lib.rs 5.55% <0.00%> (-0.40%) ⬇️
core/types/src/key_request.rs 84.66% <94.59%> (+4.50%) ⬆️
constant_time_derive/src/lib.rs 96.55% <96.55%> (ø)
dcap/types/src/certification_data.rs 97.08% <99.05%> (+0.74%) ⬆️
core/types/src/attestation_key.rs 98.54% <100.00%> (+1.53%) ⬆️
core/types/src/attributes.rs 79.38% <100.00%> (+4.07%) ⬆️
core/types/src/config_id.rs 100.00% <100.00%> (ø)
core/types/src/measurement.rs 100.00% <100.00%> (ø)
core/types/src/quote.rs 100.00% <100.00%> (+0.82%) ⬆️
core/types/src/report.rs 100.00% <100.00%> (ø)
... and 7 more

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

@github-actions
Copy link

github-actions bot commented Mar 24, 2023

❌ Unreviewed dependencies found

Crate Version Reviews (N/2) LoC Left-Pad Index Geiger Flags
yare 1.0.2 0 89 13 0 ____
static_assertions 1.1.0 0 243 174 0 ____
subtle 2.4.1 0 377 149 3 ____
displaydoc 0.2.3 0 409 65 24 ____
serial_test 2.0.0 0 768 44 0 ____
cargo-emit 0.2.1 0 824 10 0 ____
rand_core 0.6.4 0 829 162 2 ____
tempfile 3.5.0 0 927 110 46 ____
sha2 0.10.6 0 1061 111 204 ____
getrandom 0.2.9 0 1073 140 235 ____
once_cell 1.17.1 0 1101 139 139 ____
quote 1.0.26 0 1431 135 0 CB__
bitflags 2.1.0 0 2035 95 0 ____
textwrap 0.16.0 0 2234 77 0 ____
p256 0.13.1 0 2545 22 0 ____
proc-macro2 1.0.56 0 3696 86 18 CB__
rand 0.8.5 0 5273 62 32 ____
nom 7.1.3 0 10471 33 40 ____
bindgen 0.65.1 0 21304 15 405 CB__
syn 1.0.109 0 37583 28 74 CB__
libc 0.2.141 0 97569 16 525 CB__

@varsha888 varsha888 marked this pull request as ready for review March 24, 2023 23:26
@meowblecoinbot meowblecoinbot requested a review from a team March 24, 2023 23:26
Copy link
Collaborator

@nick-mobilecoin nick-mobilecoin left a comment

Choose a reason for hiding this comment

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

submitting without specific approval/denial since I'll be awol for the next couple of weeks

Cargo.toml Show resolved Hide resolved
constant_time_derive/Cargo.toml Show resolved Hide resolved
core/types/Cargo.toml Show resolved Hide resolved
core/build/src/lib.rs Outdated Show resolved Hide resolved
core/types/src/attestation_key.rs Outdated Show resolved Hide resolved
constant_time_derive/src/lib.rs Outdated Show resolved Hide resolved
constant_time_derive/src/lib.rs Outdated Show resolved Hide resolved
constant_time_derive/src/lib.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot added size/XXL PRs with more than 1000 lines of changes and removed size/XL PRs with more than 750 lines of changes labels Apr 11, 2023
Copy link
Collaborator

@nick-mobilecoin nick-mobilecoin left a comment

Choose a reason for hiding this comment

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

The main thing that should be done is mapping the == to Choice in the enum derive logic.
the other stuff is mainly nits and suggestions

constant_time_derive/src/lib.rs Outdated Show resolved Hide resolved
constant_time_derive/src/lib.rs Show resolved Hide resolved
constant_time_derive/src/lib.rs Outdated Show resolved Hide resolved
core/types/src/quote.rs Show resolved Hide resolved
core/types/src/svn.rs Outdated Show resolved Hide resolved
@nick-mobilecoin nick-mobilecoin requested review from a team and removed request for NotGyro April 20, 2023 22:01
@joekottke joekottke removed the request for review from jcape July 5, 2023 18:46
@nick-mobilecoin
Copy link
Collaborator

closing due to shifting focus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update Rust code size/XXL PRs with more than 1000 lines of changes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Impl ConstantTimeFoo on quote types
4 participants