Created attachment 159924 [details] proposed patch The PBKDF2 in sys/geom/eli/pkcs5v2.c is around half the speed than it could be. I have some background on this issue on my blog[1]. I've attached a patch which does the minimum needed changes. This processes the HMAC key (in PBKDF2, the password) once, and then uses this as a starting point for all the subsequent computations. - For existing volumes, this should roughly double the mounting speed (measured as 51% of previous on my machine). - For new volumes it should approximately double the security margin (measured as 196% of previous on my machine). Cheers, Joe [1]: https://jbp.io/2015/08/11/pbkdf2-performance-matters/
Take.
Thanks for the patch... I'll work at getting this integrated.. It looks like we are missing a test suite for this code, so I want to get one written before I commit.. Also, after looking at this code, I realized that we were missing the use of explicit_bzero, which I've added... Work in progress at: https://github.com/jmgurney/freebsd/tree/elipbkdf2
Need to investigate if sys/opencrypto/cryptosoft.c's implementation of HMAC can also be sped up in a similar way...
Ping?
Mentioned in a review at https://reviews.freebsd.org/D7804 jmg do you think you'll be able to integrate this patch soon?
Review for integrating this patch open at https://reviews.freebsd.org/D8236
A commit references this bug: Author: allanjude Date: Sun Feb 19 19:30:33 UTC 2017 New revision: 313962 URL: https://svnweb.freebsd.org/changeset/base/313962 Log: improve PBKDF2 performance The PBKDF2 in sys/geom/eli/pkcs5v2.c is around half the speed it could be GELI's PBKDF2 uses a simple benchmark to determine a number of iterations that will takes approximately 2 seconds. The security provided is actually half what is expected, because an attacker could use the optimized algorithm to brute force the key in half the expected time. With this change, all newly generated GELI keys will be approximately 2x as strong. Previously generated keys will talk half as long to calculate, resulting in faster mounting of encrypted volumes. Users may choose to rekey, to generate a new key with the larger default number of iterations using the geli(8) setkey command. Security of existing data is not compromised, as ~1 second per brute force attempt is still a very high threshold. PR: 202365 Original Research: https://jbp.io/2015/08/11/pbkdf2-performance-matters/ Submitted by: Joe Pixton <jpixton@gmail.com> (Original Version), jmg (Later Version) Reviewed by: ed, pjd, delphij Approved by: secteam, pjd (maintainer) MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D8236 Changes: head/etc/mtree/BSD.tests.dist head/sys/boot/geli/Makefile head/sys/geom/eli/g_eli.h head/sys/geom/eli/g_eli_hmac.c head/sys/geom/eli/pkcs5v2.c head/tests/sys/geom/Makefile head/tests/sys/geom/eli/ head/tests/sys/geom/eli/Makefile head/tests/sys/geom/eli/pbkdf2/ head/tests/sys/geom/eli/pbkdf2/Makefile head/tests/sys/geom/eli/pbkdf2/gentestvect.py head/tests/sys/geom/eli/pbkdf2/hmactest.c head/tests/sys/geom/eli/pbkdf2/testvect.h
Thanks for the contribution Joe. Sorry it took so long to get integrated. Will close bug in a few weeks when this is merged into the stable branch.
A commit references this bug: Author: asomers Date: Fri Jan 12 00:31:08 UTC 2018 New revision: 327856 URL: https://svnweb.freebsd.org/changeset/base/327856 Log: MFC r313962, r313972-r313973, r315230 r313962 by allanjude: improve PBKDF2 performance The PBKDF2 in sys/geom/eli/pkcs5v2.c is around half the speed it could be GELI's PBKDF2 uses a simple benchmark to determine a number of iterations that will takes approximately 2 seconds. The security provided is actually half what is expected, because an attacker could use the optimized algorithm to brute force the key in half the expected time. With this change, all newly generated GELI keys will be approximately 2x as strong. Previously generated keys will talk half as long to calculate, resulting in faster mounting of encrypted volumes. Users may choose to rekey, to generate a new key with the larger default number of iterations using the geli(8) setkey command. Security of existing data is not compromised, as ~1 second per brute force attempt is still a very high threshold. PR: 202365 Original Research: https://jbp.io/2015/08/11/pbkdf2-performance-matters/ Submitted by: Joe Pixton <jpixton@gmail.com> (Original Version), jmg (Later Version) Reviewed by: ed, pjd, delphij Approved by: secteam, pjd (maintainer) Differential Revision: https://reviews.freebsd.org/D8236 r313972 by ngie: Unbreak the build when "make obj" is executed beforehand Using relative paths imply working directory (in this case .OBJDIR), whereas the sources live in the .CURDIR-relative path. X-MFC with: r313962 Pointyhat to: allanjude Sponsored by: Dell EMC Isilon r313973 by ngie: A forced commit to note other portion of the Makefile change accidentally committed in r313972 The code committed in r313962 implicitly relies on python 2.x to generate testvect.h . There are a handful of issues with this approach: - python is not an explicit build dependency for FreeBSD - python 2.x is deprecated and will be removed sometime in the future (potentially before 11.x's EOL), and the script does not function with python 3.5 (it uses deprecated idioms and incompatible function calls). - python(1) (by default) lives in /usr/local/bin (${LOCALBASE}/bin) and gentestvect.py is a dependency of testvect.h (prior to r313972) which means that if the mtime of the generator script was newer than the mtime of the test vector, it could cause a spurious build failure in build time or at install time. A better solution using C/C++ should be devised. Discussed with: allanjude X-MFC with: r313962, r313972 Sponsored by: Dell EMC Isilon r315230 by ngie: Move .../sys/geom/eli/pbkdf2... to .../sys/geom/class/eli/... This change moves the tests added in r313962 to an existing directory structure used by the geli TAP tests. It also, renames the test from pbkdf2 to pbkdf2_test . The changes to ObsoleteFiles.inc are being committed separately as they aren't needed for the MFC to ^/stable/11, etc, if the MFC for the tests is done all in one commit. X-MFC with: r313962, r313972-r313973 Reviewed by: allanjude Sponsored by: Dell EMC Isilon Differential Revision: D9985 Changes: _U stable/11/ stable/11/sys/boot/geli/Makefile stable/11/sys/geom/eli/g_eli.h stable/11/sys/geom/eli/g_eli_hmac.c stable/11/sys/geom/eli/pkcs5v2.c stable/11/tests/sys/geom/class/eli/Makefile stable/11/tests/sys/geom/class/eli/gentestvect.py stable/11/tests/sys/geom/class/eli/hmac_test.c stable/11/tests/sys/geom/class/eli/testvect.h
jpixton: Thank you for the submission.