Add checks that prefix counts are consistent across multiple VDAF exe…

…cutions (#332)

Co-authored-by: Christopher Patton <cpatton@cloudflare.com>
Co-authored-by: David Cook <dcook@divviup.org>

draft-irtf-cfrg-vdaf.md

@@ -1157,7 +1157,7 @@ been used with the same input share.
 
 VDAFs MUST implement the following function:
 
-* `Vdaf.is_valid(agg_param: AggParam, previous_agg_params: set[AggParam]) ->
+* `Vdaf.is_valid(agg_param: AggParam, previous_agg_params: list[AggParam]) ->
   Bool`: Checks if the `agg_param` is compatible with all elements of
   `previous_agg_params`.
 
@@ -2550,7 +2550,7 @@ Every input share MUST only be used once, regardless of the aggregation
 parameters used.
 
 ~~~
-def is_valid(agg_param, previous_agg_params):
+def is_valid(Prio3, agg_param, previous_agg_params):
     return len(previous_agg_params) == 0
 ~~~
 {: #prio3-validity-scope title="Validity of aggregation parameters for Prio3."}
@@ -3952,15 +3952,36 @@ def prep_shares_to_prep(Poplar1, agg_param, prep_shares):
 Aggregation parameters are valid for a given input share if no aggregation
 parameter with the same level has been used with the same input share before.
 The whole preparation phase MUST NOT be run more than once for a given
-combination of input share and level.
+combination of input share and level. This function checks that levels are
+increasing between calls, and also enforces that the prefixes at each level are
+suffixes of the previous level's prefixes.
 
 ~~~
-def is_valid(agg_param, previous_agg_params):
-    (level, _) = agg_param
-    return all(
-        level != other_level
-        for (other_level, _) in previous_agg_params
-    )
+def get_ancestor(input, this_level, last_level):
+    # Helper function to determine the prefix of `input` at `last_level`.
+    return input >> (this_level - last_level)
+
+def is_valid(Poplar1, agg_param, previous_agg_params):
+    # Exit early if this is the first time.
+    if len(previous_agg_params) < 1:
+      return True
+
+    (level, prefixes) = agg_param
+    (last_level, last_prefixes) = previous_agg_params[-1]
+    last_prefixes_set = set(last_prefixes)
+
+    # Check that the level increased.
+    if level <= last_level
+      return False
+
+    # Check that prefixes are suffixes of the last level's prefixes.
+    for prefix in prefixes:
+      last_prefix = get_ancestor(prefix, level, last_level)
+      if last_prefix not in last_prefixes_set:
+        # Current prefix not a suffix of last level's prefixes.
+        return False
+
+    return True
 ~~~
 {: #poplar1-validity-scope title="Validity of aggregation parameters for
 Poplar1."}
@@ -4584,8 +4605,16 @@ perhaps unlikely) for a large set of non-heavy-hitter values to share a common
 prefix, which would be leaked by a prefix tree with a sufficiently small
 threshold.
 
-The only known, general-purpose approach to mitigating this leakage is via
-differential privacy.
+A malicious adversary controlling the Collector and one of the Aggregators can
+further turn arbitrary non-heavy prefixes into heavy ones by tampering with the
+IDPF output at any position. While our construction ensures that the nodes
+evaluated at one level are children of the nodes evaluated at the previous
+level, this still may allow an adversary to discover individual non-heavy
+strings.
+
+The only practical, general-purpose approach to mitigating these leakages is via
+differential privacy, which is RECOMMENDED for all protocols using Poplar1 for
+heavy-hitter type applications.
 
 ### Safe Usage of IDPF Outputs
 

poc/tests/test_vdaf_poplar1.py

@@ -2,7 +2,7 @@
 
 from common import TEST_VECTOR, from_be_bytes
 from tests.vdaf import test_vdaf
-from vdaf_poplar1 import Poplar1
+from vdaf_poplar1 import Poplar1, get_ancestor
 
 
 class TestPoplar1(unittest.TestCase):
@@ -66,13 +66,40 @@ def test_poplar1(self):
             [0, 2],
         )
 
+    def test_get_ancestor(self):
+        # No change.
+        assert get_ancestor(0b0, 0, 0) == 0b0
+        assert get_ancestor(0b100, 0, 0) == 0b100
+        assert get_ancestor(0b0, 1, 1) == 0b0
+        assert get_ancestor(0b100, 1, 1) == 0b100
+
+        # Shift once.
+        assert get_ancestor(0b0, 1, 0) == 0b0
+        assert get_ancestor(0b1, 1, 0) == 0b0
+        assert get_ancestor(0b100, 1, 0) == 0b10
+        assert get_ancestor(0b100, 2, 1) == 0b10
+
+        # Shift twice.
+        assert get_ancestor(0b0, 2, 0) == 0b0
+        assert get_ancestor(0b10, 2, 0) == 0b0
+        assert get_ancestor(0b100, 2, 0) == 0b1
+        assert get_ancestor(0b100, 4, 2) == 0b1
+
     def test_is_valid(self):
         # Test `is_valid` returns False on repeated levels, and True otherwise.
         cls = Poplar1.with_bits(256)
-        agg_params = [(0, ()), (1, (0,)), (1, (0, 1))]
-        assert cls.is_valid(agg_params[0], set([]))
-        assert cls.is_valid(agg_params[1], set(agg_params[:1]))
-        assert not cls.is_valid(agg_params[2], set(agg_params[:2]))
+        agg_params = [(0, (0b0, 0b1)), (1, (0b00,)), (1, (0b00, 0b10))]
+        assert cls.is_valid(agg_params[0], list([]))
+        assert cls.is_valid(agg_params[1], list(agg_params[:1]))
+        assert not cls.is_valid(agg_params[2], list(agg_params[:2]))
+
+        # Test `is_valid` accepts level jumps.
+        agg_params = [(0, (0b0, 0b1)), (2, (0b010, 0b011, 0b101, 0b111))]
+        assert cls.is_valid(agg_params[1], list(agg_params[:1]))
+
+        # Test `is_valid` rejects unconnected prefixes.
+        agg_params = [(0, (0b0,)), (2, (0b010, 0b011, 0b101, 0b111))]
+        assert not cls.is_valid(agg_params[1], list(agg_params[:1]))
 
     def test_aggregation_parameter_encoding(self):
         # Test aggregation parameter encoding.

poc/vdaf_poplar1.py

@@ -151,16 +151,31 @@ def shard(Poplar1, measurement, nonce, rand):
         input_shares = list(zip(keys, corr_seed, corr_inner, corr_leaf))
         return (public_share, input_shares)
 
-    def is_valid(agg_param, previous_agg_params):
+    @classmethod
+    def is_valid(Poplar1, agg_param, previous_agg_params):
         """
-        Checks if the level of `agg_param` appears anywhere in
-        `previous_agg_params`. Returns `False` if it does, and `True` otherwise.
+        Checks that levels are increasing between calls, and also enforces that
+        the prefixes at each level are suffixes of the previous level's
+        prefixes.
         """
-        (level, _) = agg_param
-        return all(
-            level != other_level
-            for (other_level, _) in previous_agg_params
-        )
+        if len(previous_agg_params) < 1:
+            return True
+
+        (level, prefixes) = agg_param
+        (last_level, last_prefixes) = previous_agg_params[-1]
+        last_prefixes_set = set(last_prefixes)
+
+        # Check that level increased.
+        if level <= last_level:
+            return False
+
+        # Check that prefixes are suffixes of the last level's prefixes.
+        for prefix in prefixes:
+            last_prefix = get_ancestor(prefix, level, last_level)
+            if last_prefix not in last_prefixes_set:
+                # Current prefix not a suffix of last level's prefixes.
+                return False
+        return True
 
     @classmethod
     def prep_init(Poplar1, verify_key, agg_id, agg_param,
@@ -378,3 +393,10 @@ def encode_idpf_field_vec(vec):
         Field = vec[0].__class__
         encoded += Field.encode_vec(vec)
     return encoded
+
+
+def get_ancestor(input, this_level, last_level):
+    """
+    Helper function to determine the prefix of `input` at `last_level`.
+    """
+    return input >> (this_level - last_level)