From 5e8c31c3b6672421ab7f75bffe474d58a77decf8 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Sun, 22 Mar 2009 20:21:28 -0700 Subject: [PATCH] storage: use constant-time comparison for write-enablers and lease-secrets --- src/allmydata/storage/immutable.py | 5 +++-- src/allmydata/storage/mutable.py | 9 ++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py index 61d277c6..cc58d44f 100644 --- 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 59e662c3..6282af59 100644 --- 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," -- 2.45.2