From 8b976b441e793f45b50e5d5ebcb4314beba889ee Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 28 Oct 2021 12:05:34 -0400 Subject: [PATCH] add LeaseInfo.is_renew_secret and use it --- src/allmydata/storage/immutable.py | 2 +- src/allmydata/storage/lease.py | 12 +++++++++ src/allmydata/storage/mutable.py | 2 +- src/allmydata/test/test_storage.py | 39 ++++++++++++++++++++++-------- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py index 24465c1ed..c9b8995b5 100644 --- a/src/allmydata/storage/immutable.py +++ b/src/allmydata/storage/immutable.py @@ -180,7 +180,7 @@ class ShareFile(object): secret. """ for i,lease in enumerate(self.get_leases()): - if timing_safe_compare(lease.renew_secret, renew_secret): + if lease.is_renew_secret(renew_secret): # yup. See if we need to update the owner time. if allow_backdate or new_expire_time > lease.get_expiration_time(): # yes diff --git a/src/allmydata/storage/lease.py b/src/allmydata/storage/lease.py index 17683a888..2132048ce 100644 --- a/src/allmydata/storage/lease.py +++ b/src/allmydata/storage/lease.py @@ -15,6 +15,8 @@ import struct, time import attr +from allmydata.util.hashutil import timing_safe_compare + @attr.s(frozen=True) class LeaseInfo(object): """ @@ -68,6 +70,16 @@ class LeaseInfo(object): _expiration_time=new_expire_time, ) + def is_renew_secret(self, candidate_secret): + # type: (bytes) -> bool + """ + Check a string to see if it is the correct renew secret. + + :return: ``True`` if it is the correct renew secret, ``False`` + otherwise. + """ + return timing_safe_compare(self.renew_secret, candidate_secret) + def get_grant_renew_time_time(self): # hack, based upon fixed 31day expiration period return self._expiration_time - 31*24*60*60 diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py index 1b29b4a65..017f2dbb7 100644 --- a/src/allmydata/storage/mutable.py +++ b/src/allmydata/storage/mutable.py @@ -327,7 +327,7 @@ class MutableShareFile(object): accepting_nodeids = set() with open(self.home, 'rb+') as f: for (leasenum,lease) in self._enumerate_leases(f): - if timing_safe_compare(lease.renew_secret, renew_secret): + if lease.is_renew_secret(renew_secret): # yup. See if we need to update the owner time. if allow_backdate or new_expire_time > lease.get_expiration_time(): # yes diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 8123be2c5..005309f87 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -755,28 +755,28 @@ class Server(unittest.TestCase): # Create a bucket: rs0, cs0 = self.create_bucket_5_shares(ss, b"si0") - leases = list(ss.get_leases(b"si0")) - self.failUnlessEqual(len(leases), 1) - self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs0])) + (lease,) = ss.get_leases(b"si0") + self.assertTrue(lease.is_renew_secret(rs0)) rs1, cs1 = self.create_bucket_5_shares(ss, b"si1") # take out a second lease on si1 rs2, cs2 = self.create_bucket_5_shares(ss, b"si1", 5, 0) - leases = list(ss.get_leases(b"si1")) - self.failUnlessEqual(len(leases), 2) - self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs1, rs2])) + (lease1, lease2) = ss.get_leases(b"si1") + self.assertTrue(lease1.is_renew_secret(rs1)) + self.assertTrue(lease2.is_renew_secret(rs2)) # and a third lease, using add-lease rs2a,cs2a = (hashutil.my_renewal_secret_hash(b"%d" % next(self._lease_secret)), hashutil.my_cancel_secret_hash(b"%d" % next(self._lease_secret))) ss.remote_add_lease(b"si1", rs2a, cs2a) - leases = list(ss.get_leases(b"si1")) - self.failUnlessEqual(len(leases), 3) - self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs1, rs2, rs2a])) + (lease1, lease2, lease3) = ss.get_leases(b"si1") + self.assertTrue(lease1.is_renew_secret(rs1)) + self.assertTrue(lease2.is_renew_secret(rs2)) + self.assertTrue(lease3.is_renew_secret(rs2a)) # add-lease on a missing storage index is silently ignored - self.failUnlessEqual(ss.remote_add_lease(b"si18", b"", b""), None) + self.assertIsNone(ss.remote_add_lease(b"si18", b"", b"")) # check that si0 is readable readers = ss.remote_get_buckets(b"si0") @@ -3028,3 +3028,22 @@ class ShareFileTests(unittest.TestCase): sf = self.get_sharefile() with self.assertRaises(IndexError): sf.cancel_lease(b"garbage") + + def test_renew_secret(self): + """ + A lease loaded from a share file can have its renew secret verified. + """ + renew_secret = b"r" * 32 + cancel_secret = b"c" * 32 + expiration_time = 2 ** 31 + + sf = self.get_sharefile() + lease = LeaseInfo( + owner_num=0, + renew_secret=renew_secret, + cancel_secret=cancel_secret, + expiration_time=expiration_time, + ) + sf.add_lease(lease) + (loaded_lease,) = sf.get_leases() + self.assertTrue(loaded_lease.is_renew_secret(renew_secret))