Page MenuHomeFreeBSD

Set minimum GELI pkcsv5 iterations
AbandonedPublic

Authored by allanjude on Sep 7 2016, 1:05 AM.

Details

Summary

If the user does not specify the number of iterations to use, GELI does a live system benchmark to find a number of iterations that takes more than 2 seconds.
Enforce a minimum on the auto-calculated value to prevent this ever being underestimated.

The only consumer of this function is sbin/geom/class/eli/geom_eli.c

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 5076
Build 5156: arc lint + arc unit

Event Timeline

allanjude updated this revision to Diff 20120.Sep 7 2016, 1:05 AM
allanjude retitled this revision from to Set minimum GELI pkcsv5 iterations.
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: delphij, pjd, oshogbo, emaste, jonathan.
emaste accepted this revision.Sep 7 2016, 1:35 AM
emaste edited edge metadata.
emaste added inline comments.
sys/geom/eli/pkcs5v2.c
111

In general I prefer avoiding repeating the code in a comment, so that it can't be out of sync -- e.g. perhaps just append "with a minimum bound" to the existing comment.

This revision is now accepted and ready to land.Sep 7 2016, 1:35 AM
delphij requested changes to this revision.Sep 8 2016, 5:23 AM
delphij edited edge metadata.

This is not the problem. For fast hardware, this is a no-op; for slower hardware, this might make the code perform even slower -- which by itself is not necessarily a problem, but if we chose a magic number, it would become outdated very soon.

What I was referring to was bug 202365, which makes the code 50% faster.

This revision now requires changes to proceed.Sep 8 2016, 5:23 AM
delphij added a reviewer: jmg.Sep 8 2016, 5:24 AM
delphij added a subscriber: jmg.

@jmg could you please integrate the change you have worked on at bug 202365?

This is not the problem. For fast hardware, this is a no-op; for slower hardware, this might make the code perform even slower -- which by itself is not necessarily a problem, but if we chose a magic number, it would become outdated very soon.
What I was referring to was bug 202365, which makes the code 50% faster.

Do you think setting a minimum is still valuable? Having a lower bound seems better than not

Do you think setting a minimum is still valuable? Having a lower bound seems better than not

I prefer not landing this change.

In general we do not want to introduce magic numbers that would become obsolete soon (they would be a big headache to maintain, and it's always hard to reason the choice), and it does not solve the actual problem.

Do you think setting a minimum is still valuable? Having a lower bound seems better than not

I prefer not landing this change.
In general we do not want to introduce magic numbers that would become obsolete soon (they would be a big headache to maintain, and it's always hard to reason the choice), and it does not solve the actual problem.

Ok. I have started integrating jmg's version of the user-submitted patch. I've resolved the merge conflicts (the geli hmac functions were split into their own file, etc), and just have to test it, then I'll post it as a separate review.

I just thought It would make sense to set a power bound to the number of iterations, so that a busy system wouldn't generate a weak key. This was mostly brought on by the discovery that openbsd uses a fixed value of 8000 interactions in their FDE system (and the HMAC is sha1).

pjd edited edge metadata.Feb 3 2017, 9:38 PM

I agree with delphij that we should not introduce lower limit.

allanjude abandoned this revision.Feb 5 2017, 11:26 AM