-
Notifications
You must be signed in to change notification settings - Fork 712
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
bench: s2n_constant_time_equals #4717
base: main
Are you sure you want to change the base?
Conversation
unsafe { s2n_constant_time_equals(a.as_ptr(), b.as_ptr(), a.len() as u32) } | ||
} | ||
|
||
fn comparison(criterion: &mut Criterion) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are we trying to do with this PR? Like, it is relevant for #4709, but once that is merged we're not really going to have a use for this comparison. If we're actually trying to benchmark s2n_constant_time_equals(), I would expect to see a test that checks that the comparison of [1, 2, 3, 4, 5] to [1, 0, 0, 0, 0] takes exactly as much time as the comparison of [1, 2, 3, 4, 5] to [1, 2, 3, 4, 5]. But maybe we trust that that case is covered with cbmc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, I see this benchmark function as being strictly interested in the performance of s2n_constant_time_equals
, and not at all concerned with the correctness of it. We formally assert its correctness with the ctverif
proofs.
Co-authored-by: maddeleine <[email protected]>
Co-authored-by: maddeleine <[email protected]>
Co-authored-by: maddeleine <[email protected]>
- add comment for black_box
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Resolved issues:
Related to #4709
Description of changes:
This adds benchmarks specifically for
s2n_constant_time_equals
.Testing:
Existing CI should pass.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.