Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-29 Thread Zooko Wilcox-OHearn
On Mon, Oct 28, 2013 at 6:49 AM, Richard Elling
richard.ell...@gmail.com wrote:

 I hate to keep this thread going, but it cannot end with an open-ended 
 threat... please, let's kill it off nice and proper.

Hey, I don't want to waste anyone's time, including my own. If nobody
is interested in this — possibly including the original author of the
patch, Saso Kiselkov, judging from ¹ — then by all means let's drop
the subject.

¹ http://article.gmane.org/gmane.os.illumos.zfs/3103

However, in case someone out there is reading this…

 Do you agree that if the attacker does not have DDT key (including the hash) 
 of the future intended write (ignoring the fact that we haven't invented a 
 properly working time machine yet) that this attack is extraordinarily 
 difficult to conduct with any hope of a fruitful outcome? If so, let's kill 
 this thread.

I'm not sure what you mean about the future intended write. The risk I
was talking about was that an attacker can cause two blocks (on
someone else's ZFS system) to hash to the same fingerprint.

Assuming that “the DDT key” is the secret which is prefixed to the
block contents in the current patch, then I agree it is extremely
difficult to cause two blocks to hash to the same fingerprint. A way
to be more precise about how difficult it is, is to talk about what
property we depend on the hash function to have in order to prevent
this attack.

If the attacker steals the secret, or if there is some variant of ZFS
which shares that secret among multiple parties ², then the property
that we rely on the hash function to have is “collision-resistance”.
If the attacker doesn't have the secret, then the property that we
rely on the hash function to have one which is closely related to, and
even easier-to-achieve than, “MAC”.

² http://article.gmane.org/gmane.os.illumos.zfs/3015

Functions which, in my opinion, have this easier-to-achieve-than-MAC
property include SHA-256, HMAC-MD5, Skein, BLAKE2, and
BLAKE2-reduced-to-5-rounds. Almost all cryptographic hash functions
have this property! One of the few cryptographic hash functions which
I would be not so confident in is Edon-R. It *probably* still has this
property, but it might not, and cryptographers haven't studied it
much.

Functions which, in my opinion, have the much harder-to-achieve
“collision-resistance” property include SHA-256, Skein, BLAKE2, and
*probably* BLAKE2-reduced-to-5-rounds.

 I'll let the fact that there is no future dedup run and there is no 
 replace blocks later in ZFS fall quietly in the forest with nobody 
 listening.

I'm sorry if I've misunderstood; I'm not an expert on ZFS. If you'd
like to take some of your valuable time to explain it to me, I'll
spend some of my valuable time to learn, because I'm interested in
filesystems in general and ZFS in particular. If not, I'm pretty sure
everything I've written above is still true.

Regards,

Zooko Wilcox-O'Hearn

Founder, CEO, and Customer Support Rep
https://LeastAuthority.com
Freedom matters.


---
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/22842876-ced276b8
Modify Your Subscription: 
https://www.listbox.com/member/?member_id=22842876id_secret=22842876-4984dade
Powered by Listbox: http://www.listbox.com
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-23 Thread Joachim Strömbergson
Aloha!

CodesInChaos wrote:
 My argument concerning performance is that for long messages SipHash
 isn't actually significantly faster than (possibly round reduced) MD5,
 Skein, Blake2 etc.

Do you have any pointers to benchmarks that show this? The SipHash paper
shows significant performance gains compared to MD5 for long messages.
Besides that the fact that you _never_ shall use MD5 for new designs and
unless forced to. A reduced round even less so.

-- 
Med vänlig hälsning, Yours

Joachim Strömbergson - Alltid i harmonisk svängning.

___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-22 Thread CodesInChaos
My argument concerning performance is that for long messages SipHash
isn't actually significantly faster than (possibly round reduced) MD5,
Skein, Blake2 etc.

The main selling point of SipHash is that it's faster than normal
crypto hashes for short messages. Since ZFS almost always hashes long
messages,
the advantage of SipHash doesn't matter.
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-22 Thread Zooko Wilcox-OHearn
On Tue, Oct 22, 2013 at 6:05 AM, Schlacta, Christ aarc...@aarcane.org wrote:

 If any weakened algorithm is to be implemented, how can we know how weak is 
 too weak, and how strong is sufficient?  Each professional Cryptographer has 
 given different opinions and all those at our immediate disposal have now 
 been biased.

A good way to do that is use an algorithm that has attracted interest
from a large number of independent cryptographers. If many
cryptographers have invested extensive effort trying to find
weaknesses in a algorithm, and haven't reported any, then we can feel
more confident that it is less likely to harbor undiscovered
weaknesses.

Among the algorithms we've been talking about in this thread, SHA-256,
HMAC-MD5, Skein, Keccak, and BLAKE are all in this category of being
well-studied.

Cryptographers publish it if they find a weakness in a reduced-round
variant of an important algorithm. You can see a summary of the best
results against weakened variants of BLAKE in ¹ (Table 1).

¹ http://eprint.iacr.org/2013/467

The rows labeled perm. and cf. are attacks on just one component
of the hash, not the whole algorithm. The # Rounds column shows how
many rounds of a reduced-round variant would be vulnerable to that
attack.

Don't forget to look at the Complexity column, too! That shows
(roughly) how many calculations would be necessary to implement the
attack. Yes, almost all of them are computations that are completely
impossible for anyone to actually execute in the forseeable future.
But still, they are the best attack that anyone has (publicly) come up
with against those weakened variants of BLAKE so they serve as a
heuristic indicator of how strong it is.

Among the well-studied algorithms listed above, BLAKE is one of the
best-studied. It was one of the five finalists in the SHA-3 contest,
and in the final report of the contest ², NIST wrote “The
cryptanalysis performed on BLAKE […] appears to have a great deal of
depth”. Here is a list of research reports that analyzed BLAKE: ³.

² http://dx.doi.org/10.6028/NIST.IR.7896
³ https://131002.net/blake/#cr

Now, BLAKE2 is not necessarily as secure as BLAKE. We could have
accidentally introduced weaknesses into BLAKE2 when making tweaks to
optimize it. The paper ¹ looked for such weaknesses and reported that
they found nothing to make them distrust BLAKE2.

We use a stream cipher named ChaCha ⁴,⁵ as the core of BLAKE and
BLAKE2, and nobody has found any weakness in ChaCha. Again, that
doesn't mean we didn't manage to screw it up somehow, but I think it
helps! If anyone found a weakness in ChaCha, it would *probably* also
show them a weakness in BLAKE2, and vice versa.

⁴ https://en.wikipedia.org/wiki/ChaCha_%28cipher%29#ChaCha_variant
⁵ https://tools.ietf.org/html/draft-agl-tls-chacha20poly1305-02

In sum, there has been a lot of independent analysis of BLAKE2, BLAKE,
and ChaCha, and I hope there will be more in the future. If you use a
reduced-round version of BLAKE2, you can look at these results to see
whether anyone has published an attack that would break that
reduced-round version. Of course, more rounds is safer against future
breakthroughs.

It was in that context that I recommended that ZFS use the most rounds
of BLAKE2 that it can while still being faster than Edon-R. ☺ That
will probably be around 5 rounds.

Regards,

Zooko Wilcox-O'Hearn

Founder, CEO, and Customer Support Rep
https://LeastAuthority.com
Freedom matters.


---
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=22842876id_secret=22842876-a25d3366
Powered by Listbox: http://www.listbox.com
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-21 Thread Joachim Strömbergson
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Aloha!


Eugen Leitl wrote:
 The reason is purely for dedup and pretty much nothing else. As such,
 we only need a hash with a good pseudo-random output distribution
 and collision resistance. We don't specifically need it to be
 super-secure. The salted hashing support I added was simply to
 silence the endless stream of wild hypotheticals on security.

If that is all you want, have you considered SipHash? It is much faster
than the other algorithms, yet more secure than CityHash, Murmurhash and
friends. And it provides an IV/salt to make it per instance unique.

https://131002.net/siphash/

Designed by DJB and Aumasson, the latter the designer of BLAKE and
BLAKE2 which you referred.

(Sorry to butt in and if I might have suggested something you already know.)

- -- 
Med vänlig hälsning, Yours

Joachim Strömbergson - Alltid i harmonisk svängning.

-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.18 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlJk1jsACgkQZoPr8HT30QG3CwCgzh4wjtVibnVTAsocqtGpkig/
yQsAoMtZujs8AH7v5SawXWl/06ahlfSb
=2Ps4
-END PGP SIGNATURE-
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-21 Thread CodesInChaos
 If that is all you want, have you considered SipHash? It is much faster
 than the other algorithms, yet more secure than CityHash, Murmurhash and
 friends. And it provides an IV/salt to make it per instance unique.

Is SipHash really that fast in this context? AFAIK it's only much
faster for short strings, since its block size is so small.

The downsides of SipHash are:

* lack of collision resistance when the key is known
* small 64 bit output, which means that collisions will happen
frequently and need to be handled

SipHash is great for use in hash tables, where it's fine if a bucket
contains two or three items, but I don't think it's a good match for ZFS.
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-19 Thread Eugen Leitl
- Forwarded message from Pawel Jakub Dawidek p...@freebsd.org -

Date: Sat, 19 Oct 2013 13:26:08 +0200
From: Pawel Jakub Dawidek p...@freebsd.org
To: z...@lists.illumos.org
Subject: Re: [zfs] [Review] 4185 New hash algorithm support
Message-ID: 20131019112608.gf1...@garage.freebsd.pl
User-Agent: Mutt/1.5.21 (2010-09-15)
Reply-To: z...@lists.illumos.org

On Mon, Oct 07, 2013 at 11:18:21PM +0100, Saso Kiselkov wrote:
 On 10/7/13 10:17 PM, Zooko Wilcox-OHearn wrote:
  So, before I go on with my pitch for why you should consider BLAKE2,
  first please clarify for me whether ZFS really needs a
  collision-resistant hash function, or whether it needs only a MAC. I
  had thought until now that ZFS doesn't need a collision-resistant hash
  unless dedup is turned on, and that if dedup is turned on it needs a
  collision-resistant hash.
 
 The reason is purely for dedup and pretty much nothing else. As such, we
 only need a hash with a good pseudo-random output distribution and
 collision resistance. We don't specifically need it to be super-secure.
 The salted hashing support I added was simply to silence the endless
 stream of wild hypotheticals on security.

Just FYI, Richard Yao just proposed providing VM images with existing
ZFS pool also for production use. This is great idea, but also a nice
proof why making assumptions on how exactly a general purpose software
will be used is bad. In this case your salted hashing is pointless as
everyone knows about the salt in those images. And we are back to square
one.

Saso, there are countless examples where so called hypothetical security
bugs turned out to be exploitable in practise. Which is especially true
for general purpose software that we have no control over how it is
being used.

As Zooko mentioned cryptoanalysis of the Edon-R algorithm stopped at the
point where we know it is not cryptographically secure and this is
unlikely we will see any further work in the subject, which in my eyes
is even worse, as we don't know how bad is it.

To sum up. Even if the OpenZFS community will agree to integrate Edon-R,
I'll strongly oppose having it in FreeBSD. In my opinion it is just
asking for trouble. I still like your change very much and would love to
see new, cryptographically secure hash algorithms for dedup in ZFS.
Currently there is no alternative, which is bad for security too.

-- 
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=22842876id_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
___
cryptography mailing list
cryptography@randombit.net
http://lists.randombit.net/mailman/listinfo/cryptography


Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-08 Thread Eugen Leitl
- Forwarded message from Saso Kiselkov skiselkov...@gmail.com -

Date: Mon, 07 Oct 2013 23:18:21 +0100
From: Saso Kiselkov skiselkov...@gmail.com
To: z...@lists.illumos.org
CC: Zooko Wilcox-OHearn zo...@leastauthority.com
Subject: Re: [zfs] [Review] 4185 New hash algorithm support
Message-ID: 5253332d.6090...@gmail.com
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) 
Gecko/20130801 Thunderbird/17.0.8
Reply-To: z...@lists.illumos.org

