search

private-attribution / ipa

A raw implementation of Interoperable Private Attribution
MIT License
42 stars 25 forks source link

Use constant-time comparisons #1423

Closed andyleiserson closed 2 weeks ago

andyleiserson commented 3 weeks ago

I suspect it would be quite difficult to construct a meaningful attack on one of these, but it still seems wise to use constant-time comparisons where best practice would dictate doing so.

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.30%. Comparing base (e8ef583) to head (94b7cbf). Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
...protocol/ipa_prf/validation_protocol/validation.rs 85.71% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1423 +/- ## ========================================== + Coverage 93.27% 93.30% +0.03% ========================================== Files 225 225 Lines 38739 38749 +10 ========================================== + Hits 36134 36156 +22 + Misses 2605 2593 -12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

akoshelev commented 3 weeks ago

we should measure the performance impact of this change before merging. I agree constant time comparisons are important, but we need to understand whether this is going to make things significantly worse

andyleiserson commented 2 weeks ago

Draft run, 4M, before: https://draft-mpc.vercel.app/query/view/obese-oat2024-11-11T2252, 24:51 After: https://draft-mpc.vercel.app/query/view/gone-coke2024-11-11T2333, 24:48

andyleiserson commented 2 weeks ago

I am going to merge this. If we want to use the constant time algorithm for all equality checks, we can do that -- although there's nothing special about equality checks at that point -- it becomes a general exercise of scrubbing everything for constant-time execution. (Which it probably should be, but that is not a small undertaking.)