Set minimum GELI pkcsv5 iterations
AbandonedPublic
Actions

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

Details

Reviewers

emaste
jmg
delphij
jonathan
pjd
oshogbo

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 - subversion

Lint

Lint Passed

Unit

No 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.

Herald added a subscriber: imp. · View Herald TranscriptSep 7 2016, 1:05 AM

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

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

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

In D7804#162346, @delphij wrote:

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

In D7804#162650, @allanjude wrote:

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.

In D7804#162912, @delphij wrote:

In D7804#162650, @allanjude wrote:

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).