On 10/7/13 10:17 PM, Zooko Wilcox-OHearn wrote:
 Hi folks:
 
 I just joined this list because I saw this thread. I'm one of the
 architects of a distributed storage system named Tahoe-LAFS
 (https://Tahoe-LAFS.org). It has quite a few things in common with
 ZFS, architecturally, but it is also very different from ZFS, because
 it's not so much a real *filesystem* as it is like a BitTorrent that
 has an upload button as well as a download button. But it is like ZFS
 inasmuch as they both involve a heck of a lot of hashing for
 error-detection.
 
 [..snip..]

Hi,

Thanks for joining, I'm glad we got somebody educated on the matters in
on the discussion! I studied BLAKE for inclusion instead of Skein, but
its performance was worse at the time. BLAKE2 is a different beast
(explained below).

 So, before I go on with my pitch for why you should consider BLAKE2,
 first please clarify for me whether ZFS really needs a
 collision-resistant hash function, or whether it needs only a MAC. I
 had thought until now that ZFS doesn't need a collision-resistant hash
 unless dedup is turned on, and that if dedup is turned on it needs a
 collision-resistant hash.

The reason is purely for dedup and pretty much nothing else. As such, we
only need a hash with a good pseudo-random output distribution and
collision resistance. We don't specifically need it to be super-secure.
The salted hashing support I added was simply to silence the endless
stream of wild hypotheticals on security.

 But this thread seems to indicate that even when dedup is turned on,
 it might be possible to use a MAC, by having a pool-wide secret to use
 for the MAC key… If I understand correctly (which I probably don't),
 that would make it impossible for anyone who doesn't know the secret
 to cause collisions during dedup, but still possible for someone who
 knows the secret (presumably root on that system, or someone who stole
 the secret) to generate blocks that would collide during dedup. If you
 used a collision-resistant hash for that purpose, then nobody would be
 able to cause collisions.

Oh boy, I already regret having referred to the feature in the block
comments as a MAC... it's not and was never intended to be. It's really
just a way to mitigate fears of the potential exploitation of a
not-quite-secure hash function (Edon-R mostly). It is not meant to
authenticate data on the pool to the machine itself (how could it if we
store the key unencrypted on the pool anyway?), or protect us from
somebody who had physical access to the underlying pool. It's just to
prevent attacks on the dedup tables.

 If you need a MAC, I suggest Poly1305-AES. It is very efficient, has a
 nice proof that it is as secure as AES is, and it is part of a new
 proposed cipher suite for TLS ².
 
 ² http://tools.ietf.org/html/draft-agl-tls-chacha20poly1305-01

Looked at it and looks fairly similar to the VMAC that Radio młodych
bandytów suggested. This is a MAC, whereas we need a hash (for dedup).

 If you need a collision-resistant hash function, I suggest BLAKE2. It
 is more efficient than SHA-256, Skein, or Keccak (see ³), and it has a
 better reputation among cryptographers than Edon-R has. In fact,
 BLAKE2's parent, BLAKE, was rated by NIST as being even more
 well-studied than Keccak was — see my slides, linked above, for quotes
 from NIST's final report on the SHA-3 contest.
 
 [...snip...]
 
 You can get the academic papers, source code (both simple reference
 implementations and optimized implementations in various languages),
 test vectors, and so on:
 
 https://blake2.net

I tried BLAKE2 on my machine and it is true that it provides a slight
performance boost over Skein, though it doesn't even approach Edon-R.
For non-trivial reasons we can't use floating point registers in kernel
code without jumping through many hoops (so no SSE/AVX), plus we need to
be able to run on SPARC, but the pure 64-bit C implementation is a bit
faster than Skein, ~5.1 CPB for BLAKE2 vs ~5.9 CPB for Skein on my Core
i5 test rig. On an Athlon II 1.3 GHz it's actually a bit slower, at 7.75
CPB for BLAKE2 vs 7.45 CPB for Skein. Used the blake2b-ref.c
implementation, compiled GCC 4.4.4 with -fno-builtin -O2 -std=c99 to
mirror in-kernel build environment.

Overall, I'd say: I don't know. I've already sunk a fair amount of
effort to get Skein to work well and integrated with the KCF and ZFS and
I'm not exactly willing to jump in and expend more effort to get another
-0.1 to +0.7 CPB...

Cheers,
-- 
Saso



Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-07 Thread Eugen Leitl
- 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=22842876id_secret=22842876-a25d3366
Powered by Listbox: http://www.listbox.com



- End forwarded message -
-- 
Eugen* Leitl a 

Re: [cryptography] [zfs] [Review] 4185 New hash algorithm support

2013-10-07 Thread Eugen Leitl
- Forwarded message from Saso Kiselkov skiselkov...@gmail.com -

Date: Mon, 07 Oct 2013 13:04:52 +0100
From: Saso Kiselkov skiselkov...@gmail.com
To: z...@lists.illumos.org
CC: Pawel Jakub Dawidek p...@freebsd.org
Subject: Re: [zfs] [Review] 4185 New hash algorithm support
Message-ID: 5252a364.4000...@gmail.com
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) 
Gecko/20130801 Thunderbird/17.0.8
Reply-To: z...@lists.illumos.org

On 10/7/13 10:44 AM, Pawel Jakub Dawidek wrote:
 I do like your patch and addition of first two algorithms, but I have to
 comment on the third one.

I sort of expected you'd be among the first ones :)

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

Both Edon-R and Skein are resistant to length-extension attacks, so
simply prepending the salt to the input is sufficient (in fact, I use
Skein's built-in support for MACs, which does just that). There is, as
far as I can tell, one paper which demonstrates a practical secret-IV
recovery attack on Edon-R, but it requires that the attacker be able to
observe the algorithm's output (which in our case they can't) and it
only works on Edonr-R-256 and doesn't work on the tweaked version (which
is part of this webrev). The paper in question is:
http://www.di.ens.fr/~leurent/files/Edon_RSA10.pdf
It is possible trivially avert this attack by switching over to using
HMAC, but I see this as pointless, as so many of the attack's
fundamental requirements aren't met in our environment.

 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.

You should probably best look at implementing straight encryption when
you're already doing this. The target isn't that far removed.

 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.

That would require a version bump of the send stream format, which might
make interoperability a bit more thorny... not too hard I guess, but I
don't currently see a need for it (perhaps somebody might request it
later on).

 Few minor observations:
 
 --- old/usr/src/lib/libzfs/common/libzfs_dataset.cMon Oct  7 09:23:07 2013
 +++ new/usr/src/lib/libzfs/common/libzfs_dataset.cMon 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?

No, this is to cover the ERANGE return from zfs_ioctl.c:3784 where we
check to make sure nobody enables any salted checksums on a root pool,
since GRUB doesn't have support for that feature. Bootloader support on
FreeBSD might differ, though.

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

Nice macro, but on Illumos it doesn't appear to exist. So I copied it
and used it :) Thanks for the suggestion. Webrev updated.

 + ctx = kmem_zalloc(sizeof (*ctx), KM_SLEEP);
 
 For Skein you also use KM_PUSHPAGE flag.

That was a leftover, removed it now. Thanks for spotting this.

Cheers,
-- 
Saso


---
illumos-zfs
Archives: