storage: use constant-time comparison for write-enablers and lease-secrets

diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py
index 61d277c63d27079eeab1a438715f5c569315f777..cc58d44f6ffce7f8aad1afaa031c04601a08d3f4 100644 (file)
--- a/src/allmydata/storage/immutable.py
+++ b/src/allmydata/storage/immutable.py
@@ -6,6 +6,7 @@ from zope.interface import implements
 from allmydata.interfaces import RIBucketWriter, RIBucketReader
 from allmydata.util import base32, fileutil, 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 UnknownImmutableContainerVersionError, \
      DataTooLargeError
@@ -142,7 +143,7 @@ class ShareFile:
 
     def renew_lease(self, renew_secret, new_expire_time):
         for i,lease in enumerate(self.get_leases()):
-            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
@@ -172,7 +173,7 @@ class ShareFile:
         leases = list(self.get_leases())
         num_leases_removed = 0
         for i,lease in enumerate(leases):
-            if lease.cancel_secret == cancel_secret:
+            if constant_time_compare(lease.cancel_secret, cancel_secret):
                 leases[i] = None
                 num_leases_removed += 1
         if not num_leases_removed:

diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py
index 59e662c3d7f07132b1c425281c4b6baa8de78fb6..6282af596693dfe7c2947d8ed210f995c322d66b 100644 (file)
--- a/src/allmydata/storage/mutable.py
+++ b/src/allmydata/storage/mutable.py
@@ -3,6 +3,7 @@ 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 UnknownMutableContainerVersionError, \
      DataTooLargeError
@@ -266,7 +267,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
@@ -312,7 +313,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:
@@ -365,7 +366,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,"