diff --git a/default.nix b/default.nix index 27401736371276c58a0df8b8112c496998aaa4b0..3a47363302f17d5aa412692d43ce90609fb6783d 100644 --- a/default.nix +++ b/default.nix @@ -1,2 +1,2 @@ -{ pkgs ? import ./nixpkgs.nix { }, hypothesisProfile ? null, collectCoverage ? false }: -pkgs.python27Packages.zkapauthorizer.override { inherit hypothesisProfile collectCoverage; } +{ pkgs ? import ./nixpkgs.nix { }, hypothesisProfile ? null, collectCoverage ? false, testSuite ? null }: +pkgs.python27Packages.zkapauthorizer.override { inherit hypothesisProfile collectCoverage testSuite; } diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py index b7a94f3e78e7aa0cb42e1085ec7cf074a783c24a..3973164db6257f2d5d8b4bc1f4bc94c0b609db97 100644 --- a/src/_zkapauthorizer/_storage_client.py +++ b/src/_zkapauthorizer/_storage_client.py @@ -127,19 +127,33 @@ class ZKAPAuthorizerStorageClient(object): storage_index, ) + @inlineCallbacks def add_lease( self, storage_index, renew_secret, cancel_secret, ): - return self._rref.callRemote( - "add_lease", - self._get_encoded_passes(add_lease_message(storage_index), 1), + share_sizes = (yield self._rref.callRemote( + "share_sizes", storage_index, - renew_secret, - cancel_secret, - ) + None, + )).values() + num_passes = required_passes(BYTES_PER_PASS, share_sizes) + # print("Adding lease to {!r} with sizes {} with {} passes".format( + # storage_index, + # share_sizes, + # num_passes, + # )) + returnValue(( + yield self._rref.callRemote( + "add_lease", + self._get_encoded_passes(add_lease_message(storage_index), num_passes), + storage_index, + renew_secret, + cancel_secret, + ) + )) def renew_lease( self, @@ -190,7 +204,7 @@ class ZKAPAuthorizerStorageClient(object): # of the current size of all of the specified shares (keys of # tw_vectors). current_sizes = yield self._rref.callRemote( - "slot_share_sizes", + "share_sizes", storage_index, set(tw_vectors), ) diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index 5390b995be0177730f56b146726e4cc97a51da76..f76443a1e285f5743cea9a1cf0e568e71ec3ad7f 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -24,6 +24,10 @@ from __future__ import ( absolute_import, ) +from struct import ( + unpack, +) + from errno import ( ENOENT, ) @@ -37,9 +41,10 @@ from os.path import ( ) from os import ( listdir, - stat, ) - +from datetime import ( + timedelta, +) import attr from attr.validators import ( provides, @@ -136,6 +141,15 @@ class ZKAPAuthorizerStorageServer(Referenceable): A class which wraps an ``RIStorageServer`` to insert pass validity checks before allowing certain functionality. """ + + # This is the amount of time an added or renewed lease will last. We + # duplicate the value used by the underlying anonymous-access storage + # server which does not expose it via a Python API or allow it to be + # configured or overridden. It would be great if the anonymous-access + # storage server eventually made lease time a parameter so we could just + # control it ourselves. + LEASE_PERIOD = timedelta(days=31) + _original = attr.ib(validator=provides(RIStorageServer)) _signing_key = attr.ib(validator=instance_of(SigningKey)) _clock = attr.ib( @@ -176,12 +190,14 @@ class ZKAPAuthorizerStorageServer(Referenceable): :return list[bytes]: The passes which are found to be valid. """ - return list( + result = list( pass_ for pass_ in passes if not self._is_invalid_pass(message, pass_) ) + # print("{}: {} passes, {} valid".format(message, len(passes), len(result))) + return result def remote_get_version(self): """ @@ -222,7 +238,17 @@ class ZKAPAuthorizerStorageServer(Referenceable): Pass-through after a pass check to ensure clients can only extend the duration of share storage if they present valid passes. """ - self._validate_passes(add_lease_message(storage_index), passes) + # print("server add_lease({}, {!r})".format(len(passes), storage_index)) + valid_passes = self._validate_passes(add_lease_message(storage_index), passes) + allocated_sizes = dict( + get_share_sizes( + self._original, storage_index, + list(get_all_share_numbers(self._original, storage_index)), + ), + ).values() + # print("allocated_sizes: {}".format(allocated_sizes)) + check_pass_quantity(len(valid_passes), allocated_sizes) + # print("Checked out") return self._original.remote_add_lease(storage_index, *a, **kw) def remote_renew_lease(self, passes, storage_index, *a, **kw): @@ -240,9 +266,9 @@ class ZKAPAuthorizerStorageServer(Referenceable): """ return self._original.remote_advise_corrupt_share(*a, **kw) - def remote_slot_share_sizes(self, storage_index, sharenums): + def remote_share_sizes(self, storage_index_or_slot, sharenums): return dict( - get_slot_share_sizes(self._original, storage_index, sharenums) + get_share_sizes(self._original, storage_index_or_slot, sharenums) ) def remote_slot_testv_and_readv_and_writev( @@ -276,11 +302,12 @@ class ZKAPAuthorizerStorageServer(Referenceable): ) if has_active_lease(self._original, storage_index, self._clock.seconds()): # Some of the storage is paid for already. - current_sizes = dict(get_slot_share_sizes( + current_sizes = dict(get_share_sizes( self._original, storage_index, tw_vectors.keys(), )) + # print("has writes, has active lease, current sizes: {}".format(current_sizes)) else: # None of it is. current_sizes = {} @@ -335,6 +362,27 @@ def has_active_lease(storage_server, storage_index, now): ) +def check_pass_quantity(valid_count, share_sizes): + """ + Check that the given number of passes is sufficient to cover leases for + one period for shares of the given sizes. + + :param int valid_count: The number of passes. + :param list[int] share_sizes: The sizes of the shares for which the lease + is being created. + + :raise MorePassesRequired: If the given number of passes is too few for + the given share sizes. + + :return: ``None`` if the given number of passes is sufficient. + """ + required_pass_count = required_passes(BYTES_PER_PASS, share_sizes) + if valid_count < required_pass_count: + raise MorePassesRequired( + valid_count, + required_pass_count, + ) + def check_pass_quantity_for_write(valid_count, sharenums, allocated_size): """ Determine if the given number of valid passes is sufficient for an @@ -349,34 +397,21 @@ def check_pass_quantity_for_write(valid_count, sharenums, allocated_size): :return: ``None`` if the number of valid passes given is sufficient. """ - required_pass_count = required_passes(BYTES_PER_PASS, [allocated_size] * len(sharenums)) - # print("valid_count = {}".format(valid_count)) - # print("sharenums = {}".format(len(sharenums))) - # print("allocated size = {}".format(allocated_size)) - # print("required_pass_count = {}".format(required_pass_count)) - if valid_count < required_pass_count: - raise MorePassesRequired( - valid_count, - required_pass_count, - ) + check_pass_quantity(valid_count, [allocated_size] * len(sharenums)) -def get_slot_share_sizes(storage_server, storage_index, sharenums): +def get_all_share_paths(storage_server, storage_index): """ - Retrieve the on-disk storage committed to the given shares in the given - storage index. + Get the paths of all shares in the given storage index (or slot). :param allmydata.storage.server.StorageServer storage_server: The storage - server which owns the on-disk storage. - - :param bytes storage_index: The storage index to inspect. + server which owns the storage index. - :param list[int] sharenums: The share numbers to consider. + :param bytes storage_index: The storage index (or slot) in which to look + up shares. - :return generator[(int, int)]: Pairs of share number, bytes on disk of the - given shares. Note this is naive with respect to filesystem features - like compression or sparse files. It is just the size reported by the - filesystem. + :return: A generator of tuples of (int, bytes) giving a share number and + the path to storage for that share number. """ bucket = join(storage_server.sharedir, storage_index_to_dir(storage_index)) try: @@ -392,28 +427,85 @@ def get_slot_share_sizes(storage_server, storage_index, sharenums): except ValueError: pass else: - if sharenum in sharenums: - try: - metadata = stat(join(bucket, candidate)) - except Exception as e: - print(e) + yield sharenum, join(bucket, candidate) + + +def get_all_share_numbers(storage_server, storage_index): + """ + Get all share numbers in the given storage index (or slot). + + :param allmydata.storage.server.StorageServer storage_server: The storage + server which owns the storage index. + + :param bytes storage_index: The storage index (or slot) in which to look + up share numbers. + + :return: A generator of int giving share numbers. + """ + for sharenum, sharepath in get_all_share_paths(storage_server, storage_index): + yield sharenum + + +def get_share_sizes(storage_server, storage_index_or_slot, sharenums): + """ + Get the sizes of the given share numbers for the given storage index *or* + slot. + + :param allmydata.storage.server.StorageServer storage_server: The storage + server which owns the storage index. + + :param bytes storage_index_or_slot: The storage index (or slot) in which + to look up share numbers. + + :param sharenums: A container of share numbers to use to filter the + results. Only information about share numbers in this container is + included in the result. Or, ``None`` to get sizes for all shares + which exist. + + :return: A generator of tuples of (int, int) where the first element is a + share number and the second element is the data size for that share + number. + """ + get_size = None + for sharenum, sharepath in get_all_share_paths(storage_server, storage_index_or_slot): + if get_size is None: + # Figure out if it is a storage index or a slot. + with open(sharepath) as share_file: + magic = share_file.read(32) + if magic == "Tahoe mutable container v1\n" + "\x75\x09\x44\x03\x8e": + get_size = get_slot_share_size else: - # Compared to calculating how much *user* data we're - # storing, the on-disk file is larger by at *least* - # SLOT_HEADER_SIZE. There is also a variable sized - # trailer which is harder to compute but which is at least - # LEASE_TRAILER_SIZE. Fortunately it's often exactly - # LEASE_TRAILER_SIZE so I'm just going to ignore it for - # now. - # - # By measuring that the slots are larger than the data the - # user is storing we'll overestimate how many passes are - # required right around the boundary between two costs. - # Oops. - yield ( - sharenum, - metadata.st_size - SLOT_HEADER_SIZE - LEASE_TRAILER_SIZE, - ) + get_size = get_storage_index_share_size + if sharenums is None or sharenum in sharenums: + yield sharenum, get_size(sharepath) + + +def get_storage_index_share_size(sharepath): + """ + Get the size of a share belonging to a storage index (an immutable share). + + :param bytes sharepath: The path to the share file. + + :return int: The data size of the share in bytes. + """ + with open(sharepath) as share_file: + share_data_length_bytes = share_file.read(8)[4:] + (share_data_length,) = unpack('>L', share_data_length_bytes) + return share_data_length + + +def get_slot_share_size(sharepath): + """ + Get the size of a share belonging to a slot (a mutable share). + + :param bytes sharepath: The path to the share file. + + :return int: The data size of the share in bytes. + """ + with open(sharepath) as share_file: + share_data_length_bytes = share_file.read(92)[-8:] + (share_data_length,) = unpack('>Q', share_data_length_bytes) + return share_data_length # I don't understand why this is required. diff --git a/src/_zkapauthorizer/foolscap.py b/src/_zkapauthorizer/foolscap.py index eb70f11711ec7915c8dbc6f3c0a8c7ba47c797d1..4e77b5a5075ec4b215d221cbbb8c9e941401cf29 100644 --- a/src/_zkapauthorizer/foolscap.py +++ b/src/_zkapauthorizer/foolscap.py @@ -6,8 +6,9 @@ from foolscap.constraint import ( ByteStringConstraint, ) from foolscap.api import ( - DictOf, + ChoiceOf, SetOf, + DictOf, ListOf, ) from foolscap.remoteinterface import ( @@ -122,16 +123,13 @@ class RIPrivacyPassAuthorizedStorageServer(RemoteInterface): get_buckets = RIStorageServer["get_buckets"] - def slot_share_sizes( - storage_index=StorageIndex, - sharenums=SetOf(int, maxLength=MAX_BUCKETS), + def share_sizes( + storage_index_or_slot=StorageIndex, + sharenums=ChoiceOf(None, SetOf(int, maxLength=MAX_BUCKETS)), ): """ - Get the size of the given shares in the given storage index. If a share - has no stored state, its size is reported as 0. - - The reported size may be larger than the actual share size if there - are more than four leases on the share. + Get the size of the given shares in the given storage index or slot. If a + share has no stored state, its size is reported as 0. """ return DictOf(int, Offset) diff --git a/src/_zkapauthorizer/storage_common.py b/src/_zkapauthorizer/storage_common.py index d1e7240d3c3f97f86d577588017512664e189b38..d5a24ff7ec0deb9241f8ce4b99dc8ed185fcff46 100644 --- a/src/_zkapauthorizer/storage_common.py +++ b/src/_zkapauthorizer/storage_common.py @@ -59,11 +59,13 @@ def required_passes(bytes_per_pass, share_sizes): :return int: The number of passes required to cover the storage cost. """ - return int( + result = int( ceil( sum(share_sizes, 0) / bytes_per_pass, ), ) + # print("required_passes({}, {}) == {}".format(bytes_per_pass, share_sizes, result)) + return result def has_writes(tw_vectors): @@ -112,21 +114,29 @@ def get_allocated_size(tw_vectors): ) -def get_implied_data_length(data_vector): +def get_implied_data_length(data_vector, new_length): """ :param data_vector: See ``allmydata.interfaces.DataVector``. + :param new_length: See + ``allmydata.interfaces.RIStorageServer.slot_testv_and_readv_and_writev``. + :return int: The amount of data, in bytes, implied by a data vector and a size. """ - return max( + data_based_size = max( offset + len(data) for (offset, data) in data_vector ) if data_vector else 0 + if new_length is None: + return data_based_size + # new_length is only allowed to truncate, not expand. + return min(new_length, data_based_size) def get_required_new_passes_for_mutable_write(current_sizes, tw_vectors): + # print("get_required_new_passes_for_mutable_write({}, {})".format(current_sizes, summarize(tw_vectors))) current_passes = required_passes( BYTES_PER_PASS, current_sizes.values(), @@ -134,7 +144,7 @@ def get_required_new_passes_for_mutable_write(current_sizes, tw_vectors): new_sizes = current_sizes.copy() size_updates = { - sharenum: get_implied_data_length(data_vector) + sharenum: get_implied_data_length(data_vector, new_length) for (sharenum, (_, data_vector, new_length)) in tw_vectors.items() } @@ -150,7 +160,22 @@ def get_required_new_passes_for_mutable_write(current_sizes, tw_vectors): required_new_passes = new_passes - current_passes # print("Current sizes: {}".format(current_sizes)) - # print("Current passeS: {}".format(current_passes)) + # print("Current passes: {}".format(current_passes)) # print("New sizes: {}".format(new_sizes)) # print("New passes: {}".format(new_passes)) return required_new_passes + +def summarize(tw_vectors): + return { + sharenum: ( + test_vector, + list( + (offset, len(data)) + for (offset, data) + in data_vectors + ), + new_length, + ) + for (sharenum, (test_vector, data_vectors, new_length)) + in tw_vectors.items() + } diff --git a/src/_zkapauthorizer/tests/storage_common.py b/src/_zkapauthorizer/tests/storage_common.py index 6ae3a4c2c67fa2077e537ec3f33d06c96a94b4d1..4baf4de7b89ff6ab3c0ea7145b5d0fa347f3a8e9 100644 --- a/src/_zkapauthorizer/tests/storage_common.py +++ b/src/_zkapauthorizer/tests/storage_common.py @@ -20,6 +20,11 @@ from twisted.python.filepath import ( FilePath, ) +from .strategies import ( + # Not really a strategy... + bytes_for_share, +) + def cleanup_storage_server(storage_server): """ Delete all of the shares held by the given storage server. @@ -35,3 +40,36 @@ def cleanup_storage_server(storage_server): for p in start.walk(): if p is not start: p.remove() + + +def write_toy_shares( + storage_server, + storage_index, + renew_secret, + cancel_secret, + sharenums, + size, + canary, +): + """ + Write some immutable shares to the given storage server. + + :param allmydata.storage.server.StorageServer storage_server: + :param bytes storage_index: + :param bytes renew_secret: + :param bytes cancel_secret: + :param set[int] sharenums: + :param int size: + :param IRemoteReference canary: + """ + _, allocated = storage_server.remote_allocate_buckets( + storage_index, + renew_secret, + cancel_secret, + sharenums, + size, + canary=canary, + ) + for (sharenum, writer) in allocated.items(): + writer.remote_write(0, bytes_for_share(sharenum, size)) + writer.remote_close() diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index b9b781271f8d78d800e5a75537734baa7d858fa9..dc9e0fb63df2a7872291f3fcdbf527662ae81617 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -42,10 +42,7 @@ from twisted.web.test.requesthelper import ( ) from allmydata.interfaces import ( - StorageIndex, - LeaseRenewSecret, - LeaseCancelSecret, - WriteEnablerSecret, + HASH_SIZE, ) from allmydata.client import ( @@ -267,8 +264,11 @@ def storage_indexes(): Build Tahoe-LAFS storage indexes. """ return binary( - min_size=StorageIndex.minLength, - max_size=StorageIndex.maxLength, + # It is tempting to use StorageIndex.minLength and + # StorageIndex.maxLength but these are effectively garbage. See the + # implementation of ByteStringConstraint for details. + min_size=16, + max_size=16, ) @@ -277,8 +277,8 @@ def lease_renew_secrets(): Build Tahoe-LAFS lease renewal secrets. """ return binary( - min_size=LeaseRenewSecret.minLength, - max_size=LeaseRenewSecret.maxLength, + min_size=HASH_SIZE, + max_size=HASH_SIZE, ) @@ -287,8 +287,8 @@ def lease_cancel_secrets(): Build Tahoe-LAFS lease cancellation secrets. """ return binary( - min_size=LeaseCancelSecret.minLength, - max_size=LeaseCancelSecret.maxLength, + min_size=HASH_SIZE, + max_size=HASH_SIZE, ) @@ -297,8 +297,8 @@ def write_enabler_secrets(): Build Tahoe-LAFS write enabler secrets. """ return binary( - min_size=WriteEnablerSecret.minLength, - max_size=WriteEnablerSecret.maxLength, + min_size=HASH_SIZE, + max_size=HASH_SIZE, ) diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py index 9789d50a63e7c359de9424e42b70060bd35fd10d..c045a03f37d7418c1adb1e7862c0b27e61853939 100644 --- a/src/_zkapauthorizer/tests/test_storage_protocol.py +++ b/src/_zkapauthorizer/tests/test_storage_protocol.py @@ -91,6 +91,7 @@ from .fixtures import ( ) from .storage_common import ( cleanup_storage_server, + write_toy_shares, ) from ..api import ( ZKAPAuthorizerStorageServer, @@ -251,10 +252,10 @@ class ShareTests(TestCase): storage_index=storage_indexes(), renew_secrets=tuples(lease_renew_secrets(), lease_renew_secrets()), cancel_secret=lease_cancel_secrets(), - sharenum=sharenums(), + sharenums=sharenum_sets(), size=sizes(), ) - def test_add_lease(self, storage_index, renew_secrets, cancel_secret, sharenum, size): + def test_add_lease(self, storage_index, renew_secrets, cancel_secret, sharenums, size): """ A lease can be added to an existing immutable share. """ @@ -273,7 +274,7 @@ class ShareTests(TestCase): storage_index, add_lease_secret, cancel_secret, - {sharenum}, + sharenums, size, canary=self.canary, ) @@ -292,10 +293,10 @@ class ShareTests(TestCase): storage_index=storage_indexes(), renew_secret=lease_renew_secrets(), cancel_secret=lease_cancel_secrets(), - sharenum=sharenums(), + sharenums=sharenum_sets(), size=sizes(), ) - def test_renew_lease(self, storage_index, renew_secret, cancel_secret, sharenum, size): + 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. """ @@ -314,7 +315,7 @@ class ShareTests(TestCase): storage_index, renew_secret, cancel_secret, - {sharenum}, + sharenums, size, canary=self.canary, ) @@ -327,13 +328,10 @@ class ShareTests(TestCase): ), ) - # Based on Tahoe-LAFS' hard-coded renew time. - RENEW_INTERVAL = 60 * 60 * 24 * 31 - [lease] = self.anonymous_storage_server.get_leases(storage_index) self.assertThat( lease.get_expiration_time(), - Equals(int(now + RENEW_INTERVAL)), + Equals(int(now + self.server.LEASE_PERIOD.total_seconds())), ) @given( @@ -626,36 +624,3 @@ def write_vector_to_read_vector(write_vector): write vector. """ return (write_vector[0], len(write_vector[1])) - - -def write_toy_shares( - storage_server, - storage_index, - renew_secret, - cancel_secret, - sharenums, - size, - canary, -): - """ - Write some immutable shares to the given storage server. - - :param allmydata.storage.server.StorageServer storage_server: - :param bytes storage_index: - :param bytes renew_secret: - :param bytes cancel_secret: - :param set[int] sharenums: - :param int size: - :param IRemoteReference canary: - """ - _, allocated = storage_server.remote_allocate_buckets( - storage_index, - renew_secret, - cancel_secret, - sharenums, - size, - canary=canary, - ) - for (sharenum, writer) in allocated.items(): - writer.remote_write(0, bytes_for_share(sharenum, size)) - writer.remote_close() diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index cde2b7c7498d67ae96a7337aab28afe5332cae10..d74fa0a5bf05b59f78ffe9f207a8257adc084b14 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -21,6 +21,9 @@ from __future__ import ( division, ) +from time import ( + time, +) from random import ( shuffle, ) @@ -30,22 +33,29 @@ from testtools import ( from testtools.matchers import ( Equals, AfterPreprocessing, + MatchesStructure, raises, ) from hypothesis import ( given, note, - # reproduce_failure, ) from hypothesis.strategies import ( integers, lists, tuples, + one_of, + just, ) from privacypass import ( RandomToken, random_signing_key, ) + +from twisted.internet.task import ( + Clock, +) + from foolscap.referenceable import ( LocalReferenceable, ) @@ -55,6 +65,8 @@ from .privacypass import ( ) from .strategies import ( zkaps, + sizes, + sharenum_sets, storage_indexes, write_enabler_secrets, lease_renew_secrets, @@ -66,6 +78,7 @@ from .fixtures import ( ) from .storage_common import ( cleanup_storage_server, + write_toy_shares, ) from ..api import ( ZKAPAuthorizerStorageServer, @@ -73,25 +86,32 @@ from ..api import ( ) from ..storage_common import ( BYTES_PER_PASS, + required_passes, allocate_buckets_message, + add_lease_message, slot_testv_and_readv_and_writev_message, get_implied_data_length, get_required_new_passes_for_mutable_write, - + summarize, ) - class PassValidationTests(TestCase): """ Tests for pass validation performed by ``ZKAPAuthorizerStorageServer``. """ def setUp(self): super(PassValidationTests, self).setUp() + self.clock = Clock() + # anonymous_storage_server uses time.time() so get our Clock close to + # 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.signing_key = random_signing_key() self.storage_server = ZKAPAuthorizerStorageServer( self.anonymous_storage_server, self.signing_key, + self.clock, ) @given(integers(min_value=0, max_value=64), lists(zkaps(), max_size=64)) @@ -229,19 +249,7 @@ class PassValidationTests(TestCase): in test_and_write_vectors_for_shares.items() } - note("tw_vectors summarized: {}".format({ - sharenum: ( - test_vector, - list( - (offset, len(data)) - for (offset, data) - in data_vectors - ), - new_length, - ) - for (sharenum, (test_vector, data_vectors, new_length)) - in tw_vectors.items() - })) + note("tw_vectors summarized: {}".format(summarize(tw_vectors))) # print("test suite") required_pass_count = get_required_new_passes_for_mutable_write( @@ -277,18 +285,17 @@ class PassValidationTests(TestCase): "Server denied initial write.", ) - # Find the largest sharenum so we can make it even larger. - sharenum = max( - tw_vectors.keys(), - key=lambda k: get_implied_data_length(tw_vectors[k][1]), - ) + # Pick any share to make larger. + sharenum = next(iter(tw_vectors)) _, data_vector, new_length = tw_vectors[sharenum] - current_length = get_implied_data_length(data_vector) + current_length = get_implied_data_length(data_vector, new_length) new_tw_vectors = { sharenum: make_data_vector(current_length), } + note("new tw_vectors: {}".format(summarize(new_tw_vectors))) + do_extend = lambda: self.storage_server.doRemoteCall( "slot_testv_and_readv_and_writev", (), @@ -305,40 +312,15 @@ class PassValidationTests(TestCase): result = do_extend() except MorePassesRequired as e: self.assertThat( - e.required_count, - Equals(1), + e, + MatchesStructure( + valid_count=Equals(0), + required_count=Equals(1), + ), ) else: self.fail("expected MorePassesRequired, got {}".format(result)) - # @reproduce_failure('4.7.3', 'AXicY2CgMWAEQijr/39GRjCn+D+QxwQX72FgAABQ4QQI') - # @given( - # storage_index=storage_indexes(), - # secrets=tuples( - # write_enabler_secrets(), - # lease_renew_secrets(), - # lease_cancel_secrets(), - # ), - # test_and_write_vectors_for_shares=test_and_write_vectors_for_shares(), - # ) - # def test_extend_mutable_with_new_length_fails_without_passes(self, storage_index, secrets, test_and_write_vectors_for_shares): - # """ - # If ``remote_slot_testv_and_readv_and_writev`` is invoked to increase - # storage usage by supplying a ``new_length`` greater than the current - # share size and without supplying passes, the operation fails with - # ``MorePassesRequired``. - # """ - # return self._test_extend_mutable_fails_without_passes( - # storage_index, - # secrets, - # test_and_write_vectors_for_shares, - # lambda current_length: ( - # [], - # [], - # current_length + BYTES_PER_PASS, - # ), - # ) - @given( storage_index=storage_indexes(), secrets=tuples( @@ -364,3 +346,196 @@ class PassValidationTests(TestCase): None, ), ) + + @given( + storage_index=storage_indexes(), + secrets=tuples( + lease_renew_secrets(), + lease_cancel_secrets(), + ), + sharenums=sharenum_sets(), + allocated_size=sizes(), + ) + def test_add_lease_fails_without_passes(self, storage_index, secrets, sharenums, allocated_size): + """ + If ``remote_add_lease`` is invoked without supplying enough passes to + cover the storage for all shares on the given storage index, the + operation fails with ``MorePassesRequired``. + """ + # hypothesis causes our storage server to be used many times. Clean + # up between iterations. + cleanup_storage_server(self.anonymous_storage_server) + + renew_secret, cancel_secret = secrets + + required_count = required_passes(BYTES_PER_PASS, [allocated_size] * len(sharenums)) + # Create some shares at a slot which will require lease renewal. + write_toy_shares( + self.anonymous_storage_server, + storage_index, + renew_secret, + cancel_secret, + sharenums, + allocated_size, + LocalReferenceable(None), + ) + + # Advance time to a point where the lease is expired. This simplifies + # the logic behind how many passes will be required by the add_leases + # call (all of them). If there is prorating for partially expired + # leases then the calculation for a non-expired lease involves more + # work. + # + # Add some slop here because time.time() is used by some parts of the + # system. :/ + self.clock.advance(self.storage_server.LEASE_PERIOD.total_seconds() + 10.0) + + # Attempt to take out a new lease with one fewer pass than is + # required. + passes = make_passes( + self.signing_key, + add_lease_message(storage_index), + list(RandomToken.create() for i in range(required_count - 1)), + ) + # print("tests add_lease({}, {!r})".format(len(passes), storage_index)) + try: + result = self.storage_server.doRemoteCall( + "add_lease", ( + passes, + storage_index, + renew_secret, + cancel_secret, + ), + {}, + ) + except MorePassesRequired as e: + self.assertThat( + e, + MatchesStructure( + valid_count=Equals(len(passes)), + required_count=Equals(required_count), + ), + ) + else: + self.fail("Expected MorePassesRequired, got {}".format(result)) + + + @given( + storage_index=storage_indexes(), + secrets=tuples( + lease_renew_secrets(), + lease_cancel_secrets(), + ), + sharenums=sharenum_sets(), + allocated_size=sizes(), + ) + def test_immutable_share_sizes(self, storage_index, secrets, sharenums, allocated_size): + """ + ``share_sizes`` returns the size of the requested iimutable shares in the + requested storage index. + """ + # hypothesis causes our storage server to be used many times. Clean + # up between iterations. + cleanup_storage_server(self.anonymous_storage_server) + + renew_secret, cancel_secret = secrets + write_toy_shares( + self.anonymous_storage_server, + storage_index, + renew_secret, + cancel_secret, + sharenums, + allocated_size, + LocalReferenceable(None), + ) + + actual_sizes = self.storage_server.doRemoteCall( + "share_sizes", ( + storage_index, + sharenums, + ), + {}, + ) + self.assertThat( + actual_sizes, + Equals({ + sharenum: allocated_size + for sharenum + in sharenums + }), + ) + + @given( + slot=storage_indexes(), + secrets=tuples( + write_enabler_secrets(), + lease_renew_secrets(), + lease_cancel_secrets(), + ), + sharenums=one_of(just(None), sharenum_sets()), + test_and_write_vectors_for_shares=test_and_write_vectors_for_shares(), + ) + def test_mutable_share_sizes(self, slot, secrets, sharenums, test_and_write_vectors_for_shares): + """ + ``share_sizes`` returns the size of the requested mutable shares in the + requested slot. + """ + # hypothesis causes our storage server to be used many times. Clean + # up between iterations. + cleanup_storage_server(self.anonymous_storage_server) + + tw_vectors = { + k: v.for_call() + for (k, v) + in test_and_write_vectors_for_shares.items() + } + + # Create an initial share to toy with. + required_pass_count = get_required_new_passes_for_mutable_write( + dict.fromkeys(tw_vectors.keys(), 0), + tw_vectors, + ) + valid_passes = make_passes( + self.signing_key, + slot_testv_and_readv_and_writev_message(slot), + list( + RandomToken.create() + for i + in range(required_pass_count) + ), + ) + test, read = self.storage_server.doRemoteCall( + "slot_testv_and_readv_and_writev", + (), + dict( + passes=valid_passes, + storage_index=slot, + secrets=secrets, + tw_vectors=tw_vectors, + r_vector=[], + ), + ) + self.assertThat( + test, + Equals(True), + "Server denied initial write.", + ) + + expected_sizes = { + sharenum: get_implied_data_length(data_vector, new_length) + for (sharenum, (testv, data_vector, new_length)) + in tw_vectors.items() + if sharenums is None or sharenum in sharenums + } + + actual_sizes = self.storage_server.doRemoteCall( + "share_sizes", ( + slot, + sharenums, + ), + {}, + ) + self.assertThat( + actual_sizes, + Equals(expected_sizes), + ) diff --git a/zkapauthorizer.nix b/zkapauthorizer.nix index 75c594d97a048f3439b632231ed835fd9869f33d..462d8fbc23d692ccce570900ef4efc963e59c411 100644 --- a/zkapauthorizer.nix +++ b/zkapauthorizer.nix @@ -3,9 +3,11 @@ , fixtures, testtools, hypothesis, pyflakes, treq, coverage , hypothesisProfile ? null , collectCoverage ? false +, testSuite ? null }: let hypothesisProfile' = if hypothesisProfile == null then "default" else hypothesisProfile; + testSuite' = if testSuite == null then "_zkapauthorizer" else testSuite; in buildPythonPackage rec { version = "0.0"; @@ -45,7 +47,7 @@ buildPythonPackage rec { ZKAPAUTHORIZER_HYPOTHESIS_PROFILE=${hypothesisProfile'} python -m ${if collectCoverage then "coverage run --branch --source _zkapauthorizer,twisted.plugins.zkapauthorizer --module" else "" - } twisted.trial _zkapauthorizer + } twisted.trial ${testSuite'} runHook postCheck ''; @@ -54,6 +56,7 @@ buildPythonPackage rec { python -m coverage html mkdir -p "$doc/share/doc/${name}" cp -vr .coverage htmlcov "$doc/share/doc/${name}" + python -m coverage report '' else ""; }