Some clarifications for SecretSharing

lib/Crypto/Protocol/SecretSharing.py

@@ -77,7 +77,8 @@ def _div_gf2(a, b):
 class _Element(object):
     """Element of GF(2^128) field"""
 
-    # The irreducible polynomial defining this field is 1+x+x^2+x^7+x^128
+    # The irreducible polynomial defining
+    # this field is 1 + x + x^2 + x^7 + x^128
     irr_poly = 1 + 2 + 4 + 128 + 2 ** 128
 
     def __init__(self, encoded_value):
@@ -178,43 +179,53 @@ class Shamir(object):
 
         Args:
           k (integer):
-            The sufficient number of shares to reconstruct the secret (``k < n``).
+            The number of shares needed to reconstruct the secret.
           n (integer):
-            The number of shares that this method will create.
+            The number of shares to create (at least ``k``).
           secret (byte string):
-            A byte string of 16 bytes (e.g. the AES 128 key).
+            A byte string of 16 bytes (e.g. an AES 128 key).
           ssss (bool):
-            If ``True``, the shares can be used with the ``ssss`` utility.
+            If ``True``, the shares can be used with the ``ssss`` utility
+            (without using the "diffusion layer").
             Default: ``False``.
 
         Return (tuples):
-            ``n`` tuples. A tuple is meant for each participant and it contains two items:
+            ``n`` tuples, one per participant.
+            A tuple contains two items:
 
             1. the unique index (an integer)
-            2. the share (a byte string, 16 bytes)
+            2. the share (16 bytes)
         """
 
         #
         # We create a polynomial with random coefficients in GF(2^128):
         #
-        # p(x) = \sum_{i=0}^{k-1} c_i * x^i
+        # p(x) = c_0 + \sum_{i=1}^{k-1} c_i * x^i
         #
-        # c_0 is the encoded secret
+        # c_0 is the secret.
         #
 
         coeffs = [_Element(rng(16)) for i in range(k - 1)]
         coeffs.append(_Element(secret))
 
-        # Each share is y_i = p(x_i) where x_i is the public index
+        # Each share is y_i = p(x_i) where x_i
-        # associated to each of the n users.
+        # is the index assigned to the share.
 
         def make_share(user, coeffs, ssss):
             idx = _Element(user)
+
+            # Horner's method
             share = _Element(0)
             for coeff in coeffs:
                 share = idx * share + coeff
+
+            # The ssss utility actually uses:
+            #
+            # p(x) = c_0 + \sum_{i=1}^{k-1} c_i * x^i + x^k
+            #
             if ssss:
                 share += _Element(user) ** len(coeffs)
+
             return share.encode()
 
         return [(i, make_share(i, coeffs, ssss)) for i in range(1, n + 1)]
@@ -225,11 +236,18 @@ class Shamir(object):
 
         Args:
           shares (tuples):
-            The *k* tuples, each containin the index (an integer) and
+            The *k* tuples, each containing the index (an integer) and
             the share (a byte string, 16 bytes long) that were assigned to
             a participant.
+
+            .. note::
+
+                Pass exactly as many share as they are required,
+                and no more.
+
           ssss (bool):
-            If ``True``, the shares were produced by the ``ssss`` utility.
+            If ``True``, the shares were produced by the ``ssss`` utility
+            (without using the "diffusion layer").
             Default: ``False``.
 
         Return:
@@ -275,4 +293,5 @@ class Shamir(object):
                     numerator *= x_m
                     denominator *= x_j + x_m
             result += y_j * numerator * denominator.inverse()
+
         return result.encode()

lib/Crypto/SelfTest/Protocol/test_SecretSharing.py

@@ -35,11 +35,13 @@ from unittest import main, TestCase, TestSuite
 from binascii import unhexlify, hexlify
 
 from Crypto.Util.py3compat import *
+from Crypto.Hash import SHAKE128
 from Crypto.SelfTest.st_common import list_test_cases
 
 from Crypto.Protocol.SecretSharing import Shamir, _Element, \
                                           _mult_gf2, _div_gf2
 
+
 class GF2_Tests(TestCase):
 
     def test_mult_gf2(self):
@@ -129,6 +131,7 @@ class Element_Tests(TestCase):
         y = x.inverse()
         self.assertEqual(int(x * y), 1)
 
+
 class Shamir_Tests(TestCase):
 
     def test1(self):
@@ -143,6 +146,8 @@ class Shamir_Tests(TestCase):
         # Test recombine
         from itertools import permutations
 
+        # Generated by ssss (index, secret, shares)
+        # in hex mode, without "diffusion" mode
         test_vectors = (
             (2, "d9fe73909bae28b3757854c0af7ad405",
              "1-594ae8964294174d95c33756d2504170",
@@ -227,7 +232,12 @@ class Shamir_Tests(TestCase):
 
     def test3(self):
         # Loopback split/recombine
-        secret = unhexlify(b("000102030405060708090a0b0c0d0e0f"))
+
+        rng = SHAKE128.new(b"test3")
+
+        for _ in range(100):
+
+            secret = rng.read(16)
 
             shares = Shamir.split(2, 3, secret)
 
@@ -239,13 +249,26 @@ class Shamir_Tests(TestCase):
 
     def test4(self):
         # Loopback split/recombine (SSSS)
-        secret = unhexlify(b("000102030405060708090a0b0c0d0e0f"))
+
+        rng = SHAKE128.new(b"test4")
+
+        for _ in range(10):
+            secret = rng.read(16)
 
             shares = Shamir.split(2, 3, secret, ssss=True)
 
             secret2 = Shamir.combine(shares[:2], ssss=True)
             self.assertEqual(secret, secret2)
 
+        for _ in range(10):
+            secret = rng.read(16)
+
+            shares = Shamir.split(3, 7, secret, ssss=True)
+
+            secret2 = Shamir.combine([shares[3], shares[4], shares[6]], ssss=True)
+            self.assertEqual(secret, secret2)
+
+
     def test5(self):
         # Detect duplicate shares
         secret = unhexlify(b("000102030405060708090a0b0c0d0e0f"))