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

[minion-cvs] Flushing patches from my laptop)



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

Modified Files:
	BuildMessage.py ClientMain.py Common.py MMTPClient.py 
	Packet.py test.py 
Log Message:
(Flushing patches from my laptop)

BuildMessage, ClientMain:
	- Improve comments and documentation

Common:
	- Repair file mermissions better
	- Don't assume all filesystems have the same block size

MMTPClient, MMTPServer:
	- Increment protocol version; drop 0.1 and 0.2 support.

Packet:
	- Drop obsolete packet format

test:
	- Tests for rejected packets
	- Tests for EventStats
	
EventStats:
	- Debug; make rotation explicit

Hashlog:
	- Don't die when a half-created hashlog is found.
	- Sync on startup

Modules:
	- Don't leak zombie mixmaster processes when flushing

PacketHandler:
	- Change key rotation semantics from add/remove to set.

ServerConfig:
	- Upgrade comments from ???? to ????004

ServerKeys
	- Fix hashlog deletion

ServerMain:
	- Refactor scheduler into a separate class.
	- Drop umask in all cases, even when not daemon.

ServerQueue:
	- Drop some legacy code










Index: BuildMessage.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/BuildMessage.py,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -d -r1.43 -r1.44
--- BuildMessage.py	26 Apr 2003 14:39:58 -0000	1.43
+++ BuildMessage.py	5 May 2003 00:38:45 -0000	1.44
@@ -6,8 +6,10 @@
    Code to construct messages and reply blocks, and to decode received
    message payloads."""
 
-import sys
 import operator
+import sys
+import types
+
 import mixminion.Crypto as Crypto
 from mixminion.Packet import *
 from mixminion.Common import MixError, MixFatalError, LOG, UIError
@@ -238,13 +240,17 @@
                                 seed)[0]
 
 def checkPathLength(path1, path2, exitType, exitInfo, explicitSwap=0):
-    "XXXX DOCDOC"
-    err = 0
+    """Given two path legs, an exit type and an exitInfo, raise an error
+       if we can't build a hop with the provided legs.
+
+       The leg "path1" may be null."""
+    err = 0 # 0: no error. 1: 1st leg too big. 2: 1st leg okay, 2nd too big.
     if path1 is not None:
         try:
             _getRouting(path1, SWAP_FWD_TYPE, path2[0].getRoutingInfo().pack())
         except MixError:
             err = 1
+    # Add tag as needed to last exitinfo.
     if exitType != DROP_TYPE and exitInfo is not None:
         exitInfo += "X"*20
     else:
@@ -264,23 +270,20 @@
 # MESSAGE DECODING
 
 def decodePayload(payload, tag, key=None,
-                  userKey=None,
                   userKeys={}):
     """Given a 28K payload and a 20-byte decoding tag, attempt to decode and
        decompress the original message.
 
            key: an RSA key to decode encrypted forward messages, or None
-           userKey: our encryption key for reply blocks, or None.
+           userKeys: a map from identity names to keys for reply blocks,
+                or None.
 
        If we can successfully decrypt the payload, we return it.  If we
        might be able to decrypt the payload given more/different keys,
        we return None.  If the payload is corrupt, we raise MixError.
