From 4fb2546ae805bed1b906849ad041a6f54efbe546 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 17 Dec 2021 13:44:01 -0500
Subject: [PATCH] Completely disallow non-None `new_length` in the mutable
protocol
---
src/_zkapauthorizer/_storage_server.py | 18 ++++++
src/_zkapauthorizer/tests/strategies.py | 14 ++---
.../tests/test_storage_server.py | 60 ++++++++++++++++++-
3 files changed, 83 insertions(+), 9 deletions(-)
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index b847499..6599cc5 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -84,6 +84,17 @@ SLOT_HEADER_SIZE = 468
LEASE_TRAILER_SIZE = 4
+class NewLengthRejected(Exception):
+ """
+ A non-None value for ``new_length`` was given to
+ ``slot_testv_and_readv_and_writev``.
+
+ This is disallowed by ZKAPAuthorizer because of the undesirable
+ interactions with the current spending protocol and because there are no
+ known real-world use-cases for this usage.
+ """
+
+
@attr.s
class _ValidationResult(object):
"""
@@ -438,6 +449,13 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# part of this call.
now = self._clock.seconds()
+ # We're not exactly sure what to do with mutable container truncations
+ # and the official client doesn't ever use that feature so just
+ # disable it by rejecting all attempts here.
+ for (testv, writev, new_length) in tw_vectors.values():
+ if new_length is not None:
+ raise NewLengthRejected(new_length)
+
# Check passes for cryptographic validity.
validation = _ValidationResult.validate_passes(
slot_testv_and_readv_and_writev_message(storage_index),
diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py
index c439874..61b4019 100644
--- a/src/_zkapauthorizer/tests/strategies.py
+++ b/src/_zkapauthorizer/tests/strategies.py
@@ -917,13 +917,13 @@ def slot_test_and_write_vectors():
TestAndWriteVectors,
slot_test_vectors(),
slot_data_vectors(),
- one_of(
- # The new length might be omitted completely.
- just(None),
- # Or it might be given as an integer. Allow a zero size which
- # means "delete this share" in this context.
- sizes(),
- ),
+ # The underlying Tahoe-LAFS storage protocol allows None or an integer
+ # here (new_length) to set the size of the mutable container. The
+ # real Tahoe-LAFS storage client never uses this feature and always
+ # passes None. There are undesirable interactions between non-None
+ # values for new_length and our spending protocol. Therefore, we
+ # disable non-None values. So don't generate any here.
+ just(None),
)
diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py
index ba28632..fadd948 100644
--- a/src/_zkapauthorizer/tests/test_storage_server.py
+++ b/src/_zkapauthorizer/tests/test_storage_server.py
@@ -24,14 +24,14 @@ from time import time
from allmydata.storage.mutable import MutableShareFile
from challenge_bypass_ristretto import random_signing_key
from foolscap.referenceable import LocalReferenceable
-from hypothesis import given, note
+from hypothesis import given, note, assume
from hypothesis.strategies import integers, just, lists, one_of, tuples
from testtools import TestCase
from testtools.matchers import AfterPreprocessing, Equals, MatchesAll
from twisted.internet.task import Clock
from twisted.python.runtime import platform
-from .._storage_server import _ValidationResult
+from .._storage_server import _ValidationResult, NewLengthRejected
from ..api import MorePassesRequired, ZKAPAuthorizerStorageServer
from ..storage_common import (
add_lease_message,
@@ -428,6 +428,62 @@ class PassValidationTests(TestCase):
),
)
+ @given(
+ storage_index=storage_indexes(),
+ secrets=tuples(
+ write_enabler_secrets(),
+ lease_renew_secrets(),
+ lease_cancel_secrets(),
+ ),
+ sharenums=sharenum_sets(),
+ test_and_write_vectors_for_shares=slot_test_and_write_vectors_for_shares(),
+ new_length=integers(),
+ )
+ def test_mutable_new_length_rejected(
+ self, storage_index, secrets, sharenums, test_and_write_vectors_for_shares, new_length,
+ ):
+ """
+ If ``new_length`` is not ``None`` then ``slot_testv_and_readv_and_writev``
+ rejects the operation.
+ """
+ tw_vectors = {
+ k: v.for_call() for (k, v) in test_and_write_vectors_for_shares.items()
+ }
+ # Change some tw_vector to have a non-None new_length.
+ sharenum, (testv, writev, ignored) = tw_vectors.popitem()
+ tw_vectors[sharenum] = (testv, writev, new_length)
+
+ required_pass_count = get_required_new_passes_for_mutable_write(
+ self.pass_value,
+ dict.fromkeys(tw_vectors.keys(), 0),
+ tw_vectors,
+ )
+ valid_passes = get_passes(
+ slot_testv_and_readv_and_writev_message(storage_index),
+ required_pass_count,
+ self.signing_key,
+ )
+
+ # Try to do a write with the non-None new_length and expect it to be
+ # rejected.
+ try:
+ result = self.storage_server.doRemoteCall(
+ "slot_testv_and_readv_and_writev",
+ (),
+ dict(
+ passes=_encode_passes(valid_passes),
+ storage_index=storage_index,
+ secrets=secrets,
+ tw_vectors=tw_vectors,
+ r_vector=[],
+ ),
+ )
+ except NewLengthRejected:
+ pass
+ else:
+ self.fail("expected a failure but got {!r}".format(result))
+
+
@given(
storage_index=storage_indexes(),
secrets=tuples(
--
GitLab