[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[minion-cvs] Gurski found a bug in expired-key removal. This (untes...



Update of /home/minion/cvsroot/src/minion/lib/mixminion/server
In directory moria.mit.edu:/tmp/cvs-serv17488/lib/mixminion/server

Modified Files:
	HashLog.py ServerKeys.py 
Log Message:
Gurski found a bug in expired-key removal.  This (untested) patch
should fix it.

Bump version back down to 0.0.4rc4.



Index: HashLog.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/server/HashLog.py,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -d -r1.15 -r1.16
--- HashLog.py	6 Jun 2003 06:04:58 -0000	1.15
+++ HashLog.py	10 Jun 2003 10:40:21 -0000	1.16
@@ -13,10 +13,10 @@
 import anydbm, dumbdbm
 import threading
 from mixminion.Common import MixFatalError, LOG, createPrivateDir, readFile, \
-     tryUnlink
+     secureDelete, tryUnlink
 from mixminion.Packet import DIGEST_LEN
 
-__all__ = [ 'HashLog' ]
+__all__ = [ 'HashLog', 'getHashLog', 'deleteHashLog' ]
 
 # FFFF Mechanism to force a different default db module.
 
@@ -27,9 +27,8 @@
 MAX_JOURNAL = 128
 
 # Lock to pretect _OPEN_HASHLOGS
-_HASHLOG_DICT_LOCK = threading.Lock()
-# Map from (filename,keyid) to the open HashLog object with that fname and
-# ID.  Needed to implement getHashLog.
+_HASHLOG_DICT_LOCK = threading.RLock()
+# Map from (filename) to (keyid,open HashLog). Needed to implement getHashLog.
 _OPEN_HASHLOGS = {}
 
 def getHashLog(filename, keyid):
@@ -40,14 +39,43 @@
     try:
         _HASHLOG_DICT_LOCK.acquire()
         try:
-            return _OPEN_HASHLOGS[(filename, keyid)]
+            keyid_orig, hl = _OPEN_HASHLOGS[filename]
+            if keyid != keyid_orig:
+                raise MixFatalError("KeyID changed for hashlog %s"%filename)
+            LOG.trace("getHashLog() returning open hashlog at %s",filename)
         except KeyError:
+            LOG.trace("getHashLog() opening hashlog at %s",filename)
             hl = HashLog(filename, keyid)
-            _OPEN_HASHLOGS[(filename, keyid)] = hl
-            return hl
+            _OPEN_HASHLOGS[filename] = (keyid, hl)
+        return hl
     finally:
         _HASHLOG_DICT_LOCK.release()
 
+def deleteHashLog(filename):
+    """Remove all files associated with a hashlog."""
+    try:
+        _HASHLOG_DICT_LOCK.acquire()
+        try:
+            _, hl = _OPEN_HASHLOGS[filename]
+            LOG.trace("deleteHashLog() removing open hashlog at %s",filename)
+            hl.close()
+        except KeyError:
+            LOG.trace("deleteHashLog() removing closed hashlog at %s",filename)
+            pass
+        remove = []
+        parent,name = os.path.split(filename)
+        prefix1 = name+"."
+        prefix2 = name+"."
+        if os.path.exists(parent):
+            for fn in os.listdir(parent):
+                if fn.startswith(prefix1) or fn.startswith(prefix2):
+                    files.append(os.path.join(parent, fn))
+        remove = [f for f in remove if os.path.exists(f)]                
+        secureDelete(remove, blocking=1)
+    finally:
+        _HASHLOG_DICT_LOCK.release()
+        
+
 # flags to pass to os.open when opening the journal file.
 _JOURNAL_OPEN_FLAGS = os.O_WRONLY|os.O_CREAT|getattr(os,'O_SYNC',0)
 class HashLog:
@@ -181,18 +209,17 @@
     def close(self):
         """Closes this log."""
         try:
+            _HASHLOG_DICT_LOCK.acquire()
             self.__lock.acquire()
+            LOG.trace("Closing hashlog at self.filename")
             self.sync()
             self.log.close()
+            self.log = None
             os.close(self.journalFile)
             try:
-                _HASHLOG_DICT_LOCK.acquire()
-                try:
-                    del _OPEN_HASHLOGS[(self.filename, self.keyid)]
-                except KeyError:
-                    pass
-            finally:
-                _HASHLOG_DICT_LOCK.release()
-                    
+                del _OPEN_HASHLOGS[self.filename]
+            except KeyError:
+                pass
         finally:
             self.__lock.release()
+            _HASHLOG_DICT_LOCK.release()

Index: ServerKeys.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/server/ServerKeys.py,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -d -r1.43 -r1.44
--- ServerKeys.py	6 Jun 2003 09:12:52 -0000	1.43
+++ ServerKeys.py	10 Jun 2003 10:40:21 -0000	1.44
@@ -343,9 +343,18 @@
         return nextKeygen
 
     def removeDeadKeys(self, now=None):
-        """Remove all keys that have expired"""
+        """Remove all keys that have expired."""
+        self.checkKeys()
+        keys = self.getDeadKeys(now)
+        for message, keyset in keys:
+            LOG.info(message)
+            keyset.delete()
         self.checkKeys()
 
+    def getDeadKeys(self,now=None):
+        """DOCDOC
+           doesn't checkKeys.
+        """
         if now is None:
             now = time.time()
             expiryStr = " expired"
@@ -354,15 +363,16 @@
 
         cutoff = now - self.keyOverlap
 
+        result = []
         for va, vu, keyset in self.keySets:
             if vu >= cutoff:
                 continue
             name = keyset.keyname
-            LOG.info("Removing%s key %s (valid from %s through %s)",
-                     expiryStr, name, formatDate(va), formatDate(vu-3600))
-            keyset.delete()
+            message ="Removing%s key %s (valid from %s through %s)"%(
+                      expiryStr, name, formatDate(va), formatDate(vu))
+            result.append((message, keyset))
 
-        self.checkKeys()
+        return result
 
     def _getLiveKeys(self, now=None):
         """Find all keys that are now valid.  Return list of (Valid-after,
@@ -427,9 +437,14 @@
 
            This function is idempotent.
         """
-        self.removeDeadKeys()
+        self.checkKeys()
+        deadKeys = self.getDeadKeys()
         self.currentKeys = keys = self.getServerKeysets(when)
-        LOG.info("Updating keys: %s currently valid", len(keys))
+        keyNames = [k.keyname for k in keys]
+        deadKeyNames = [k.keyname for msg, k in deadKeys]
+        LOG.info("Updating keys: %s currently valid (%s); %s expired (%s)",
+                 len(keys), " ".join(keyNames),
+                 len(deadKeys), " ".join(deadKeyNames))
         if mmtpServer is not None:
             context = self._getTLSContext(keys[-1])
             mmtpServer.setContext(context)
@@ -439,8 +454,7 @@
 
             for k in keys:
                 packetKeys.append(k.getPacketKey())
-                hashLogs.append(mixminion.server.HashLog.getHashLog(
-                    k.getHashLogFileName(), k.getPacketKeyID()))
+                hashLogs.append(k.getHashLog())
             packetHandler.setKeys(packetKeys, hashLogs)
 
         if statusFile:
@@ -448,6 +462,13 @@
                     "".join(["%s\n"%k.getDescriptorFileName() for k in keys]),
                     0644)
 
+        for msg, ks in deadKeys:
+            LOG.info(msg)
+            ks.delete()
+
+        if deadKeys:
+            self.checkKeys()
+
         self.nextUpdate = None
         self.getNextKeyRotation(keys)
 
@@ -563,15 +584,8 @@
                  self.publishedFile,
                  self.hashlogFile ]
         files = [f for f in files if os.path.exists(f)]
-        hashdir, name = os.path.split(self.hashlogFile)
-        if os.path.exists(hashdir):
-            start1 = name+"."
-            start2 = name+"_"
-            for fn in os.listdir(hashdir):
-                if fn.startswith(start1) or fn.startswith(start2):
-                    files.append(os.path.join(hashdir, fn))
-
         secureDelete(files, blocking=1)
+        mixminion.server.HashLog.deleteHashLog(self.hashlogFile)
         os.rmdir(self.keydir)
 
     def load(self, password=None):
@@ -603,6 +617,9 @@
         if self.serverinfo is None:
             self.serverinfo = ServerInfo(fname=self.descFile)
         return self.serverinfo
+    def getHashLog(self):
+        return mixminion.server.HashLog.getHashLog(
+            self.getHashLogFileName(), self.getPacketKeyID())
     def getLiveness(self):
         """Return a 2-tuple of validAfter/validUntil for this server."""
         if self.validAfter is None or self.validUntil is None: