diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index 8bae23d92c31d59ee4405d3ccfca4c0c09f30d51..68e61b61267ae5be18fb896d0353386ee641be6c 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -24,6 +24,10 @@ from __future__ import ( absolute_import, ) +from functools import ( + partial, +) + import attr from attr.validators import ( provides, @@ -48,6 +52,14 @@ from privacypass import ( VerificationSignature, SigningKey, ) + +from twisted.python.reflect import ( + namedAny, +) +from twisted.internet.interfaces import ( + IReactorTime, +) + from .foolscap import ( RIPrivacyPassAuthorizedStorageServer, ) @@ -85,6 +97,14 @@ class MorePassesRequired(Exception): return repr(self) +class LeaseRenewalRequired(Exception): + """ + Mutable write operations fail with ``LeaseRenewalRequired`` when the slot + which is the target of the write does not have an active lease and no + passes are supplied to create one. + """ + + @implementer_only(RIPrivacyPassAuthorizedStorageServer, IReferenceable, IRemotelyCallable) # It would be great to use `frozen=True` (value-based hashing) instead of # `cmp=False` (identity based hashing) but Referenceable wants to set some @@ -97,6 +117,10 @@ class ZKAPAuthorizerStorageServer(Referenceable): """ _original = attr.ib(validator=provides(RIStorageServer)) _signing_key = attr.ib(validator=instance_of(SigningKey)) + _clock = attr.ib( + validator=provides(IReactorTime), + default=attr.Factory(partial(namedAny, "twisted.internet.reactor")), + ) def _is_invalid_pass(self, message, pass_): """ @@ -216,7 +240,32 @@ class ZKAPAuthorizerStorageServer(Referenceable): data in already-allocated storage. These cases may not be the same from the perspective of pass validation. """ - self._validate_passes(slot_testv_and_readv_and_writev_message(storage_index), passes) + renew_leases = False + + if has_writes(tw_vectors): + # Writes are allowed to shares with active leases. + if not has_active_lease( + self._original, + storage_index, + self._clock.seconds(), + ): + # Passes may be supplied with the write to create the + # necessary lease as part of the same operation. + valid_passes = self._validate_passes( + slot_testv_and_readv_and_writev_message(storage_index), + passes, + ) + sharenums = get_sharenums(tw_vectors) + allocated_size = get_allocated_size(tw_vectors) + required_pass_count = required_passes(BYTES_PER_PASS, sharenums, allocated_size) + if len(valid_passes) < required_pass_count: + raise MorePassesRequired( + len(valid_passes), + required_pass_count, + ) + + renew_leases = True + # Skip over the remotely exposed method and jump to the underlying # implementation which accepts one additional parameter that we know # about (and don't expose over the network): renew_leases. We always @@ -227,7 +276,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): secrets, tw_vectors, r_vector, - renew_leases=False, + renew_leases=renew_leases, ) def remote_slot_readv(self, *a, **kw): @@ -237,6 +286,73 @@ class ZKAPAuthorizerStorageServer(Referenceable): """ return self._original.remote_slot_readv(*a, **kw) + +def has_writes(tw_vectors): + """ + :param tw_vectors: See + ``allmydata.interfaces.TestAndWriteVectorsForShares``. + + :return bool: ``True`` if any only if there are writes in ``tw_vectors``. + """ + return any( + data + for (test, data, new_length) + in tw_vectors.values() + ) + + +def get_sharenums(tw_vectors): + """ + :param tw_vectors: See + ``allmydata.interfaces.TestAndWriteVectorsForShares``. + + :return set[int]: The share numbers which the given test/write vectors would write to. + """ + return set( + sharenum + for (sharenum, (test, data, new_length)) + in tw_vectors.items() + if data + ) + + +def get_allocated_size(tw_vectors): + """ + :param tw_vectors: See + ``allmydata.interfaces.TestAndWriteVectorsForShares``. + + :return int: The largest position ``tw_vectors`` writes in any share. + """ + return max( + list( + max(offset + len(s) for (offset, s) in data) + for (sharenum, (test, data, new_length)) + in tw_vectors.items() + if data + ), + ) + +def has_active_lease(storage_server, storage_index, now): + """ + :param allmydata.storage.server.StorageServer storage_server: A storage + server to use to look up lease information. + + :param bytes storage_index: A storage index to use to look up lease + information. + + :param float now: The current time as a POSIX timestamp. + + :return bool: ``True`` if any only if the given storage index has a lease + with an expiration time after ``now``. + """ + leases = storage_server.get_slot_leases(storage_index) + return any( + lease.get_expiration_time() > now + for lease + in leases + ) + + # I don't understand why this is required. # ZKAPAuthorizerStorageServer is-a Referenceable. It seems like # the built in adapter should take care of this case. diff --git a/src/_zkapauthorizer/api.py b/src/_zkapauthorizer/api.py index 8b89611ba10e3ee3833f2d5c7c45d1d8365ee320..365e39a23026e1b7692267d12c895f45cfaeda64 100644 --- a/src/_zkapauthorizer/api.py +++ b/src/_zkapauthorizer/api.py @@ -14,6 +14,7 @@ __all__ = [ "MorePassesRequired", + "LeaseRenewalRequired", "ZKAPAuthorizerStorageServer", "ZKAPAuthorizerStorageClient", "ZKAPAuthorizer", @@ -21,6 +22,7 @@ __all__ = [ from ._storage_server import ( MorePassesRequired, + LeaseRenewalRequired, ZKAPAuthorizerStorageServer, ) from ._storage_client import ( diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index ad8ba0eb5fcaa615cfb39b7b7056c963295d7048..f4ecbbbd1ba7fea0df793fcdf20836f2ac07628b 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -38,6 +38,7 @@ from hypothesis import ( from hypothesis.strategies import ( integers, lists, + tuples, ) from privacypass import ( RandomToken, @@ -52,6 +53,10 @@ from .privacypass import ( ) from .strategies import ( zkaps, + storage_indexes, + write_enabler_secrets, + lease_renew_secrets, + lease_cancel_secrets, ) from .fixtures import ( AnonymousStorageServer, @@ -137,3 +142,45 @@ class PassValidationTests(TestCase): allocate_buckets, raises(MorePassesRequired), ) + + + @given( + storage_index=storage_indexes(), + secrets=tuples( + write_enabler_secrets(), + lease_renew_secrets(), + lease_cancel_secrets(), + ), + ) + def test_create_mutable_fails_without_passes(self, storage_index, secrets): + """ + If ``remote_slot_testv_and_readv_and_writev`` is invoked to perform + initial writes on shares without supplying passes, the operation fails + with ``LeaseRenewalRequired``. + """ + data = b"01234567" + offset = 0 + sharenum = 0 + mutable_write = lambda: self.storage_server.doRemoteCall( + "slot_testv_and_readv_and_writev", + (), + dict( + passes=[], + storage_index=storage_index, + secrets=secrets, + tw_vectors={ + sharenum: ([], [(offset, data)], None), + }, + r_vector=[], + ), + ) + + try: + result = mutable_write() + except MorePassesRequired: + pass + else: + self.fail("expected LeaseRenewalRequired, got {}".format(result)) + + # TODO + # a write that increases the storage cost of the share requires passes too