From 512897eca0310ce6ddfe262520f3c21bcf345ccf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 19 Jan 2021 13:46:32 -0500 Subject: [PATCH 1/4] news fragment --- newsfragments/3592.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3592.minor diff --git a/newsfragments/3592.minor b/newsfragments/3592.minor new file mode 100644 index 000000000..e69de29bb From 61d5f920bb9db703edb9f3aed73f2d0488ee0d2c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 19 Jan 2021 14:28:16 -0500 Subject: [PATCH 2/4] Add tests for the tag construction code and make it a bit safer Check for sane inputs, reject insane ones --- src/allmydata/test/test_hashutil.py | 36 +++++++++++++++++++++++++++++ src/allmydata/util/hashutil.py | 29 ++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_hashutil.py b/src/allmydata/test/test_hashutil.py index 6ec861c9f..6ac73ae60 100644 --- a/src/allmydata/test/test_hashutil.py +++ b/src/allmydata/test/test_hashutil.py @@ -126,6 +126,42 @@ class HashUtilTests(unittest.TestCase): base32.a2b(b"2ckv3dfzh6rgjis6ogfqhyxnzy"), ) + def test_convergence_hasher_tag(self): + """ + ``_convergence_hasher_tag`` constructs the convergence hasher tag from a + unique prefix, the required, total, and segment size parameters, and a + convergence secret. + """ + self.assertEqual( + "allmydata_immutable_content_to_key_with_added_secret_v1+" + "16:\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42," + "9:3,10,1024,", + hashutil._convergence_hasher_tag( + k=3, + n=10, + segsize=1024, + convergence=b"\x42" * 16, + ), + ) + + def test_convergence_hasher_out_of_bounds(self): + """ + ``_convergence_hasher_tag`` raises ``ValueError`` if k or n is not between + 1 and 256 inclusive or if k is greater than n. + """ + segsize = 1024 + secret = b"\x42" * 16 + for bad_k in (0, 2, 257): + with self.assertRaises(ValueError): + hashutil._convergence_hasher_tag( + k=bad_k, n=1, segsize=segsize, convergence=secret, + ) + for bad_n in (0, 1, 257): + with self.assertRaises(ValueError): + hashutil._convergence_hasher_tag( + k=2, n=bad_n, segsize=segsize, convergence=secret, + ) + def test_known_answers(self): """ Verify backwards compatibility by comparing hash outputs for some diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py index ebb2f12af..f82c04efd 100644 --- a/src/allmydata/util/hashutil.py +++ b/src/allmydata/util/hashutil.py @@ -176,10 +176,37 @@ def convergence_hash(k, n, segsize, data, convergence): return h.digest() -def convergence_hasher(k, n, segsize, convergence): +def _convergence_hasher_tag(k, n, segsize, convergence): + """ + Create the convergence hashing tag. + + :param int k: Required shares. + :param int n: Total shares. + :param int segsize: Maximum segment size. + :param bytes convergence: The convergence secret. + + :return bytes: The bytestring to use as a tag in the convergence hash. + """ assert isinstance(convergence, bytes) + if k > n: + raise ValueError( + "k > n not allowed; k = {}, n = {}".format(k, n), + ) + if k < 1 or n < 1: + raise ValueError( + "k, n < 1 not allowed; k = {}, n = {}".format(k, n), + ) + if k > 256 or n > 256: + raise ValueError( + "k, n > 256 not allowed; k = {}, n = {}".format(k, n), + ) param_tag = netstring(b"%d,%d,%d" % (k, n, segsize)) tag = CONVERGENT_ENCRYPTION_TAG + netstring(convergence) + param_tag + return tag + + +def convergence_hasher(k, n, segsize, convergence): + tag = _convergence_hasher_tag(k, n, segsize, convergence) return tagged_hasher(tag, KEYLEN) From 5568170c243ab717221571ca61a48c9a717dadbf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 11 Feb 2021 15:46:04 -0500 Subject: [PATCH 3/4] Slightly better docs for the share count limits on convergence hash tag --- src/allmydata/util/hashutil.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py index f82c04efd..8525dd95e 100644 --- a/src/allmydata/util/hashutil.py +++ b/src/allmydata/util/hashutil.py @@ -180,8 +180,8 @@ def _convergence_hasher_tag(k, n, segsize, convergence): """ Create the convergence hashing tag. - :param int k: Required shares. - :param int n: Total shares. + :param int k: Required shares (in [1..256]). + :param int n: Total shares (in [1..256]). :param int segsize: Maximum segment size. :param bytes convergence: The convergence secret. @@ -193,10 +193,17 @@ def _convergence_hasher_tag(k, n, segsize, convergence): "k > n not allowed; k = {}, n = {}".format(k, n), ) if k < 1 or n < 1: + # It doesn't make sense to have zero shares. Zero shares carry no + # information, cannot encode any part of the application data. raise ValueError( "k, n < 1 not allowed; k = {}, n = {}".format(k, n), ) if k > 256 or n > 256: + # ZFEC supports encoding application data into a maximum of 256 + # shares. If we ignore the limitations of ZFEC, it may be fine to use + # a configuration with more shares than that and it may be fine to + # construct a convergence tag from such a configuration. Since ZFEC + # is the only supported encoder, though, this is moot for now. raise ValueError( "k, n > 256 not allowed; k = {}, n = {}".format(k, n), ) From a8b1c204d2b836e70d84f8ae9179fba523648059 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 11 Feb 2021 16:06:18 -0500 Subject: [PATCH 4/4] Mark the expected result literal as the correct type, bytes --- src/allmydata/test/test_hashutil.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_hashutil.py b/src/allmydata/test/test_hashutil.py index 6ac73ae60..482e79c0b 100644 --- a/src/allmydata/test/test_hashutil.py +++ b/src/allmydata/test/test_hashutil.py @@ -133,9 +133,9 @@ class HashUtilTests(unittest.TestCase): convergence secret. """ self.assertEqual( - "allmydata_immutable_content_to_key_with_added_secret_v1+" - "16:\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42," - "9:3,10,1024,", + b"allmydata_immutable_content_to_key_with_added_secret_v1+" + b"16:\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42," + b"9:3,10,1024,", hashutil._convergence_hasher_tag( k=3, n=10,