----- Forwarded message from Pawel Jakub Dawidek <p...@freebsd.org> -----
Date: Mon, 7 Oct 2013 11:44:57 +0200 From: Pawel Jakub Dawidek <p...@freebsd.org> To: z...@lists.illumos.org Subject: Re: [zfs] [Review] 4185 New hash algorithm support Message-ID: <20131007094456.gb1...@garage.freebsd.pl> User-Agent: Mutt/1.5.21 (2010-09-15) Reply-To: z...@lists.illumos.org On Mon, Oct 07, 2013 at 12:47:52AM +0100, Saso Kiselkov wrote: > Please review what frankly has become a bit of a large-ish feature: > http://cr.illumos.org/~webrev/skiselkov/new_hashes/ > > This webrev implements new hash algorithms for ZFS with much improved > performance. There are three algorithms included: > > * SHA-512/256: truncated version of SHA-512 per FIPS 180-4. Uses > all existing code from the sha2 module (with new H(0) consts), > but the native 64-bit arithmetic used in SHA-512 provides a > ~50% performance boost relative to SHA-256 on 64-bit hardware. > > * Skein-512: 80% faster than SHA-256 in optimized C implementation, > and a very high security margin (Skein was a finalist in SHA-3). > Also includes a KCF SW provider. > > * Edon-R-512: 350% faster than SHA-256 in optimized C implementation. > Security margin lower than Skein. > > To address any security concerns associated with using new algorithms > this patch also implements salted checksum support. We store a random > 256-bit secret key (the salt) in the MOS and use it to pre-seed the hash > algorithms (Skein and Edon-R use this, SHA-512/256 is just a straight > hash). Any attacker thus cannot simply mount a collision attack on the > algorithm, since they can't completely control the input. I do like your patch and addition of first two algorithms, but I have to comment on the third one. By adding a salt to Edon-R-512 you assume that prerequisite to creating collision is guessing the entire salt, so having 256bit salt makes it safe. I'd say this is brave assumption. We (at least I) don't know how this algorithm treats those first 256 bits. Maybe for large blocks some (most?) of the salt enropy is lost? If we think Edon-R-512 is not cryptographically secure, making any assumptions that weren't properly analysed comes at too high risk, IMHO. You are also calling your salting method being a MAC, where we do have well-known and secure MAC algorithms that are based on hash functions (ie. HMAC). Personally I'd love to have an option to use HMAC/SHA256 for example with secret key stored in pool. Currently in our product we put ZFS with SHA256 on top of block-level disk encryption. I'd feel much better to have proper data authentication using HMAC. At some point I may find time to implement that based on your patch. Do we want to allow to use different algorithms for using deduplication within send/recv streams? I think SHA256 is now hardcoded? It can of course be implemented separately, just wondering. Few minor observations: --- old/usr/src/lib/libzfs/common/libzfs_dataset.c Mon Oct 7 09:23:07 2013 +++ new/usr/src/lib/libzfs/common/libzfs_dataset.c Mon Oct 7 09:23:07 2013 @@ -1377,6 +1377,12 @@ "property setting is not allowed on " "bootable datasets")); (void) zfs_error(hdl, EZFS_NOTSUP, errbuf); + } else if (prop == ZFS_PROP_CHECKSUM || + prop == ZFS_PROP_DEDUP) { + (void) zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "property setting is not allowed on " + "root pools")); + (void) zfs_error(hdl, EZFS_NOTSUP, errbuf); } else { (void) zfs_standard_error(hdl, err, errbuf); } When I read this change it looks like we don't allow to change checksum property on root pools at all? --- /dev/null Mon Oct 7 09:23:09 2013 +++ new/usr/src/uts/common/fs/zfs/edonr_zfs.c Mon Oct 7 09:23:09 2013 [...] +void * +zio_checksum_edonr_tmpl_init(const zio_cksum_salt_t *salt) +{ [...] + /*LINTED E_TRUE_LOGICAL_EXPR*/ + ASSERT(EDONR_BLOCK_SIZE == 2 * (EDONR_MODE / 8)); There is CTASSERT() macro in FreeBSD that can be used for checks like that, which generates compile-time error if it isn't true. + ctx = kmem_zalloc(sizeof (*ctx), KM_SLEEP); For Skein you also use KM_PUSHPAGE flag. -- Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://mobter.com ------------------------------------------- illumos-zfs Archives: https://www.listbox.com/member/archive/182191/=now RSS Feed: https://www.listbox.com/member/archive/rss/182191/22842876-6fe17e6f Modify Your Subscription: https://www.listbox.com/member/?member_id=22842876&id_secret=22842876-a25d3366 Powered by Listbox: http://www.listbox.com ----- End forwarded message ----- -- Eugen* Leitl <a href="http://leitl.org";>leitl</a> http://leitl.org ______________________________________________________________ ICBM: 48.07100, 11.36820 http://ativel.com http://postbiota.org AC894EC5: 38A5 5F46 A4FF 59B8 336B 47EE F46E 3489 AC89 4EC5
signature.asc
Description: Digital signature
_______________________________________________ cryptography mailing list cryptography@randombit.net http://lists.randombit.net/mailman/listinfo/cryptography
