diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py index 8a3a0956660984a5eb1b5d05cf8fb04759f6870d..6d15951d6411f9861d4358b6043e66342c5c38e7 100644 --- a/src/_zkapauthorizer/lease_maintenance.py +++ b/src/_zkapauthorizer/lease_maintenance.py @@ -55,6 +55,8 @@ from allmydata.interfaces import ( from allmydata.util.hashutil import ( file_renewal_secret_hash, bucket_renewal_secret_hash, + file_cancel_secret_hash, + bucket_cancel_secret_hash, ) from .controller import ( @@ -165,6 +167,7 @@ def renew_leases( storage_indexes = yield iter_storage_indexes(visit_assets) renewal_secret = secret_holder.get_renewal_secret() + cancel_secret = secret_holder.get_cancel_secret() servers = list( server.get_storage_server() for server @@ -176,6 +179,7 @@ def renew_leases( yield renew_leases_on_server( min_lease_remaining, renewal_secret, + cancel_secret, storage_indexes, server, activity, @@ -189,6 +193,7 @@ def renew_leases( def renew_leases_on_server( min_lease_remaining, renewal_secret, + cancel_secret, storage_indexes, server, activity, @@ -202,8 +207,9 @@ def renew_leases_on_server( :param timedelta min_lease_remaining: The minimum amount of time remaining to allow on a lease without renewing it. - :param renewal_secret: A seed for the renewal secret hash calculation for - any leases which need to be renewed. + :param renewal_secret: See ``renew_lease``. + + :param cancel_secret: See ``renew_lease``. :param list[bytes] storage_indexes: The storage indexes to check. @@ -230,16 +236,19 @@ def renew_leases_on_server( # All shares have the same lease information. stat = stat_dict.popitem()[1] if needs_lease_renew(min_lease_remaining, stat, now): - yield renew_lease(renewal_secret, storage_index, server) + yield renew_lease(renewal_secret, cancel_secret, storage_index, server) -def renew_lease(renewal_secret, storage_index, server): +def renew_lease(renewal_secret, cancel_secret, storage_index, server): """ Renew the lease on the shares in one storage index on one server. :param renewal_secret: A seed for the renewal secret hash calculation for any leases which need to be renewed. + :param cancel_secret: A seed for the cancel secret hash calculation for + any leases which need to be renewed. + :param bytes storage_index: The storage index to operate on. :param StorageServer server: The storage server to operate on. @@ -254,9 +263,19 @@ def renew_lease(renewal_secret, storage_index, server): ), server.get_lease_seed(), ) - return server.renew_lease( + cancel_secret = bucket_cancel_secret_hash( + file_cancel_secret_hash( + cancel_secret, + storage_index, + ), + server.get_lease_seed(), + ) + # Use add_lease to add a new lease *or* renew an existing one with a + # matching renew secret. + return server.add_lease( storage_index, renew_secret, + cancel_secret, ) diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index 2ba763ab985d6414623a3b037f105dde1bf4bdd4..70ec582cb96ff560d5c9cdfa612689d3fd540d5a 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -827,7 +827,7 @@ def clocks(now=posix_safe_datetimes()): @implementer(IFilesystemNode) -@attr.s +@attr.s(frozen=True) class _LeafNode(object): _storage_index = attr.ib() diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py index 4e0143ce899ee19136c8ed309e902d013d989ea6..142eff8243620f67de7d2aabf396c3f7badde3b6 100644 --- a/src/_zkapauthorizer/tests/test_lease_maintenance.py +++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py @@ -160,12 +160,41 @@ class DummyStorageServer(object): def get_lease_seed(self): return self.lease_seed - def renew_lease(self, storage_index, renew_secret): + def add_lease(self, storage_index, renew_secret, cancel_secret): self.buckets[storage_index].lease_expiration = ( self.clock.seconds() + timedelta(days=31).total_seconds() ) +class SharesAlreadyExist(Exception): + pass + + +def create_shares(storage_server, storage_index, size, lease_expiration): + """ + Initialize a storage index ("bucket") with some shares. + + :param DummyServer storage_server: The server to populate with shares. + :param bytes storage_index: The storage index of the shares. + :param int size: The application data size of the shares. + :param int lease_expiration: The expiration time for the lease to attach + to the shares. + + :raise SharesAlreadyExist: If there are already shares at the given + storage index. + + :return: ``None`` + """ + if storage_index in storage_server.buckets: + raise SharesAlreadyExist( + "Cannot create shares for storage index where they already exist.", + ) + storage_server.buckets[storage_index] = ShareStat( + size=size, + lease_expiration=lease_expiration, + ) + + def lease_seeds(): return binary( min_size=20, @@ -439,7 +468,7 @@ class RenewLeasesTests(TestCase): """ Tests for ``renew_leases``. """ - @given(storage_brokers(clocks()), lists(leaf_nodes())) + @given(storage_brokers(clocks()), lists(leaf_nodes(), unique=True)) def test_renewed(self, storage_broker, nodes): """ ``renew_leases`` renews the leases of shares on all storage servers which @@ -451,6 +480,22 @@ class RenewLeasesTests(TestCase): secret_holder = SecretHolder(lease_secret, convergence_secret) min_lease_remaining = timedelta(days=3) + # Make sure that the storage brokers have shares at the storage + # indexes we're going to operate on. + for storage_server in storage_broker.get_connected_servers(): + for node in nodes: + try: + create_shares( + storage_server.get_storage_server(), + node.get_storage_index(), + size=123, + lease_expiration=int(storage_broker.clock.seconds()), + ) + except SharesAlreadyExist: + # If Hypothesis already put some shares in this storage + # index, that's okay too. + pass + def get_now(): return datetime.utcfromtimestamp( storage_broker.clock.seconds(),