improve the storage/mutable.py asserts even more

[tahoe-lafs/tahoe-lafs.git] / src / allmydata / storage / mutable.py

diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py
index de78a6559039ac8b567bb6ac5f02eff35ada7c98..447e6bcc2c8172b0024156f2a58f439db9590e0b 100644 (file)
--- a/src/allmydata/storage/mutable.py
+++ b/src/allmydata/storage/mutable.py
@@ -3,8 +3,10 @@ import os, stat, struct
 from allmydata.interfaces import BadWriteEnablerError
 from allmydata.util import idlib, log
 from allmydata.util.assertutil import precondition
+from allmydata.util.hashutil import constant_time_compare
 from allmydata.storage.lease import LeaseInfo
-from allmydata.storage.common import DataTooLargeError
+from allmydata.storage.common import UnknownMutableContainerVersionError, \
+     DataTooLargeError
 
 # the MutableShareFile is like the ShareFile, but used for mutable data. It
 # has a different layout. See docs/mutable.txt for more details.
@@ -26,11 +28,14 @@ from allmydata.storage.common import DataTooLargeError
 # 9   ??        n*92    extra leases
 
 
-assert struct.calcsize("L"), 4 # The struct module doc says that L's are 4 bytes in size.
-assert struct.calcsize("Q"), 8 # The struct module doc says that Q's are 8 bytes in size (at least with big-endian ordering).
+# The struct module doc says that L's are 4 bytes in size., and that Q's are
+# 8 bytes in size. Since compatibility depends upon this, double-check it.
+assert struct.calcsize(">L") == 4, struct.calcsize(">L")
+assert struct.calcsize(">Q") == 8, struct.calcsize(">Q")
 
 class MutableShareFile:
 
+    sharetype = "mutable"
     DATA_LENGTH_OFFSET = struct.calcsize(">32s20s32s")
     EXTRA_LEASE_OFFSET = DATA_LENGTH_OFFSET + 8
     HEADER_SIZE = struct.calcsize(">32s20s32sQQ") # doesn't include leases
@@ -56,7 +61,10 @@ class MutableShareFile:
              write_enabler_nodeid, write_enabler,
              data_length, extra_least_offset) = \
              struct.unpack(">32s20s32sQQ", data)
-            assert magic == self.MAGIC
+            if magic != self.MAGIC:
+                msg = "sharefile %s had magic '%r' but we wanted '%r'" % \
+                      (filename, magic, self.MAGIC)
+                raise UnknownMutableContainerVersionError(msg)
         self.parent = parent # for logging
 
     def log(self, *args, **kwargs):
@@ -230,23 +238,22 @@ class MutableShareFile:
                 return i
         return None
 
+    def get_leases(self):
+        """Yields a LeaseInfo instance for all leases."""
+        f = open(self.home, 'rb')
+        for i, lease in self._enumerate_leases(f):
+            yield lease
+        f.close()
+
     def _enumerate_leases(self, f):
-        """Yields (leasenum, (ownerid, expiration_time, renew_secret,
-        cancel_secret, accepting_nodeid)) for all leases."""
         for i in range(self._get_num_lease_slots(f)):
             try:
                 data = self._read_lease_record(f, i)
                 if data is not None:
-                    yield (i,data)
+                    yield i,data
             except IndexError:
                 return
 
-    def debug_get_leases(self):
-        f = open(self.home, 'rb')
-        leases = list(self._enumerate_leases(f))
-        f.close()
-        return leases
-
     def add_lease(self, lease_info):
         precondition(lease_info.owner_num != 0) # 0 means "no lease here"
         f = open(self.home, 'rb+')
@@ -262,7 +269,7 @@ class MutableShareFile:
         accepting_nodeids = set()
         f = open(self.home, 'rb+')
         for (leasenum,lease) in self._enumerate_leases(f):
-            if lease.renew_secret == renew_secret:
+            if constant_time_compare(lease.renew_secret, renew_secret):
                 # yup. See if we need to update the owner time.
                 if new_expire_time > lease.expiration_time:
                     # yes
@@ -308,7 +315,7 @@ class MutableShareFile:
         f = open(self.home, 'rb+')
         for (leasenum,lease) in self._enumerate_leases(f):
             accepting_nodeids.add(lease.nodeid)
-            if lease.cancel_secret == cancel_secret:
+            if constant_time_compare(lease.cancel_secret, cancel_secret):
                 self._write_lease_record(f, leasenum, blank_lease)
                 modified += 1
             else:
@@ -361,7 +368,9 @@ class MutableShareFile:
         (real_write_enabler, write_enabler_nodeid) = \
                              self._read_write_enabler_and_nodeid(f)
         f.close()
-        if write_enabler != real_write_enabler:
+        # avoid a timing attack
+        #if write_enabler != real_write_enabler:
+        if not constant_time_compare(write_enabler, real_write_enabler):
             # accomodate share migration by reporting the nodeid used for the
             # old write enabler.
             self.log(format="bad write enabler on SI %(si)s,"