diff --git a/.circleci/config.yml b/.circleci/config.yml index 0957b80b613f58de506bc2edc50d5b010638c4a3..d477118ab06beb28edc6d95b03a6d2767342e67b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -163,7 +163,7 @@ jobs: # somewhat current as of the time of this comment. We can bump it to a # newer version when that makes sense. Meanwhile, the platform won't # shift around beneath us unexpectedly. - NIX_PATH: "nixpkgs=https://github.com/PrivateStorageio/nixpkgs/archive/730129887a84a8f84f3b78ffac7add72aeb551b6.tar.gz" + NIX_PATH: "nixpkgs=https://github.com/PrivateStorageio/nixpkgs/archive/c12c213c1c96bd1fea9f83f9e9e1fea28d0eaec6.tar.gz" steps: - run: diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py index a9cced29fa16c610492e53c1f9622ed9eadd6610..656679768f898ae67a4a63ea2bac01f88bf9ec80 100644 --- a/src/_zkapauthorizer/_storage_client.py +++ b/src/_zkapauthorizer/_storage_client.py @@ -66,7 +66,6 @@ from .storage_common import ( required_passes, allocate_buckets_message, add_lease_message, - renew_lease_message, slot_testv_and_readv_and_writev_message, has_writes, get_required_new_passes_for_mutable_write, @@ -403,33 +402,6 @@ class ZKAPAuthorizerStorageClient(object): ) returnValue(result) - @inline_callbacks - @with_rref - def renew_lease( - self, - rref, - storage_index, - renew_secret, - ): - share_sizes = (yield rref.callRemote( - "share_sizes", - storage_index, - None, - )).values() - num_passes = required_passes(self._pass_value, share_sizes) - - result = yield call_with_passes( - lambda passes: rref.callRemote( - "renew_lease", - _encode_passes(passes), - storage_index, - renew_secret, - ), - num_passes, - partial(self._get_passes, renew_lease_message(storage_index).encode("utf-8")), - ) - returnValue(result) - @with_rref def stat_shares(self, rref, storage_indexes): return rref.callRemote( diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index ffe74b8e024cd51e81d3d7ef25257e776ac42f15..a7a5616f35ee934868c5a3ae3b33c1953bbbb98a 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -103,7 +103,6 @@ from .storage_common import ( required_passes, allocate_buckets_message, add_lease_message, - renew_lease_message, slot_testv_and_readv_and_writev_message, has_writes, get_required_new_passes_for_mutable_write, @@ -302,24 +301,6 @@ class ZKAPAuthorizerStorageServer(Referenceable): ) return self._original.remote_add_lease(storage_index, *a, **kw) - def remote_renew_lease(self, passes, storage_index, *a, **kw): - """ - Pass-through after a pass check to ensure clients can only extend the - duration of share storage if they present valid passes. - """ - valid_passes = _ValidationResult.validate_passes( - renew_lease_message(storage_index), - passes, - self._signing_key, - ) - check_pass_quantity_for_lease( - self._pass_value, - storage_index, - valid_passes, - self._original, - ) - return self._original.remote_renew_lease(storage_index, *a, **kw) - def remote_advise_corrupt_share(self, *a, **kw): """ Pass-through without a pass check to let clients inform us of possible diff --git a/src/_zkapauthorizer/foolscap.py b/src/_zkapauthorizer/foolscap.py index 29ed94e7374c22b4a7534a9cc843a2db9970be79..b8f12425e72f3c3e80df6350f59f16a6c93bb635 100644 --- a/src/_zkapauthorizer/foolscap.py +++ b/src/_zkapauthorizer/foolscap.py @@ -141,7 +141,6 @@ def add_arguments(schema, kwargs): return modified_schema - class RIPrivacyPassAuthorizedStorageServer(RemoteInterface): """ An object which can store and retrieve shares, subject to pass-based @@ -162,8 +161,6 @@ class RIPrivacyPassAuthorizedStorageServer(RemoteInterface): add_lease = add_passes(RIStorageServer["add_lease"]) - renew_lease = add_passes(RIStorageServer["renew_lease"]) - get_buckets = RIStorageServer["get_buckets"] def share_sizes( diff --git a/src/_zkapauthorizer/storage_common.py b/src/_zkapauthorizer/storage_common.py index 487c164aa7f5cce69a2e9a66d5cbbfaa475ecd4e..829d5a17e3facf46a12fa2685e788d012e086622 100644 --- a/src/_zkapauthorizer/storage_common.py +++ b/src/_zkapauthorizer/storage_common.py @@ -70,7 +70,6 @@ def _message_maker(label): # construction for different Tahoe-LAFS storage operations. allocate_buckets_message = _message_maker(u"allocate_buckets") add_lease_message = _message_maker(u"add_lease") -renew_lease_message = _message_maker(u"renew_lease") slot_testv_and_readv_and_writev_message = _message_maker(u"slot_testv_and_readv_and_writev") # The number of bytes we're willing to store for a lease period for each pass diff --git a/src/_zkapauthorizer/tests/fixtures.py b/src/_zkapauthorizer/tests/fixtures.py index 1b226500bc327358f4dfa3ee28b981b209ca9d88..392520c90b82778aeb5879a972c92517dee52ba2 100644 --- a/src/_zkapauthorizer/tests/fixtures.py +++ b/src/_zkapauthorizer/tests/fixtures.py @@ -37,6 +37,7 @@ from twisted.python.filepath import ( from twisted.internet.task import ( Clock, ) +from allmydata import __version__ as allmydata_version from allmydata.storage.server import ( StorageServer, ) @@ -51,6 +52,7 @@ from ..controller import ( PaymentController, ) +@attr.s class AnonymousStorageServer(Fixture): """ Supply an instance of allmydata.storage.server.StorageServer which @@ -61,12 +63,28 @@ class AnonymousStorageServer(Fixture): :ivar allmydata.storage.server.StorageServer storage_server: The storage server. + + :ivar twisted.internet.task.Clock clock: The ``IReactorTime`` provider to + supply to ``StorageServer`` for its time-checking needs. """ + clock = attr.ib() + def _setUp(self): self.tempdir = FilePath(self.useFixture(TempDir()).join(b"storage")) + if allmydata_version >= "1.16.": + # This version of Tahoe adds a new StorageServer argument for + # controlling time. + timeargs = {"get_current_time": self.clock.seconds} + else: + # Older versions just use time.time() and there's not much we can + # do _here_. Code somewhere else will have to monkey-patch that + # to control things. + timeargs = {} + self.storage_server = StorageServer( self.tempdir.asBytesMode().path, b"x" * 20, + **timeargs ) diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index 70ec582cb96ff560d5c9cdfa612689d3fd540d5a..d20eb91613d294470a76f8b71933615d7d732162 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -812,20 +812,31 @@ def posix_safe_datetimes(): ) -def clocks(now=posix_safe_datetimes()): +def posix_timestamps(): + """ + Build floats in a range that can represent time without losing microsecond + precision. + """ + return posix_safe_datetimes().map( + lambda when: (when - _POSIX_EPOCH).total_seconds(), + ) + + +def clocks(now=posix_timestamps()): """ Build ``twisted.internet.task.Clock`` instances set to a time built by ``now``. + + :param now: A strategy that builds POSIX timestamps (ie, ints or floats in + the range of time_t). """ def clock_at_time(when): c = Clock() - c.advance((when - _POSIX_EPOCH).total_seconds()) + c.advance(when) return c return now.map(clock_at_time) - - @implementer(IFilesystemNode) @attr.s(frozen=True) class _LeafNode(object): diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py index d79b0fc84599902aa5737aaba45efc9377ba07b6..afe963c24d3f5e932fc1871d5122fe3dbe2eb91c 100644 --- a/src/_zkapauthorizer/tests/test_storage_protocol.py +++ b/src/_zkapauthorizer/tests/test_storage_protocol.py @@ -63,6 +63,9 @@ from twisted.python.runtime import ( from twisted.python.filepath import ( FilePath, ) +from twisted.internet.task import ( + Clock, +) from foolscap.referenceable import ( LocalReferenceable, @@ -90,7 +93,7 @@ from .strategies import ( sharenum_sets, sizes, slot_test_and_write_vectors_for_shares, - clocks, + posix_timestamps, # Not really a strategy... bytes_for_share, ) @@ -130,6 +133,7 @@ from ..foolscap import ( ShareStat, ) + class RequiredPassesTests(TestCase): """ Tests for ``required_passes``. @@ -192,23 +196,38 @@ class ShareTests(TestCase): def setUp(self): super(ShareTests, self).setUp() self.canary = LocalReferenceable(None) - self.anonymous_storage_server = self.useFixture(AnonymousStorageServer()).storage_server self.signing_key = random_signing_key() - self.pass_factory = pass_factory(get_passes=privacypass_passes(self.signing_key)) + self.clock = Clock() + self.anonymous_storage_server = self.useFixture( + AnonymousStorageServer(self.clock), + ).storage_server + self.server = ZKAPAuthorizerStorageServer( self.anonymous_storage_server, self.pass_value, self.signing_key, + clock=self.clock, ) self.local_remote_server = LocalRemote(self.server) self.client = ZKAPAuthorizerStorageClient( self.pass_value, get_rref=lambda: self.local_remote_server, get_passes=self.pass_factory.get, + clock=self.clock, ) + def setup_example(self): + """ + Initialize any necessary state prior to each Hypothesis iteration of a + test method. + """ + # Reset the mutable, shared clock to the epoch to simplify related + # code in the tests and ensure consistent starting state for each + # iteration. + self.clock.advance(-self.clock.seconds()) + def test_get_version(self): """ Version information about the storage server can be retrieved using @@ -522,60 +541,23 @@ class ShareTests(TestCase): leases = list(self.anonymous_storage_server.get_leases(storage_index)) self.assertThat(leases, HasLength(2)) - @given( - storage_index=storage_indexes(), - renew_secret=lease_renew_secrets(), - cancel_secret=lease_cancel_secrets(), - sharenums=sharenum_sets(), - size=sizes(), - ) - def test_renew_lease(self, storage_index, renew_secret, cancel_secret, sharenums, size): - """ - A lease on an immutable share can be updated to expire at a later time. - """ + def _stat_shares_immutable_test(self, storage_index, sharenum, size, when, leases, write_shares): # Hypothesis causes our storage server to be used many times. Clean # up between iterations. cleanup_storage_server(self.anonymous_storage_server) - # Take control of time (in this hacky, fragile way) so we can verify - # the expiration time gets bumped by the renewal. - now = 1000000000.5 - self.useFixture(MonkeyPatch("time.time", lambda: now)) + # Lease cancellation is unimplemented in Tahoe so this doesn't matter. + cancel_secret = b"" - # Create a share we can toy with. - write_toy_shares( - self.anonymous_storage_server, - storage_index, - renew_secret, - cancel_secret, - sharenums, - size, - canary=self.canary, - ) - - now += 100000 - self.assertThat( - self.client.renew_lease( - storage_index, - renew_secret, - ), - succeeded(Always()), - ) - - [lease] = self.anonymous_storage_server.get_leases(storage_index) - self.assertThat( - lease.get_expiration_time(), - Equals(int(now + self.server.LEASE_PERIOD.total_seconds())), - ) + self.clock.advance(when) - def _stat_shares_immutable_test(self, storage_index, sharenum, size, clock, leases, write_shares): - # Hypothesis causes our storage server to be used many times. Clean - # up between iterations. - cleanup_storage_server(self.anonymous_storage_server) - - # anonymous_storage_server uses time.time(), unfortunately. And - # useFixture does not interact very well with Hypothesis. - patch = MonkeyPatch("time.time", clock.seconds) + # anonymous_storage_server uses time.time() before Tahoe-LAFS 1.16, + # unfortunately. For Tahoe-LAFS 1.16, AnonymousStorageServer will + # glue self.clock in for us. For older versions we still need this + # monkey-patching. + # + # And useFixture does not interact very well with Hypothesis. + patch = MonkeyPatch("time.time", self.clock.seconds) try: patch.setUp() # Create a share we can toy with. @@ -592,7 +574,7 @@ class ShareTests(TestCase): self.anonymous_storage_server.remote_add_lease( storage_index, renew_secret, - b"", + cancel_secret, ) finally: patch.cleanUp() @@ -600,7 +582,7 @@ class ShareTests(TestCase): expected = [{ sharenum: ShareStat( size=size, - lease_expiration=int(clock.seconds() + LEASE_INTERVAL), + lease_expiration=int(self.clock.seconds() + LEASE_INTERVAL), ), }] self.assertThat( @@ -614,10 +596,10 @@ class ShareTests(TestCase): cancel_secret=lease_cancel_secrets(), sharenum=sharenums(), size=sizes(), - clock=clocks(), + when=posix_timestamps(), leases=lists(lease_renew_secrets(), unique=True), ) - def test_stat_shares_immutable(self, storage_index, renew_secret, cancel_secret, sharenum, size, clock, leases): + def test_stat_shares_immutable(self, storage_index, renew_secret, cancel_secret, sharenum, size, when, leases): """ Size and lease information about immutable shares can be retrieved from a storage server. @@ -626,7 +608,7 @@ class ShareTests(TestCase): storage_index, sharenum, size, - clock, + when, leases, lambda storage_server, storage_index, sharenums, size, canary: write_toy_shares( storage_server, @@ -643,11 +625,11 @@ class ShareTests(TestCase): storage_index=storage_indexes(), sharenum=sharenums(), size=sizes(), - clock=clocks(), + when=posix_timestamps(), leases=lists(lease_renew_secrets(), unique=True, min_size=1), version=share_versions(), ) - def test_stat_shares_immutable_wrong_version(self, storage_index, sharenum, size, clock, leases, version): + def test_stat_shares_immutable_wrong_version(self, storage_index, sharenum, size, when, leases, version): """ If a share file with an unexpected version is found, ``stat_shares`` declines to offer a result (by raising ``ValueError``). @@ -670,7 +652,7 @@ class ShareTests(TestCase): version=version, size=size, leases=leases, - now=clock.seconds(), + now=when, ) self.assertThat( @@ -687,12 +669,12 @@ class ShareTests(TestCase): storage_index=storage_indexes(), sharenum=sharenums(), size=sizes(), - clock=clocks(), + when=posix_timestamps(), version=share_versions(), # Encode our knowledge of the share header format and size right here... position=integers(min_value=0, max_value=11), ) - def test_stat_shares_truncated_file(self, storage_index, sharenum, size, clock, version, position): + def test_stat_shares_truncated_file(self, storage_index, sharenum, size, when, version, position): """ If a share file is truncated in the middle of the header, ``stat_shares`` declines to offer a result (by raising @@ -716,7 +698,7 @@ class ShareTests(TestCase): # We know leases are at the end, where they'll get chopped off, so # we don't bother to write any. leases=[], - now=clock.seconds(), + now=when, ) with sharepath.open("wb") as fobj: fobj.truncate(position) @@ -737,10 +719,10 @@ class ShareTests(TestCase): storage_index=storage_indexes(), sharenum=sharenums(), size=sizes(min_value=2 ** 18, max_value=2 ** 40), - clock=clocks(), + when=posix_timestamps(), leases=lists(lease_renew_secrets(), unique=True, min_size=1), ) - def test_stat_shares_immutable_large(self, storage_index, sharenum, size, clock, leases): + def test_stat_shares_immutable_large(self, storage_index, sharenum, size, when, leases): """ Size and lease information about very large immutable shares can be retrieved from a storage server. @@ -763,14 +745,14 @@ class ShareTests(TestCase): version=1, size=size, leases=leases, - now=clock.seconds(), + now=when, ) return self._stat_shares_immutable_test( storage_index, sharenum, size, - clock, + when, leases, write_shares, ) @@ -784,9 +766,9 @@ class ShareTests(TestCase): lease_cancel_secrets(), ), test_and_write_vectors_for_shares=slot_test_and_write_vectors_for_shares(), - clock=clocks(), + when=posix_timestamps(), ) - def test_stat_shares_mutable(self, storage_index, secrets, test_and_write_vectors_for_shares, clock): + def test_stat_shares_mutable(self, storage_index, secrets, test_and_write_vectors_for_shares, when): """ Size and lease information about mutable shares can be retrieved from a storage server. @@ -795,9 +777,9 @@ class ShareTests(TestCase): # up between iterations. cleanup_storage_server(self.anonymous_storage_server) - # anonymous_storage_server uses time.time(), unfortunately. And - # useFixture does not interact very well with Hypothesis. - patch = MonkeyPatch("time.time", clock.seconds) + self.clock.advance(when) + + patch = MonkeyPatch("time.time", self.clock.seconds) try: patch.setUp() # Create a share we can toy with. @@ -827,7 +809,7 @@ class ShareTests(TestCase): vectors.write_vector, vectors.new_length, ), - lease_expiration=int(clock.seconds() + LEASE_INTERVAL), + lease_expiration=int(self.clock.seconds() + LEASE_INTERVAL), ) for (sharenum, vectors) in test_and_write_vectors_for_shares.items() @@ -996,7 +978,7 @@ class ShareTests(TestCase): storage_index=storage_indexes(), sharenum=sharenums(), size=sizes(), - clock=clocks(), + when=posix_timestamps(), write_enabler=write_enabler_secrets(), renew_secret=lease_renew_secrets(), cancel_secret=lease_cancel_secrets(), @@ -1005,7 +987,7 @@ class ShareTests(TestCase): def test_mutable_rewrite_renews_expired_lease( self, storage_index, - clock, + when, sharenum, size, write_enabler, @@ -1021,9 +1003,7 @@ class ShareTests(TestCase): # up between iterations. cleanup_storage_server(self.anonymous_storage_server) - # Make the client and server use our clock. - self.server._clock = clock - self.client._clock = clock + self.clock.advance(when) secrets = (write_enabler, renew_secret, cancel_secret) @@ -1039,9 +1019,7 @@ class ShareTests(TestCase): r_vector=[], ) - # anonymous_storage_server uses time.time() to assign leases, - # unfortunately. - patch = MonkeyPatch("time.time", clock.seconds) + patch = MonkeyPatch("time.time", self.clock.seconds) try: patch.setUp() @@ -1050,7 +1028,7 @@ class ShareTests(TestCase): # Advance time by more than a lease period so the lease is no # longer valid. - clock.advance(self.server.LEASE_PERIOD.total_seconds() + 1) + self.clock.advance(self.server.LEASE_PERIOD.total_seconds() + 1) self.assertThat(write(), is_successful_write()) finally: @@ -1066,7 +1044,7 @@ class ShareTests(TestCase): test_and_write_vectors_for_shares[num].write_vector, test_and_write_vectors_for_shares[num].new_length, ), - lease_expiration=int(clock.seconds() + self.server.LEASE_PERIOD.total_seconds()), + lease_expiration=int(self.clock.seconds() + self.server.LEASE_PERIOD.total_seconds()), ) for num in test_and_write_vectors_for_shares diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index a3d1451ba38bc28576948f9eda7cd39a99af9f68..1adab62a8b9e6e6605424f2a2ee78dc75f2f87bd 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -97,7 +97,6 @@ from ..storage_common import ( required_passes, allocate_buckets_message, add_lease_message, - renew_lease_message, slot_testv_and_readv_and_writev_message, get_implied_data_length, get_required_new_passes_for_mutable_write, @@ -204,7 +203,9 @@ class PassValidationTests(TestCase): # the same time so we can do lease expiration calculations more # easily. self.clock.advance(time()) - self.anonymous_storage_server = self.useFixture(AnonymousStorageServer()).storage_server + self.anonymous_storage_server = self.useFixture( + AnonymousStorageServer(self.clock), + ).storage_server self.signing_key = random_signing_key() self.storage_server = ZKAPAuthorizerStorageServer( self.anonymous_storage_server, @@ -539,40 +540,6 @@ class PassValidationTests(TestCase): add_lease_message, ) - @given( - storage_index=storage_indexes(), - secrets=tuples( - lease_renew_secrets(), - lease_cancel_secrets(), - ), - sharenums=sharenum_sets(), - allocated_size=sizes(), - ) - def test_renew_lease_fails_without_passes(self, storage_index, secrets, sharenums, allocated_size): - """ - If ``remote_renew_lease`` is invoked without supplying enough passes to - cover the storage for all shares on the given storage index, the - operation fails with ``MorePassesRequired``. - """ - renew_secret, cancel_secret = secrets - def renew_lease(storage_server, passes): - return storage_server.doRemoteCall( - "renew_lease", ( - passes, - storage_index, - renew_secret, - ), - {}, - ) - return self._test_lease_operation_fails_without_passes( - storage_index, - secrets, - sharenums, - allocated_size, - renew_lease, - renew_lease_message, - ) - @given( slot=storage_indexes(), secrets=tuples(