-
-       DOCDOC userKeys
     """
-    # FFFF Take a list of keys?
-    if userKey and not userKeys:
-        userKeys = { "" : userKey }
+    if type(userKeys) is types.StringType:
+        userKeys = { "" : userKeys }
 
     if len(payload) != PAYLOAD_LEN or len(tag) != TAG_LEN:
         raise MixError("Wrong payload or tag length")
@@ -469,11 +472,6 @@
     headerKeys = [ Crypto.Keyset(secret).get(Crypto.HEADER_SECRET_MODE)
                        for secret in secrets ]
 
-##     # junkKeys[i]==the AES key object that node i will use to re-pad the
-##     # header.
-##     junkKeys = [ Crypto.Keyset(secret).get(Crypto.RANDOM_JUNK_MODE)
-##                        for secret in secrets ]
-
     # Length of padding needed for the header
     paddingLen = HEADER_LEN - totalSize
 
@@ -531,21 +529,23 @@
             underflow = ""
 
         # Do we need to spill some of the routing info out from the
-        # RSA-encrypted portion?
-        #XXXX004 most of these asserts are silly.
-        assert not subhead.getOverflow() or not subhead.getUnderflowLength()
+        # RSA-encrypted portion?  If so, prepend it.
         header = subhead.getOverflow() + header
 
+        # Encrypt the symmetrically encrypted part of the header
         header = Crypto.ctr_crypt(header, headerKeys[i])
 
-        assert len(header)+len(junkSeen[i])+ENC_SUBHEADER_LEN == HEADER_LEN
+        # What digest will the next server see?
         subhead.digest = Crypto.sha1(header+junkSeen[i])
+
+        # Encrypt the subheader, plus whatever portion of the previous header
+        # underflows, into 'esh'.
         pubkey = path[i].getPacketKey()
         rsaPart = subhead.pack() + underflow
-        assert len(rsaPart) + OAEP_OVERHEAD == ENC_SUBHEADER_LEN
-        assert pubkey.get_modulus_bytes() == ENC_SUBHEADER_LEN
         esh = Crypto.pk_encrypt(rsaPart, pubkey)
-        assert len(esh) == ENC_SUBHEADER_LEN == 256
+
+        # Concatentate the asymmetric and symmetric parts, to get the next
+        # header.
         header = esh + header
 
     return header
@@ -647,7 +647,9 @@
 
     # Uncompress the body.
     contents = payload.getContents()
-    # ???? Should we make this rule configurable?  I say no.
+    # If the payload would expand to be more than 20K long, and the
+    # compression factor is greater than 20, we warn of a possible zlib
+    # bomb.
     maxLen = max(20*1024, 20*len(contents))
 
     return uncompressData(contents, maxLength=maxLen)
@@ -657,7 +659,15 @@
     return payload[2:22] == Crypto.sha1(payload[22:])
 
 def _getRouting(path, exitType, exitInfo):
-    "XXXX DOCDOC"
+    """Given a list of ServerInfo, and a final exitType and exitInfo,
+       return a 3-tuple of:
+           1) A list of routingtype/routinginfo tuples for the header
+           2) The size (in bytes) added to the header in order to
+              route to each of the nodes
+           3) Minimum size (in bytes) needed for the header.  If this
+              is greater than HEADER_LEN, we can't build the header at
+              all.
+        """
     # Construct a list 'routing' of exitType, exitInfo.
     routing = [ (FWD_TYPE, node.getRoutingInfo().pack()) for
                 node in path[1:] ]

Index: ClientMain.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ClientMain.py,v
retrieving revision 1.71
retrieving revision 1.72
diff -u -d -r1.71 -r1.72
--- ClientMain.py	26 Apr 2003 14:39:58 -0000	1.71
+++ ClientMain.py	5 May 2003 00:38:45 -0000	1.72
@@ -909,14 +909,13 @@
     return path2
     
 class ClientKeyring:
-    # XXXX004 testme
     """Class to manage storing encrypted keys for a client.  Right now, this
        is limited to a single SURB decryption key.  In the future, we may
        include more SURB keys, as well as end-to-end encryption keys.
     """
     ## Fields:
     # keyDir: The directory where we store our keys.
-    # keyring: DICT XXXX
+    # keyring: DICT XXXX DOCDOC
     # keyringPassword: The password for our encrypted keyfile
     ## Format:
     # We store keys in a file holding:
@@ -1434,7 +1433,6 @@
               messages)
             servers1,servers2 -- lists of ServerInfo.
             """
