Page MenuHomeFreeBSD

Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c
ClosedPublic

Authored by allanjude on Oct 17 2015, 8:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 15, 9:19 PM
Unknown Object (File)
Fri, Mar 15, 9:14 PM
Unknown Object (File)
Fri, Mar 15, 9:14 PM
Unknown Object (File)
Fri, Mar 15, 9:14 PM
Unknown Object (File)
Fri, Mar 15, 9:14 PM
Unknown Object (File)
Tue, Mar 12, 1:37 AM
Unknown Object (File)
Fri, Mar 1, 5:57 AM
Unknown Object (File)
Tue, Feb 27, 6:33 PM

Details

Summary

cperciva's libmd implementation is 5-30% faster (also seen here: https://svnweb.freebsd.org/changeset/base/r263218 )

cperciva's implementation was lacking SHA-384 which I implemented
Extend sbin/md5 to create sha384(1)

In a separate commit, I will also add SHA-512/256 (required by new ZFS features)

Chase dependancies on sys/crypto/sha2/sha2.{c,h} and replace them with sha512{c.c,.h}

Will extend reviewers to include secteam@ once the first round of issues are dealt with

Test Plan

benchmark results: http://www.allanjude.com/bsd/sha512_results.txt

The output of the new SHA-384 implementation were verified against openssl sha384 and http://csrc.nist.gov/groups/ST/toolkit/documents/Examples/SHA384.pdf

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1320
Build 1325: arc lint + arc unit

Event Timeline

allanjude retitled this revision from to Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c.
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: jmg, cperciva.

Just a few comments.

lib/libmd/Makefile
40

is this tab/space matching others or is this just a phab misformating issue?

sys/conf/files
559

I don't see bde using SHA256, we should probably remove it, though maybe as a different commit?

Looks like the module also compiles it. A quick compile of the module does look like it uses it.

561

I don't think you need zfs here yet. I'd add ZFS when you finally enable sha512/256 support.

allanjude added inline comments.
lib/libmd/Makefile
40

i put a tab when I shouldn't have, fixed in my local copy, will update with other changes

sys/conf/files
559

yeah, that was already there, I didn't change it. geom_eli uses both 256 and 512, should it be listed in both places?

561

the zfs SRCS= line references both, and again, I just updated sha2.c -> sha512c.c (and changed the order to make sense). But yeah, I could remove ZFS here and readd it in the next commit.

allanjude marked an inline comment as done.
allanjude edited edge metadata.

Minor updates to address feedback from jmg@

sys/conf/files
559

bde != eli.. They are different, so commenting that eli uses 256 and 512 is unrelated to my bde comment.

561

Since you're adding the line fresh, it's best to keep it minimal.

sys/conf/files
559

that should have been a separate comment.

Unrelated to geom_bde:

geom_eli uses both SHA256 and SHA512, should it be listed in this file?

allanjude edited edge metadata.

Update the diff to put sha384 prototypes in their own file as requested by cperciva

cperciva edited edge metadata.

Looks good to me.

des added a reviewer: des.
bapt added a reviewer: bapt.
delphij added a reviewer: delphij.

Approved by so, too. Sorry for the delay and thanks for working on this.

This revision is now accepted and ready to land.Dec 27 2015, 5:19 PM
This revision was automatically updated to reflect the committed changes.