c0dearm / sharks

Fast, small and secure Shamir's Secret Sharing library crate
https://crates.io/crates/sharks
Other
59 stars 12 forks source link

Add Zeroize to Share and GF256 #19

Closed film42 closed 3 years ago

film42 commented 3 years ago

This is an implementation for #8 . The derive(Copy) for GF256 was preventing the zeroize(drop) before. Copy is always a fast operation. Clone may or may not be fast. I switched over to Clone to and measured the performance, and it looks to be fast:

Using the benchmark from #18 :

WITH CLONE/ WITHOUT ZEROIZE DROP

Gnuplot not found, using plotters backend
obtain_shares_dealer    time:   [3.3146 us 3.3223 us 3.3346 us]                                  
                        change: [-0.8195% -0.5630% -0.2881%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

step_shares_dealer      time:   [732.84 ps 734.19 ps 735.70 ps]                                
                        change: [-1.3081% -1.0761% -0.8623%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

recover_secret          time:   [210.22 us 210.37 us 210.55 us]                           
                        change: [-0.1693% -0.0315% +0.1065%] (p = 0.66 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

share_from_bytes        time:   [27.105 ns 27.161 ns 27.220 ns]                              
                        change: [-1.9966% -1.8010% -1.5950%] (p = 0.00 < 0.05)
                        Performance has improved.

share_to_bytes          time:   [26.332 ns 26.389 ns 26.446 ns]                            
                        change: [-1.4125% -1.1769% -0.9238%] (p = 0.00 < 0.05)
                        Change within noise threshold.

In other words, the change to Clone kept performance within the noise threshold (M1 Macbook Air).

WITH CLONE AND ZEROIZE DROP:

Gnuplot not found, using plotters backend
obtain_shares_dealer    time:   [3.2719 us 3.2795 us 3.2875 us]                                  
                        change: [-1.8941% -1.5855% -1.3220%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  10 (10.00%) high mild

step_shares_dealer      time:   [873.66 ps 875.11 ps 876.42 ps]                                
                        change: [+18.881% +19.152% +19.415%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 30 outliers among 100 measurements (30.00%)
  16 (16.00%) low severe
  3 (3.00%) low mild
  5 (5.00%) high mild
  6 (6.00%) high severe

recover_secret          time:   [292.93 us 293.08 us 293.29 us]                           
                        change: [+39.209% +39.391% +39.567%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe

share_from_bytes        time:   [36.447 ns 36.518 ns 36.585 ns]                              
                        change: [+34.281% +34.597% +34.918%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 19 outliers among 100 measurements (19.00%)
  17 (17.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe

share_to_bytes          time:   [26.613 ns 26.626 ns 26.640 ns]                            
                        change: [+0.8541% +1.0461% +1.2337%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 12 outliers among 100 measurements (12.00%)
  1 (1.00%) low severe
  3 (3.00%) low mild
  3 (3.00%) high mild
  5 (5.00%) high severe

Now we begin to see the performance hit, so at least we know Zeroize is working!

This PR adds a new zeroize_memory feature and adds that feature to the default set. Happy to make it not enabled by default, though. I'm just a hobbyist. Do you think it would be worth adding this to the fuzzing feature set as well?

film42 commented 3 years ago

Thanks @c0dearm . I figured adding a feature flag that's not part of default (other than use of std) will likely not be used. Plus, it's easy to opt-out if you need the extra performance. Thanks for the great library!