-        #XXXX004 write unit tests
         routingType, routingInfo, _ = address.getRouting()
         LOG.info("Generating payload...")
         msg = mixminion.BuildMessage.buildForwardMessage(
@@ -2122,8 +2120,6 @@
 #     change between now and 1.0.0
 def runClient(cmd, args):
     #DOCDOC Comment this function
-    if cmd.endswith(" client"): #XXXX004 remove this.
-        print "The 'client' command is deprecated.  Use 'send' instead."
     queueMode = 0
     if cmd.endswith(" queue"):
         queueMode = 1

Index: Common.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Common.py,v
retrieving revision 1.70
retrieving revision 1.71
diff -u -d -r1.70 -r1.71
--- Common.py	26 Apr 2003 14:39:58 -0000	1.70
+++ Common.py	5 May 2003 00:38:45 -0000	1.71
@@ -161,6 +161,7 @@
        0700, and all its parents pass checkPrivateDir.  Raises MixFatalError
        if the assumtions are not met; else return None.  If 'fix' is true,
        repair permissions on the file rather than raising MixFatalError."""
+    #XXXX004 testme
     parent, _ = os.path.split(fn)
     if not checkPrivateDir(parent):
         return None
@@ -176,7 +177,7 @@
     if mode not in (0700, 0600):
         if not fix:
             raise MixFatalError("Bad mode %o on file %s" % mode)
-        newmode = {0:0600,0100:0700}[(newmode & 0100)]
+        newmode = {0:0600,0100:0700}[(mode & 0100)]
         LOG.warn("Repairing permissions on file %s" % fn)
         os.chmod(fn, newmode)
 
@@ -275,30 +276,33 @@
     _SHRED_CMD, _SHRED_OPTS = cmd, opts
 
 
-# Size of a block on the filesystem we're overwriting on; If zero, we need
-# to determine it.
-_BLKSIZE = 0
-# A string of _BLKSIZE zeros
-_NILSTR = None
+# Map from parent directory to blocksize.  We only overwrite files in a few
+# locations, so this should be safe.
+_BLKSIZEMAP = {}
+# A string of max(_BLKSIZEMAP.values()) zeros
+_NILSTR = ""
 def _overwriteFile(f):
     """Overwrite f with zeros, rounding up to the nearest block.  This is
        used as the default implementation of secureDelete."""
-    global _BLKSIZE
     global _NILSTR
-    if not _BLKSIZE:
-        #???? this assumes that all filesystems we are using have the same
-        #???? block size.
+    parent = os.path.split(f)[0]
+    try:
+        sz = _BLKSIZEMAP[parent]
+    except KeyError:
         if hasattr(os, 'statvfs'):
-            _BLKSIZE = os.statvfs(f)[statvfs.F_BSIZE]
+            sz = os.statvfs(f)[statvfs.F_BSIZE]
         else:
-            _BLKSIZE = 8192 # ???? Safe guess?
-        _NILSTR = '\x00' * _BLKSIZE
+            sz = 8192 # Should be a safe guess? (????)
+        _BLKSIZEMAP[parent] = sz
+        if sz > len(_NILSTR):
+            _NILSTR = '\x00' * sz
+    nil = _NILSTR[:sz]
     fd = os.open(f, os.O_WRONLY)
     try:
         size = os.fstat(fd)[stat.ST_SIZE]
-        blocks = ceilDiv(size, _BLKSIZE)
+        blocks = ceilDiv(size, sz)
         for _ in xrange(blocks):
-            os.write(fd, _NILSTR)
+            os.write(fd, nil)
     finally:
         os.close(fd)
 

Index: MMTPClient.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPClient.py,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -d -r1.28 -r1.29
--- MMTPClient.py	26 Apr 2003 14:39:58 -0000	1.28
+++ MMTPClient.py	5 May 2003 00:38:45 -0000	1.29
@@ -11,9 +11,7 @@
    around [A] so that clients have an easy (blocking) interface to
    introduce messages into the system, and [B] so that we've got an
    easy-to-verify reference implementation of the protocol.)
-
-   FFFF: As yet unsupported are: Session resumption and key renegotiation.
-   FFFF: Also unsupported: timeouts."""
+   """
 
 __all__ = [ "BlockingClientConnection", "sendMessages" ]
 
@@ -45,7 +43,7 @@
     #     if negotiation hasn't completed.
     # PROTOCOL_VERSIONS: (static) a list of protocol versions we allow,
     #     in decreasing order of preference.
-    PROTOCOL_VERSIONS = ['0.2', '0.1']
+    PROTOCOL_VERSIONS = ['0.3']
     def __init__(self, targetIP, targetPort, targetKeyID):
         """Open a new connection."""
         self.targetIP = targetIP
@@ -115,7 +113,6 @@
             
         LOG.debug("Handshaking with %s:%s",self.targetIP, self.targetPort)
         self.tls = self.context.sock(self.sock.fileno())
-        # FFFF session resumption
         self.tls.connect()
         LOG.debug("Connected.")
 
@@ -126,7 +123,7 @@
 
         ####
         # Protocol negotiation
-        # For now, we only support 1.0, but we call it 0.1 so we can
+        # For now, we only support 1.0, but we call it 0.3 so we can
         # change our mind between now and a release candidate, and so we
         # can obsolete betas come release time.
         LOG.debug("Negotiatiating MMTP protocol")
@@ -160,9 +157,6 @@
 
     def sendJunkPacket(self, packet):
         """Send a single 32K junk packet to the server."""
-        if self.protocol == '0.1':
-            LOG.debug("Not sending junk to a v0.1 server")
-            return
         self._sendPacket(packet,
                          control="JUNK\r\n", serverControl="RECEIVED\r\n",
                          hashExtra="JUNK", serverHashExtra="RECEIVED JUNK")
@@ -264,7 +258,6 @@
     sendMessages(routing, ["JUNK"], connectTimeout=connectTimeout)
     
 class PeerCertificateCache:
-    #XXXX004 use this properly; flush it to disk.
     "DOCDOC"
     def __init__(self):
         self.cache = {} # hashed peer pk -> identity keyid that it is valid for
@@ -281,12 +274,19 @@
 
         peer_pk = tls.get_peer_cert_pk()
         hashed_peer_pk = sha1(peer_pk.encode_key(public=1))
-        #XXXX004 Remove this option
+        # Before 0.0.4alpha, a server's keyID was a hash of its current
+        # TLS public key.  In 0.0.4alpha, we allowed this for backward
+        # compatibility.  As of 0.0.4alpha2, since we've dropped backward
+        # compatibility with earlier packet formats, we drop certificate
+        # compatibility as well.
         if targetKeyID == hashed_peer_pk:
-            LOG.warn("Non-rotatable keyid from server at %s", address)
-            return # raise MixProtocolError
+            raise MixProtocolBadAuth(
+               "Pre-0.0.4 (non-rotatable) certificate from server at %s",
+               address)
         try:
             if self.cache[hashed_peer_pk] == targetKeyID:
+                LOG.trace("Got a cached certificate from server at %s",
+                          address)
                 return # All is well.
             else:
                 raise MixProtocolBadAuth(
@@ -302,6 +302,8 @@
                                    %(address, e))
 
         hashed_identity = sha1(identity.encode_key(public=1))
+        LOG.trace("Remembering valid certificate for server at %s",
+                  address)
         self.cache[hashed_peer_pk] = hashed_identity
         if hashed_identity != targetKeyID:
             raise MixProtocolBadAuth("Invalid KeyID for server at %s" %address)

Index: Packet.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Packet.py,v
retrieving revision 1.39
retrieving revision 1.40
diff -u -d -r1.39 -r1.40
--- Packet.py	26 Apr 2003 14:39:58 -0000	1.39
+++ Packet.py	5 May 2003 00:38:45 -0000	1.40
@@ -576,8 +576,7 @@
 MESSAGE_END_LINE   = "======== TYPE III ANONYMOUS MESSAGE ENDS ========"
 _MESSAGE_START_RE  = re.compile(r"==+ TYPE III ANONYMOUS MESSAGE BEGINS ==+")
 _MESSAGE_END_RE    = re.compile(r"==+ TYPE III ANONYMOUS MESSAGE ENDS ==+")
-#XXXX004 disable "decoding handle" format
-_FIRST_LINE_RE = re.compile(r'''^Decoding[- ]handle:\s(.*)\r*\n|
+_FIRST_LINE_RE = re.compile(r'''^Decoding-handle:\s(.*)\r*\n|
                                  Message-type:\s(.*)\r*\n''', re.X+re.S)
 _LINE_RE = re.compile(r'[^\r\n]*\r*\n', re.S+re.M)
 
@@ -689,9 +688,7 @@
         if self.messageType != 'TXT':
             c = encodeBase64(c)
         else:
-            #XXXX004 disable "decoding handle" format
             if (c.startswith("Decoding-handle:") or
-                c.startswith("Decoding handle:") or
                 c.startswith("Message-type:")):
                 preNL = "\n"
                 

Index: test.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/test.py,v
retrieving revision 1.100
retrieving revision 1.101
diff -u -d -r1.100 -r1.101
--- test.py	26 Apr 2003 14:39:59 -0000	1.100
+++ test.py	5 May 2003 00:38:45 -0000	1.101
@@ -2855,14 +2855,18 @@
             ident = getRSAKey(0,2048)
             generateCertChain(certfile, pk, ident, "<testing>",
                               time.time(), time.time()+365*24*60*60)
-            
 
         pk = _ml.rsa_PEM_read_key(open(pkfile, 'r'), 0)
         return _ml.TLSContext_new(certfile, pk, dhfile)
     else:
         return _ml.TLSContext_new()
 
-def _getMMTPServer(minimal=0):
+def _getTLSContextKeyID():
+    ident = getRSAKey(0,2048)
+    keyid = sha1(ident.encode_key(1))
+    return keyid
+
+def _getMMTPServer(minimal=0,reject=0):
     """Helper function: create a new MMTP server with a listener connection
        Return a tuple of AsyncServer, ListenerConnection, list of received
        messages, and keyid."""
@@ -2873,11 +2877,13 @@
     server.nJunkPackets = 0
     def junkCallback(server=server): server.nJunkPackets += 1
     def conFactory(sock, context=_getTLSContext(1),
-                   receiveMessage=receivedHook,junkCallback=junkCallback):
+                   receiveMessage=receivedHook,junkCallback=junkCallback,
+                   reject=reject):
         tls = context.sock(sock, serverMode=1)
         sock.setblocking(0)
         con = mixminion.server.MMTPServer.MMTPServerConnection(sock,tls,
-                                                               receiveMessage)
+                                                               receiveMessage,
+                                                         rejectPackets=reject)
         con.junkCallback = junkCallback
         return con
     def conFactoryMin(sock, context=_getTLSContext(1)):
@@ -2892,12 +2898,13 @@
     listener = mixminion.server.MMTPServer.ListenConnection("127.0.0.1",
                                                  TEST_PORT, 5, conFactory)
     listener.register(server)
-    pk = _ml.rsa_PEM_read_key(open(pkfile, 'r'), public=0)
-    keyid = sha1(pk.encode_key(1))
+    keyid = _getTLSContextKeyID()
 
     return server, listener, messagesIn, keyid
 
 class MMTPTests(unittest.TestCase):
+    #XXXX This class is bulky, and has lots of cut-and-paste.  It could do
+    #XXXX with a refactoring.
     def doTest(self, fn):
         """Wraps an underlying test function 'fn' to make sure we kill the
            MMTPServer, but don't block."""
@@ -2923,6 +2930,9 @@
     def testTimeout(self):
         self.doTest(self._testTimeout)
 
+    def testRejected(self):
+        self.doTest(self._testRejected)
+
     def _testBlockingTransmission(self):
         server, listener, messagesIn, keyid = _getMMTPServer()
         self.listener = listener
@@ -3131,6 +3141,60 @@
         for _ in xrange(3):
             server.process(0.1)
 
+    def _testRejected(self):
+        server, listener, messagesIn, keyid = _getMMTPServer(reject=1)
+        self.listener = listener
+        self.server = server
+
+        messages = ["helloxxx"*4096, "helloyyy"*4096]
+
+        # Send 2 messages -- both should be rejected.
+        server.process(0.1)
+        routing = IPV4Info("127.0.0.1", TEST_PORT, keyid)
+        ok=[0];done=[0]
+        def _t(routing=routing, messages=messages,ok=ok,done=done):
+            try:
+                mixminion.MMTPClient.sendMessages(routing,messages)
+            except mixminion.Common.MixProtocolReject:
+                ok[0] = 1
+            done[0] = 1
+            
+        t = threading.Thread(None, _t)
+        t.start()
+        while not done[0]:
+            server.process(0.1)
+        t.join()
+        for _ in xrange(3):
+            server.process(0.1)
+
+        self.failUnless(ok[0])
+        self.failUnless(len(messagesIn) == 0)
+
+        # Send m1, then junk, then renegotiate, then junk, then m2.
+        messages = ["helloxxx"*4096, "helloyyy"*4096]
+        async = mixminion.server.MMTPServer.AsyncServer()
+        _failed_args = []
+        def _failed(msg, handle, retriable, _f=_failed_args):
+            _f.append((msg,handle,retriable))
+        clientcon = mixminion.server.MMTPServer.MMTPClientConnection(
+           _getTLSContext(0), "127.0.0.1", TEST_PORT, keyid,
+           messages, [None,None], None, failCallback=_failed)
+        clientcon.register(async)
+        def clientThread(clientcon=clientcon, async=async):
+            while not clientcon.isShutdown():
+                async.process(2)
+
+        t = threading.Thread(None, clientThread)
+        t.start()
+        while not clientcon.isShutdown():
+            server.process(0.1)
+        while t.isAlive():
+            server.process(0.1)
+        t.join()
+        self.assertEquals(len(messagesIn), 0)
+        self.assertEquals(_failed_args, [(messages[0], None, 1),
+                                         (messages[1], None, 1)])
+
 #----------------------------------------------------------------------
 # Config files
 
@@ -3520,9 +3584,9 @@
 
         eq(info['Incoming/MMTP']['Version'], "0.1")
         eq(info['Incoming/MMTP']['Port'], 48099)
-        eq(info['Incoming/MMTP']['Protocols'], "0.1,0.2")
+        eq(info['Incoming/MMTP']['Protocols'], "0.3")
         eq(info['Outgoing/MMTP']['Version'], "0.1")
-        eq(info['Outgoing/MMTP']['Protocols'], "0.1,0.2")
+        eq(info['Outgoing/MMTP']['Protocols'], "0.3")
         eq(info['Incoming/MMTP']['Allow'], [("192.168.0.16", "255.255.255.255",
                                             1,1024),
                                            ("0.0.0.0", "0.0.0.0",
@@ -3594,8 +3658,6 @@
             os.path.join(keydir, "mix.key"))
         eq(packetKey.get_public_key(),
            info['Server']['Packet-Key'].get_public_key())
-        mmtpKey = Crypto.pk_PEM_load(
-            os.path.join(keydir, "mmtp.key"))
         eq(Crypto.sha1(identity.encode_key(1)),
            info['Incoming/MMTP']['Key-Digest'])
 
@@ -3841,6 +3903,7 @@
 
     def testEventLog(self):
         import mixminion.server.EventStats as ES
+        tm = time.time()
         eq = self.assertEquals
         homedir = mix_mktemp()
         ES.configureLog({'Server':
@@ -3861,7 +3924,7 @@
         ES.log.failedDelivery()
         ES.log.failedDelivery("Y")
         ES.log.unretriableDelivery("Y")
-        ES.log.save()
+        ES.log.save(now=tm)
         eq(ES.log.count['UnretriableDelivery']['Y'], 1)
         eq(ES.log.count['AttemptedDelivery'][None], 2)
         # Test reload.
@@ -3896,35 +3959,72 @@
 """
         eq(s, expected)
         # Test time accumultion.
-        ES.log.save(now=time.time()+1800)
+        ES.log.save(now=tm+1800)
         self.assert_(abs(1800-ES.log.accumulatedTime) < 10)
-        ES.log.lastSave = time.time()+1900
-        ES.log.save(now=time.time()+2000)
+        self.assertEquals(ES.log.lastSave, tm+1800)
+        ES.log.lastSave = tm+1900
+        ES.log.save(now=tm+2000)
         self.assert_(abs(1900-ES.log.accumulatedTime) < 10)
         # Test rotate
-        ES.log.save(now=time.time()+3600*24)
+        ES.log.rotate(now=tm+3600*24)
         s2 = readFile(os.path.join(homedir, "stats"))
-        eq(s2, sOrig)
+        eq(s2.split("\n")[1:], sOrig.split("\n")[1:])
         eq(ES.log.count['UnretriableDelivery'], {})
+        eq(ES.log.lastSave, tm+3600*24)
+        eq(ES.log.accumulatedTime, 0)
+        self.assert_((ES.log.nextRotation - (tm+3600*25)) < 3600)
         ES.configureLog({'Server':
                          {'LogStats' : 1,
                           'Homedir' : homedir,
                  'StatsInterval' : mixminion.Config._parseInterval("1 hour")}})
         eq(ES.log.count['UnretriableDelivery'], {})
+        self.assert_(floatEq(ES.log.lastSave, time.time()))
+
         # Test time configured properly.
-        # XXXX
+        # 1) Rotation interval is a multiple of hours.
+        pm = previousMidnight(tm)
+        ES.log.rotateInterval = 2*60*60
+        # 1A) Accumulated time << interval.  We rotated at 23:00.  We were
+        #   only up for a few minutes (10), and have accumulated no more time
+        #   since the last rotation.  We just reloaded; now it's 2:00.  We
+        #   should next rotate at 4 -- since it will take us until 3:20 to
+        #   be up for 75% of an interval, and we round up.
+        ES.log.lastRotation = pm - 60*60
+        ES.log.accumulatedTime = 10*60
+        ES.log.lastSave = pm+7200
+        ES.log._setNextRotation(now=pm+7200)
+        eq(ES.log.getNextRotation(), pm+4*60*60)
+        # But suppose that it's 2:45. We should next rotate at 5 (rounding up).
+        ES.log.lastSave = pm+7200+45*60
+        ES.log._setNextRotation(now=pm+7200+45*60)
+        eq(ES.log.getNextRotation(), pm+5*60*60)
+
+        # 1B) Accumulated time ~~ interval
+        # Now suppose the same situation as above, but we've been up 100
+        #  minutes total.  We rotate *now*.
+        ES.log.accumulatedTime = 100*60
+        ES.log.lastSave = pm+7200
+        ES.log._setNextRotation(now=pm+7200)
+        eq(ES.log.getNextRotation(), pm+7200)
+
+        # 1C) Suppose all went well, and it's 23:30: Rotate at 1.
+        ES.log.accumulatedTime = 1800
+        ES.log.lastSave = pm-1800
+        ES.log._setNextRotation(now=pm-1800)
+        eq(ES.log.getNextRotation(), pm+3600)
         
-        # Test no rotation if not enough accumulated time
-        #XXXX THIS NEXT PART DOESN'T WORK....
-        ES.log.lastSave = time.time()-1000
-        ES.log._setNextRotation()
-        #print ES.log.nextRotation, time.time()+4000
-        r = ES.log._rotate
-        try:
-            ES.log._rotate = self.fail
-            ES.log.save(now=time.time()+9000)
-        finally:
-            ES.log._rotate = r
+        # 2) Rotation interval is not a multiple of hours: We don't round
+        # 2A) Accumulated time << interval
+        ES.log.rotateInterval = 40*60
+        ES.log.lastSave = pm+7200
+        ES.log.accumulatedTime = 10*60
+        ES.log._setNextRotation(now=pm+7200)
+        eq(ES.log.getNextRotation(), pm+7200+20*60)
+        # 2B) Accumulated time ~~ interval
+        ES.log.accumulatedTime = 35*60
+        ES.log.lastSave = pm+7200
+        ES.log._setNextRotation(now=pm+7200)
+        eq(ES.log.getNextRotation(), pm+7200)
 
 #----------------------------------------------------------------------
 # Modules and ModuleManager
@@ -5465,9 +5565,7 @@
     tc = loader.loadTestsFromTestCase
 
     if 0:
-        suite.addTest(tc(ServerInfoTests))
-        #suite.addTest(tc(EventStatsTests))
-        #suite.addTest(tc(MMTPTests))
+        suite.addTest(tc(MMTPTests))
         #suite.addTest(tc(MiscTests))
         return suite