Connects it up to username (libmd, libcrypt, sbin/md5), and the kernel (crypto.ko).
Will be connected to ZFS in a separate commit.
Details
- Reviewers
cem jmg - Group Reviewers
manpages - Commits
- rS300921: Import the skein hashing algorithm, based on the threefish block cipher
Verified against the test files
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Don't link sbin/md5 against both libmd and libcrypt, this was just a test to make sure it didn't break
My intention was for the copy of skein in sys/crypto to only show the differences from the unmodified import, but it didn't actually work that way.
Found a small nit in the man page.
lib/libmd/skein.3 | ||
---|---|---|
95 | Does this single "a" have to be on an line of its own? |
I don't think it's useful to drop the skein reference files in both sys/contrib and sys/crypto. Leave them in contrib and add sys/contrib/skein to $PATH in the sys/crypto Makefile.
Does anything in the kernel use Skein? Do you intend to add anything using Skein? Skein didn't win the SHA3 contest and I don't know how much real world use it gets or will get. If we don't need it for anything, I don't think we need to import it.
lib/libmd/Makefile | ||
---|---|---|
99–101 | Does this need to be conditional on actually compiling assembly? Does it hurt to always add these to the assembler flags? | |
lib/libmd/skein.3 | ||
4–7 | No he didn't. Correct the copyright header. :) | |
91–93 | Not true for short inputs or inputs where a dictionary attack is feasible, e.g. 64-bit values, first names, etc. I would just drop that sentence. | |
94–97 | I would drop this sentence too. It's not quite a magic bullet. | |
sys/contrib/skein/asm/skein_block_x64.asm | ||
1 ↗ | (On Diff #15781) | I thought this was supposed to go in machine/ per the Makefiles earlier. |
sys/contrib/skein/asm/skein_block_x64.s | ||
1 ↗ | (On Diff #15781) | Is this identical to skein_block_x64.asm? Why have both? |
sys/contrib/skein/asm/skein_block_xmm32.s | ||
1 ↗ | (On Diff #15781) | Identical to skein_block_xmm32.asm? |
sys/crypto/skein/amd64/skein_block_asm.s | ||
1–3 | Another copy of this file?! | |
sys/crypto/skein/brg_endian.h | ||
2–3 ↗ | (On Diff #15781) | I really don't think we need a second copy of this file. We have endian defines already from sys/endian.h. For the non-contrib/ directory, IMO simplify to just what is needed for FreeBSD. |
sys/crypto/skein/brg_types.h | ||
2–4 ↗ | (On Diff #15781) | Same comment as for brg_endian. |
sys/crypto/skein/skein.h | ||
103–105 | I'd prefer array-style output, e.g., int Skein_256_Final(Skein_256_Ctxt_t *, u08b_t[static SKEIN256_DIGEST_LENGTH]); | |
sys/crypto/skein/skein_freebsd.h | ||
29–31 | Drop space between * and ctx. | |
33–35 | Use static, drop excess space between * and ctx. |
I think the contrib stuff should probably go in the vendor tree instead, and then just have the one copy in sys/crypto
lib/libmd/Makefile | ||
---|---|---|
99–101 | I am not sure, I just went with what was already there. | |
lib/libmd/skein.3 | ||
94–97 | Should change these in the md5,sha* pages too then | |
sys/contrib/skein/asm/skein_block_xmm32.s | ||
1 ↗ | (On Diff #15781) | different assembly dialect. Probably don't need the unused ones I guess. This was just everything from the directories that made sense in the original distribution package. |
sys/crypto/skein/brg_endian.h | ||
2–3 ↗ | (On Diff #15781) | ok |
Refactor the skein import via ^/vendor-sys/skein
Much cleaner, and easier to see my local changes
There are a bunch of seemingly arbitrary changes to vendor/contrib code in this revision, and I don't know why.
lib/libcrypt/Makefile | ||
---|---|---|
43–44 | Is this just a rebase, or are you including this in the Skein commit? I'd keep them separate. | |
lib/libmd/Makefile | ||
12–17 | same | |
49–65 | same | |
lib/libmd/skein.3 | ||
94–97 | Yes. | |
sbin/md5/md5.c | ||
305–306 | Why'd you revert this? |
lib/libcrypt/Makefile | ||
---|---|---|
43–44 | it is a rebase, the sha512t256 commit went in earlier today, which had some merge conflicts with this patch since it adds to the same makefiles. | |
sbin/md5/Makefile | ||
22 | mismerge | |
sbin/md5/md5.c | ||
305–306 | I didn't know if it was a good idea to change the output of all of the hashing commands. Do you think it is ok to change it to the more readable format? |
sbin/md5/md5.c | ||
---|---|---|
305–306 | I don't think anything should depend on it... but now's the time to "break" it anyway, before 11 freezes. |
lib/libmd/skein.3 | ||
---|---|---|
110 | switched to using the proper mandoc em-dash markup |