[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[minion-cvs] Logging, doccing, testing, debugging. Towards an 0.0.1...



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

Modified Files:
	BuildMessage.py ClientMain.py Common.py Config.py HashLog.py 
	MMTPClient.py MMTPServer.py Modules.py Packet.py Queue.py 
	ServerInfo.py ServerMain.py test.py 
Log Message:
Logging, doccing, testing, debugging.  Towards an 0.0.1 release...

BuildMessage: Document everything; rename internal methods; add more logging.

ClientMain:  Add more logging; check for empty paths.

Common: clean up a comment

Config: Mark a FFFF for 0.0.1.

HashLog: Add more logging; use mode 0600 for journals.

MMTPClient: Add more logging

MMTPServer: Add more logging; note fd's and addresses with log messages.

Modules: Add more logging; add note on configuration; mark XXXX's for 0.0.1

Packet: Mark XXXX's for 0.0.1

Queue: Increse timeouts; mark XXXX's for 001.

ServerInfo: Improve log messages; mark XXXX for 001.

ServerMain: More log messages; move module manager configuration into 
   server object

test: Use new BuildMessage names; Add unit tests for ClientMain.


Index: BuildMessage.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/BuildMessage.py,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -d -r1.18 -r1.19
--- BuildMessage.py	2 Dec 2002 03:30:07 -0000	1.18
+++ BuildMessage.py	7 Dec 2002 04:03:35 -0000	1.19
@@ -3,7 +3,8 @@
 
 """mixminion.BuildMessage
 
-   Code to construct messages and reply blocks."""
+   Code to construct messages and reply blocks, and to decode received 
+   message payloads."""
 
 import zlib
 import operator
@@ -12,59 +13,100 @@
 import mixminion.Crypto as Crypto
 import mixminion.Modules as Modules
 
-__all__ = [ 'Address',
-           'buildForwardMessage', 'buildEncryptedMessage', 'buildReplyMessage',
-           'buildStatelessReplyBlock', 'buildReplyBlock', 'decodePayload',
-	   'decodeForwardPayload', 'decodeEncryptedForwardPayload',
-	   'decodeReplyPayload', 'decodeStatelessReplyPayload' ]
+__all__ = ['buildForwardMessage', 'buildEncryptedMessage', 'buildReplyMessage',
+           'buildStatelessReplyBlock', 'buildReplyBlock', 'decodePayload' ]
 
 def buildForwardMessage(payload, exitType, exitInfo, path1, path2,
 			paddingPRNG=None):
     """Construct a forward message.
             payload: The payload to deliver.  Must compress to under 28K-22b.
-            exitType: The routing type for the final node
+                  If it does not, MixError is raised.
+            exitType: The routing type for the final node. (2 bytes, >=0x100)
             exitInfo: The routing info for the final node, not including tag.
             path1: Sequence of ServerInfo objects for the first leg of the path
             path2: Sequence of ServerInfo objects for the 2nd leg of the path
-	    paddingPRNG
+	    paddingPRNG: random number generator used to generate padding.
+	          If None, a new PRNG is initialized.
 
-        Note: If either path is empty, the message is vulnerable to tagging
-         attacks! (FFFF we should check this.)
+        Neither path1 nor path2 may be empty.
     """
     if paddingPRNG is None: paddingPRNG = Crypto.AESCounterPRNG()
+    assert path1 and path2
+
+    getLog().debug("Encoding forward message for %s-byte payload",len(payload))
+    getLog().debug("  Using path %s/%s",
+		   [s.getNickname() for s in path1],
+		   [s.getNickname() for s in path2])
+    getLog().debug("  Delivering to %04x:%r", exitType, exitInfo)
 
+    # Compress, pad, and checksum the payload.
     payload = _encodePayload(payload, 0, paddingPRNG)
+    
+    # Choose a random decoding tag.
     tag = _getRandomTag(paddingPRNG)
     exitInfo = tag + exitInfo
     return _buildMessage(payload, exitType, exitInfo, path1, path2,paddingPRNG)
 
 def buildEncryptedForwardMessage(payload, exitType, exitInfo, path1, path2,
 				 key, paddingPRNG=None, secretRNG=None):
-    """DOCDOC
+    """Construct a forward message encrypted with the public key of a
+       given user.
+            payload: The payload to deliver.  Must compress to under 28K-60b.
+                  If it does not, MixError is raised.
+            exitType: The routing type for the final node. (2 bytes, >=0x100)
+            exitInfo: The routing info for the final node, not including tag.
+            path1: Sequence of ServerInfo objects for the first leg of the path
+            path2: Sequence of ServerInfo objects for the 2nd leg of the path
+	    key: Public key of this message's recipient.
+	    paddingPRNG: random number generator used to generate padding.
+	          If None, a new PRNG is initialized.
     """
     if paddingPRNG is None: paddingPRNG = Crypto.AESCounterPRNG()
     if secretRNG is None: secretRNG = paddingPRNG
 
+    getLog().debug("Encoding encrypted forward message for %s-byte payload",
+		   len(payload))
+    getLog().debug("  Using path %s/%s",
+		   [s.getNickname() for s in path1],
+		   [s.getNickname() for s in path2])
+    getLog().debug("  Delivering to %04x:%r", exitType, exitInfo)
+
+    # Compress, pad, and checksum the payload.
+    # (For encrypted-forward messages, we have overhead for OAEP padding
+    #   and the session  key, but we save 20 bytes by spilling into the tag.)
     payload = _encodePayload(payload, ENC_FWD_OVERHEAD, paddingPRNG)
 
+    # Generate the session key, and prepend it to the payload.
     sessionKey = secretRNG.getBytes(SECRET_LEN)
     payload = sessionKey+payload
+
+    # We'll encrypt the first part of the new payload with RSA, and the
+    # second half with Lioness, based on the session key.
     rsaDataLen = key.get_modulus_bytes()-OAEP_OVERHEAD
     rsaPart = payload[:rsaDataLen]
     lionessPart = payload[rsaDataLen:]
-    # DOCDOC
+
+    # RSA encryption: To avoid leaking information about our RSA modulus,
+    # we keep trying to encrypt until the MSBit of our encrypted value is 
+    # zero.
     while 1:
 	encrypted = Crypto.pk_encrypt(rsaPart, key)
 	if not (ord(encrypted[0]) & 0x80):
 	    break
-    # DOCDOC doc mode 'End-to-end encrypt'
+    # Lioness encryption.
+    # DOCDOC doc mode 'End-to-end encrypt' XXXX001
     k = Crypto.Keyset(sessionKey).getLionessKeys("End-to-end encrypt")
     lionessPart = Crypto.lioness_encrypt(lionessPart, k)
+
+    # Now we re-divide the payload into the part that goes into the tag, and
+    # the 28K of the payload proper...
     payload = encrypted + lionessPart
     tag = payload[:TAG_LEN]
     payload = payload[TAG_LEN:]
     exitInfo = tag + exitInfo
     assert len(payload) == 28*1024
+
+    # And now, we can finally build the message.
     return _buildMessage(payload, exitType, exitInfo, path1, path2,paddingPRNG)
 
 def buildReplyMessage(payload, path1, replyBlock, paddingPRNG=None):
@@ -73,12 +115,18 @@
     """
     if paddingPRNG is None: paddingPRNG = Crypto.AESCounterPRNG()
 
+    getLog().debug("Encoding reply message for %s-byte payload",
+		   len(payload))
+    getLog().debug("  Using path %s/??",[s.getNickname() for s in path1])
+
+    # Compress, pad, and checksum the payload.
     payload = _encodePayload(payload, 0, paddingPRNG)
 
-    # DOCDOC document this mode
+    # Encrypt the payload so that it won't appear as plaintext to the
+    #  crossover note.  (We use 'decrypt' so that the message recipient can
+    #  simply use 'encrypt' to reverse _all_ the steps of the reply path.)
     k = Crypto.Keyset(replyBlock.encryptionKey).getLionessKeys(
 	                 Crypto.PAYLOAD_ENCRYPT_MODE)
-    # DOCDOC Document why this is decrypt
     payload = Crypto.lioness_decrypt(payload, k)
 
     return _buildMessage(payload, None, None,
@@ -99,19 +147,26 @@
                  will be used to encrypt the message in reverse order.
               tag: If provided, a 159-bit tag.  If not provided, a new one
                  is generated.
+
+        (This will go away when we disable 'stateful' (non-state-carrying)
+	 reply blocks.)
        """
     if secretPRNG is None:
         secretPRNG = Crypto.AESCounterPRNG()
 
+    getLog().debug("Building reply block for path %s",
+		   [s.getNickname() for s in path])
+    getLog().debug("  Delivering to %04x:%r", exitType, exitInfo)    
+
     # The message is encrypted first by the end-to-end key, then by
     # each of the path keys in order. We need to reverse these steps, so we
     # generate the path keys back-to-front, followed by the end-to-end key.
     secrets = [ secretPRNG.getBytes(SECRET_LEN) for _ in range(len(path)+1) ]
-
     headerSecrets = secrets[:-1]
     headerSecrets.reverse()
     sharedKey = secrets[-1]
 
+    # (This will go away when we deprecate 'stateful' reply blocks
     if tag is None:
 	tag = _getRandomTag(secretPRNG)
 
@@ -125,31 +180,25 @@
 # Maybe we shouldn't even allow this to be called with userKey==None.
 def buildStatelessReplyBlock(path, exitType, exitInfo, userKey,
 			     expiryTime=0, secretRNG=None):
-    """DOCDOC XXXX DOC IS NOW WRONG HERE
-       (exitInfo doesn't include tag)
-
-       Construct a 'stateless' reply block that does not require the
+    """Construct a 'stateless' reply block that does not require the
        reply-message recipient to remember a list of secrets.
        Instead, all secrets are generated from an AES counter-mode
        stream, and the seed for the stream is stored in the 'tag'
-       field of the final block's routing info.
-
-       If the user provides a 'userkey', that key is used to encrypt
-       the seed before storing it in the tag field.  Otherwise, the
-       seed is stored in the clear.  USERS SHOULD ALWAYS SET 'userkey'
-       IF THE EXIT INFORMATION WILL BE TRAVELING OVER THE NETWORK, OR
-       IF THEY DO NOT PERSONALLY CONTROL THE EXIT NODE.  Otherwise,
-       their anonymity can be completely broken.
+       field of the final block's routing info.   (See the spec for more 
+       info).
 
-                  path: a list of ServerInfo objects
-                  user: the user's username/email address
-                  userKey: an AES key to encrypt the seed, or None.
-                  email: If true, delivers via SMTP; else delivers via MBOX
+               path: a list of ServerInfo objects
+	       exitType,exitInfo: The address to deliver the final message.
+               userKey: a string used to encrypt the seed.
        """
-
-    # ???? Out of sync with the spec. 
     if secretRNG is None: secretRNG = Crypto.AESCounterPRNG()
 
+    # We need to pick the seed to generate our keys.  To make the decoding
+    # step a little faster, we find a seed such that H(seed|userKey|"Validate")
+    # ends with 0.  This way, we can detect whether we really have a reply
+    # message with 99.6% probability.  (Otherwise, we'd need to repeatedly
+    # lioness-decrypt the payload in order to see whether the message was 
+    # a reply.)
     while 1:
 	seed = _getRandomTag(secretRNG)
 	if Crypto.sha1(seed+userKey+"Validate")[-1] == '\x00':
@@ -162,74 +211,115 @@
 #----------------------------------------------------------------------
 # MESSAGE DECODING
 
-def decodePayload(payload, tag, key=None, storedKeys=None, userKey=None):
-    """ DOCDOC 
-        Contract: return payload on success; raise MixError on certain failure,
-          return None if neither.
+def decodePayload(payload, tag, key=None, 
+		  storedKeys=None, #XXXX001 disable storedKeys
+		  userKey=None):
+    """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.
+         
+       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.
     """
+    # FFFF Take a list of keys?
+    # FFFF Allow callbacks?
+
     if len(payload) != PAYLOAD_LEN or len(tag) != TAG_LEN:
 	raise MixError("Wrong payload or tag length")
 
+    # If the payload already contains a valid checksum, it's a forward
+    # message.
     if _checkPayload(payload):
-	return decodeForwardPayload(payload)
+	return _decodeForwardPayload(payload)
 
+    # If we have a list of keys associated with the tag, it's a reply message
+    # using those keys.
+    #XXXX001 'Non-state-carrying' reply blocks are supposed to be disabled
     if storedKeys is not None:
 	secrets = storedKeys.get(tag)
-	if secrets is not None:
-	    del storedKeys[tag]
-	    return decodeReplyPayload(payload, secrets)
+ 	if secrets is not None:
+ 	    del storedKeys[tag]
+ 	    return _decodeReplyPayload(payload, secrets)
 
+    # If H(tag|userKey|"Validate") ends with 0, then the message _might_
+    # be a reply message using H(tag|userKey|"Generate") as the seed for
+    # its master secrets.  (There's a 1-in-256 chance that it isn't.)
     if userKey is not None:
 	if Crypto.sha1(tag+userKey+"Validate")[-1] == '\x00':
 	    try:
-		return decodeStatelessReplyPayload(payload, tag, userKey)
+		return _decodeStatelessReplyPayload(payload, tag, userKey)
 	    except MixError, _:
 		pass
 
+    # If we have an RSA key, and none of the above steps get us a good
+    # payload, then we may as well try to decrypt the start of tag+key with
+    # our RSA key.
     if key is not None:
-	p = decodeEncryptedForwardPayload(payload, tag, key)
+	p = _decodeEncryptedForwardPayload(payload, tag, key)
 	if p is not None:
 	    return p
 
     return None
 
-def decodeForwardPayload(payload):
-    "DOCDOC"
-    return decodePayloadImpl(payload)
+def _decodeForwardPayload(payload):
+    """Helper function: decode a non-encrypted forward payload. Return values
+       are the same as decodePayload."""
+    return _decodePayloadImpl(payload)
 
-def decodeEncryptedForwardPayload(payload, tag, key):
-    "DOCDOC"
+def _decodeEncryptedForwardPayload(payload, tag, key):
+    """Helper function: decode an encrypted forward payload.  Return values
+       are the same as decodePayload.
+             payload: the payload to decode
+	     tag: the decoding tag
+	     key: the RSA key of the payload's recipient."""
     assert len(tag) == TAG_LEN
     assert len(payload) == PAYLOAD_LEN
+
+    # Given an N-byte RSA key, the first N bytes of tag+payload will be
+    # encrypted with RSA, and the rest with a lioness key given in the
+    # first N.  Try decrypting...
     msg = tag+payload
     try:
 	rsaPart = Crypto.pk_decrypt(msg[:key.get_modulus_bytes()], key)
     except Crypto.CryptoError, _:
 	return None
     rest = msg[key.get_modulus_bytes():]
-    # XXXX magic string
-    k = Crypto.Keyset(rsaPart[:SECRET_LEN]).getLionessKeys("End-to-end encrypt")
+    # XXXX001 magic string
+    k =Crypto.Keyset(rsaPart[:SECRET_LEN]).getLionessKeys("End-to-end encrypt")
     rest = rsaPart[SECRET_LEN:] + Crypto.lioness_decrypt(rest, k)
-    return decodePayloadImpl(rest)
+    
+    # ... and then, check the checksum and continue.
+    return _decodePayloadImpl(rest)
 
-def decodeReplyPayload(payload, secrets, check=0):
-    "DOCDOC"
+def _decodeReplyPayload(payload, secrets, check=0):
+    """Helper function: decode a reply payload, given a known list of packet
+         master secrets. If 'check' is true, then 'secerets' may be overlong.
+         Return values are the same as decodePayload.
+      [secrets must be in _reverse_ order]
+    """ 
+    # Reverse the 'decrypt' operations of the reply mixes, and the initial
+    # 'decrypt' of the originating user...
     for sec in secrets:
 	k = Crypto.Keyset(sec).getLionessKeys(Crypto.PAYLOAD_ENCRYPT_MODE)
-	# DOCDOC document why this is encrypt
 	payload = Crypto.lioness_encrypt(payload, k)
 	if check and _checkPayload(payload):
 	    break
 
-    return decodePayloadImpl(payload)
+    # ... and then, check the checksum and continue.
+    return _decodePayloadImpl(payload)
 
-def decodeStatelessReplyPayload(payload, tag, userKey):
-    "DOCDOC"
+def _decodeStatelessReplyPayload(payload, tag, userKey):
+    """Decode a (state-carrying) reply payload."""
+    # Reconstruct the secrets we used to generate the reply block (possibly
+    # too many)
     seed = Crypto.sha1(tag+userKey+"Generate")[:16]
     prng = Crypto.AESCounterPRNG(seed)
     secrets = [ prng.getBytes(SECRET_LEN) for _ in xrange(17) ]
 
-    return decodeReplyPayload(payload, secrets, check=1)
+    return _decodeReplyPayload(payload, secrets, check=1)
 
 #----------------------------------------------------------------------
 def _buildMessage(payload, exitType, exitInfo,
@@ -238,17 +328,17 @@
 
     The following fields must be set:
        payload: the intended exit payload.  Must be 28K.
-       (exitType, exitInfo): the routing type and info for the final node.
-              (Ignored for reply messages)
-       path1: a sequence of ServerInfo objects, one for each of the nodes
-          on the first leg of the path.
-
-    The following fields must be set for a forward message:
-       path2: EITHER
-             a sequence of ServerInfo objects, one for each of the nodes
+       (exitType, exitInfo): the routing type and info for the final
+              node.  (Ignored for reply messages; 'exitInfo' should
+              include the 20-byte decoding tag.)
+       path1: a sequence of ServerInfo objects, one for each node on
+          the first leg of the path.
+       path2: 
+        EITHER
+             a sequence of ServerInfo objects, one for each node
              on the second leg of the path.
          OR
-             a replyBlock object.
+             a ReplyBlock object.
 
     The following fields are optional:
        paddingPRNG: A pseudo-random number generator used to pad the headers.
@@ -314,7 +404,8 @@
            secrets: A list of 16-byte strings to use as master-secrets for
                each of the subheaders.
            exitType: The routing type for the last node in the header
-           exitInfo: The routing info for the last node in the header
+           exitInfo: The routing info for the last node in the header. 
+               (Must include 20-byte decoding tag.)
            paddingPRNG: A pseudo-random number generator to generate padding
     """
     assert len(path) == len(secrets)
@@ -392,7 +483,7 @@
     """Helper method: Builds a message, given both headers, all known
        secrets, and the padded payload.
 
-       If using a reply block, secrets2 should be null.
+       If using a reply block for header2, secrets2 should be null.
     """
     assert len(payload) == PAYLOAD_LEN
     assert len(header1) == len(header2) == HEADER_LEN
@@ -445,15 +536,19 @@
        BUG: This should eventually support K-of-N.
     """
     assert overhead in (0, ENC_FWD_OVERHEAD)
-    payload = compressData(payload)
 
+    # Compress the data, and figure out how much padding we'll need.
+    payload = compressData(payload)
     length = len(payload)
     paddingLen = PAYLOAD_LEN - SINGLETON_PAYLOAD_OVERHEAD - overhead - length
+
+    # If the compressed payload doesn't fit in 28K, then we need to bail out.
     if paddingLen < 0:
 	raise MixError("Payload too long for singleton message")
 
+    # Otherwise, we pad the payload, and construct a new SingletonPayload,
+    # including this payload's size and checksum.
     payload += paddingPRNG.getBytes(paddingLen)
-
     return SingletonPayload(length, Crypto.sha1(payload), payload).pack()
 
 def _getRandomTag(rng):
@@ -461,14 +556,20 @@
     b = ord(rng.getBytes(1)) & 0x7f
     return chr(b) + rng.getBytes(TAG_LEN-1)
 
-def decodePayloadImpl(payload):
+def _decodePayloadImpl(payload):
+    """Helper: try to decode an encoded payload: checks only encoding, 
+       not encryption."""
+    # Is the hash ok?
     if not _checkPayload(payload):
 	raise MixError("Hash doesn't match")
+
+    # Parse the payload into its size, checksum, and body.
     payload = parsePayload(payload)
 
     if not payload.isSingleton():
 	raise MixError("Message fragments not yet supported")
 
+    # Uncompress the body.
     return uncompressData(payload.getContents())
 
 def _checkPayload(payload):
@@ -478,6 +579,8 @@
 #----------------------------------------------------------------------
 # COMPRESSION FOR PAYLOADS
 
+# Global: contains 0 if we haven't validated zlib; 1 if we have, and 0.5
+#    if we're in the middle of validation.
 _ZLIB_LIBRARY_OK = 0
 
 def compressData(payload):
@@ -485,7 +588,10 @@
        as specified in the remailer spec and in RFC1951."""
     if not _ZLIB_LIBRARY_OK:
 	_validateZlib()
-    # Don't change any of these options; they're all mandated.
+
+    # Don't change any of these options; if different Mixminion clients
+    # compress their data differently, an adversary could distinguish
+    # messages generated by them.
     zobj = zlib.compressobj(zlib.Z_BEST_COMPRESSION, zlib.DEFLATED,
 			    zlib.MAX_WBITS, zlib.DEF_MEM_LEVEL,
 			    zlib.Z_DEFAULT_STRATEGY)
@@ -497,7 +603,6 @@
     # these are irrelevant, as are the 4 bytes of adler-32 checksum at
     # the end.  Still, we can afford 6 bytes per payload, and
     # reconstructing the checksum to keep zlib happy is a bit of a pain.
-    # DOCDOC doc manditory '\x78\xDA' beginning in spec.
     assert s[0] == '\x78' # deflate, 32K window
     assert s[1] == '\xda' # no dict, max compression
     return s

Index: ClientMain.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ClientMain.py,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
--- ClientMain.py	2 Dec 2002 10:13:48 -0000	1.9
+++ ClientMain.py	7 Dec 2002 04:03:35 -0000	1.10
@@ -34,7 +34,7 @@
 from mixminion.Common import getLog, floorDiv, createPrivateDir, MixError, \
      MixFatalError
 import mixminion.Crypto
-from mixminion.BuildMessage import buildForwardMessage
+import mixminion.BuildMessage
 import mixminion.MMTPClient
 import mixminion.Modules
 from mixminion.ServerInfo import ServerInfo
@@ -87,6 +87,7 @@
 		    "Ignoring descriptor %s with duplicate prefix %s",
 		    p, f)
 		continue
+	    getLog().info("Loaded server %s from %s", nickname, f)
 	    self.byNickname[nickname] = info
 	    self.byFilename[f] = info
 
@@ -183,13 +184,17 @@
 	self.sendMessages([message], firstHop)
 
     def generateForwardMessage(self, address, payload, path1, path2):
+	if not path1:
+	    raise MixError("No servers in first leg of path")
+	if not path2:
+	    raise MixError("No servers in second leg of path")
+
 	servers1 = self.keystore.getPath(path1)
 	servers2 = self.keystore.getPath(path2)
 
 	routingType, routingInfo, lastHop = address.getRouting()
 	if lastHop is None:
             lastServer = servers2[-1]
-            print path2[-1], routingType
 	    # FFFF This is only a temporary solution.  It needs to get
 	    # FFFF rethought, or refactored into ServerInfo, or something.
 	    if routingType == SMTP_TYPE:
@@ -202,9 +207,8 @@
 		    raise MixError("Last hop doesn't support MBOX")
 	else:
 	    servers2.append(self.keystore.getServerInfo(lastHop))
-	msg = buildForwardMessage(payload,
-				  routingType, routingInfo,
-				  servers1, servers2)
+	msg = mixminion.BuildMessage.buildForwardMessage(
+	    payload, routingType, routingInfo, servers1, servers2)
 	return msg, servers1[0]
 
     def sendMessages(self, msgList, server):

Index: Common.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Common.py,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -d -r1.28 -r1.29
--- Common.py	3 Dec 2002 00:39:51 -0000	1.28
+++ Common.py	7 Dec 2002 04:03:35 -0000	1.29
@@ -127,8 +127,6 @@
 #----------------------------------------------------------------------
 # Secure filesystem operations.
 #
-
-
 _SHRED_CMD = "---"
 _SHRED_OPTS = None
     
@@ -164,7 +162,7 @@
     global _NILSTR
     if not _BLKSIZE:
 	#???? this assumes that all filesystems we are using have the same
-	#??? block size.
+	#???? block size.
 	if hasattr(os, 'statvfs'):
 	    _BLKSIZE = os.statvfs(f)[statvfs.F_BSIZE]
 	else:

Index: Config.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Config.py,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -d -r1.20 -r1.21
--- Config.py	2 Dec 2002 03:30:07 -0000	1.20
+++ Config.py	7 Dec 2002 04:03:35 -0000	1.21
@@ -686,6 +686,7 @@
 	# FFFF         timeout options
 	# FFFF         listen timeout??
 	# FFFF         Retry options
+	# FFFF         pool options
         }
 
 class ServerConfig(_ConfigFile):
@@ -755,6 +756,6 @@
 	return self.moduleManager
 
 def _validateHostSection(sec):
-    # FFFF
+    # FFFF001
     pass
 

Index: HashLog.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/HashLog.py,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -d -r1.13 -r1.14
--- HashLog.py	3 Dec 2002 00:40:26 -0000	1.13
+++ HashLog.py	7 Dec 2002 04:03:35 -0000	1.14
@@ -56,6 +56,7 @@
         parent = os.path.split(filename)[0]
 	createPrivateDir(parent)
         self.log = anydbm.open(filename, 'c')
+	getLog().debug("Opening database %s for packet digests", filename)
         if isinstance(self.log, dumbdbm._Database):
             getLog().warn("Warning: logging packet digests to a flat file.")
         try:
@@ -74,7 +75,7 @@
 	    f.close()
 
 	self.journalFile = os.open(self.journalFileName, 
-		    _JOURNAL_OPEN_MODE|os.O_APPEND, 0700)
+		    _JOURNAL_OPEN_MODE|os.O_APPEND, 0600)
 
     def seenHash(self, hash):
         """Return true iff 'hash' has been logged before."""
@@ -101,7 +102,7 @@
             self.log.sync()
 	os.close(self.journalFile)
 	self.journalFile = os.open(self.journalFileName,
-		   _JOURNAL_OPEN_MODE|os.O_TRUNC, 0700)
+		   _JOURNAL_OPEN_MODE|os.O_TRUNC, 0600)
 	self.journal = {}
 
     def close(self):

Index: MMTPClient.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPClient.py,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -d -r1.10 -r1.11
--- MMTPClient.py	2 Dec 2002 03:30:07 -0000	1.10
+++ MMTPClient.py	7 Dec 2002 04:03:35 -0000	1.11
@@ -18,7 +18,7 @@
 import socket
 import mixminion._minionlib as _ml
 from mixminion.Crypto import sha1
-from mixminion.Common import MixProtocolError
+from mixminion.Common import MixProtocolError, getLog
 
 class BlockingClientConnection:
     """A BlockingClientConnection represents a MMTP connection to a single
@@ -35,11 +35,15 @@
         """Negotiate the handshake and protocol."""
         self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         self.sock.setblocking(1)
+	getLog().debug("Connecting to %s:%s", self.targetIP, self.targetPort) 
         self.sock.connect((self.targetIP,self.targetPort))
-        
+        getLog().debug("Handshaking with %s:%s",self.targetIP, self.targetPort)
+
         self.tls = self.context.sock(self.sock.fileno())
         # FFFF session resumption
         self.tls.connect()
+	getLog().debug("Connected.")
+
         peer_pk = self.tls.get_peer_cert_pk()
         keyID = sha1(peer_pk.encode_key(public=1))
         if self.targetKeyID is not None and (keyID != self.targetKeyID):
@@ -51,26 +55,34 @@
         # For now, we only support 1.0, but we call it 0.1 so we can
 	# change our mind between now and a release candidate, and so we
 	# can obsolete betas come release time.
+	getLog().debug("Negotiatiating MMTP protocol")
         self.tls.write("MMTP 0.1\r\n")
         inp = self.tls.read(len("MMTP 0.1\r\n"))
         if inp != "MMTP 0.1\r\n":
             raise MixProtocolError("Protocol negotiation failed")
+	getLog().debug("MMTP protocol negotated: version 0.1")
         
     def sendPacket(self, packet):
         """Send a single packet to a server."""
         assert len(packet) == 1<<15
+	getLog().debug("Sending packet")
         self.tls.write("SEND\r\n")
         self.tls.write(packet)
         self.tls.write(sha1(packet+"SEND"))
+	getLog().debug("Packet sent; waiting for ACK")
         
         inp = self.tls.read(len("RECEIVED\r\n")+20)
         if inp != "RECEIVED\r\n"+sha1(packet+"RECEIVED"):
             raise MixProtocolError("Bad ACK received")
+	getLog().debug("ACK received; packet successfully delivered")
 
     def shutdown(self):
         """Close this connection."""
+	getLog().debug("Shutting down connection to %s:%s",
+		       self.targetIP, self.targetPort)
         self.tls.shutdown()
         self.sock.close()
+	getLog().debug("Connection closed")
 
 def sendMessages(targetIP, targetPort, targetKeyID, packetList):
     """Sends a list of messages to a server."""

Index: MMTPServer.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPServer.py,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -d -r1.18 -r1.19
--- MMTPServer.py	2 Dec 2002 03:21:12 -0000	1.18
+++ MMTPServer.py	7 Dec 2002 04:03:35 -0000	1.19
@@ -79,13 +79,13 @@
                 raise e
 
         for fd in readfds:
-            trace("Select got a read on "+str(fd))
+            trace("Select got a read on fd %s",fd)
             self.readers[fd].handleRead()
         for fd in writefds:
-            trace("Select got a write on"+str(fd))
+            trace("Select got a write on fd %s", fd)
             self.writers[fd].handleWrite()
         for fd in exfds:
-            trace("Select got an exception on"+str(fd))
+            trace("Select got an exception on fd %s", fd)
             if self.readers.has_key(fd): del self.readers[fd]
             if self.writers.has_key(fd): del self.writers[fd]
 
@@ -162,7 +162,7 @@
         self.sock.bind((self.ip, self.port))
         self.sock.listen(backlog)
         self.connectionFactory = connectionFactory
-        info("Listening at %s on port %s", ip, port)
+        info("Listening at %s on port %s (fd %s)", ip, port,self.sock.fileno())
 
     def register(self, server):
         server.registerReader(self)
@@ -170,12 +170,12 @@
 
     def handleRead(self):
         con, addr = self.sock.accept()
-        debug("Accepted connection from %s", addr)
+        debug("Accepted connection from %s (fd %s)", addr, con.fileno())
         rw = self.connectionFactory(con)
         rw.register(self.server)
 
     def shutdown(self):
-        debug("Closing listener connection")
+        debug("Closing listener connection (fd %s)", self.sock.fileno())
         self.server.unregister(self)
         del self.server
         self.sock.close()
@@ -202,6 +202,9 @@
     """
     # Fields:
     #    lastActivity: the last time when we had a read or a write.
+    #    address: Human readable IP address of our peer.  For debugging.
+    #    fd: fileno for the underlying socket __con.
+    #
     #    __con: an underlying TLS object
     #    __state: a callback to use whenever we get a read or a write. May
     #           throw _ml.TLSWantRead or _ml.TLSWantWrite.
@@ -222,7 +225,9 @@
                          otherwise, we start with a client-side negotatiation.
         """
         self.__sock = sock
+	self.address = "%s:%s" % sock.getpeername()
         self.__con = tls
+	self.fd = self.__con.fileno()
 
         if serverMode:
             self.__state = self.__acceptFn
@@ -290,7 +295,7 @@
 	# else we can deadlock on a connection from ourself to
 	# ourself.
 	if self.__con.shutdown() == 1: #may throw want*
-	    trace("Got a 1 on shutdown")
+	    trace("Got a 1 on shutdown (fd %s)", self.fd)
 	    self.__server.unregister(self)
 	    self.__state = None
 	    self.__sock.close()
@@ -299,10 +304,10 @@
 
 	# If we don't get any response on shutdown, stop blocking; the other
 	# side may be hostile, confused, or deadlocking.
-	trace("Got a 0 on shutdown")
+	trace("Got a 0 on shutdown (fd %s)", self.fd)
 	# ???? Is 'wantread' always correct?
 	# ???? Rather than waiting for a read, should we use a timer or 
-	# ????    something?
+	# ????       something?
 	raise _ml.TLSWantRead()
 
     def __readFn(self):
@@ -310,12 +315,12 @@
         while 1:
             r = self.__con.read(1024) #may throw want*
             if r == 0:
-                trace("read returned 0.")
+                trace("read returned 0 (fd %s)", self.fd)
                 self.shutdown(err=0)
                 return
             else:
                 assert isinstance(r, StringType)
-                trace("read got %s bytes" % len(r))
+                trace("read got %s bytes (fd %s)", len(r), self.fd)
                 self.__inbuf.append(r)
                 self.__inbuflen += len(r)
                 if not self.__con.pending():
@@ -325,18 +330,19 @@
             self.__inbuf = ["".join(self.__inbuf)]
 
         if self.__expectReadLen and self.__inbuflen > self.__expectReadLen:
-            warn("Protocol violation: too much data. Closing connection.")
+            warn("Protocol violation: too much data. Closing connection to %s",
+		 self.address)
             self.shutdown(err=1, retriable=0)
             return
          
         if self.__terminator and stringContains(self.__inbuf[0], 
 						self.__terminator):
-            trace("read found terminator")
+            trace("read found terminator (fd %s)", self.fd)
             self.__server.unregister(self)
             self.finished()
 
         if self.__expectReadLen and (self.__inbuflen == self.__expectReadLen):
-            trace("read got enough.")
+            trace("read got enough (fd %s)", self.fd)
             self.__server.unregister(self)
             self.finished()
 
@@ -378,16 +384,18 @@
         except _ml.TLSWantRead:
             self.__server.registerReader(self)
         except _ml.TLSClosed:
-            warn("Unexpectedly closed connection")
+            warn("Unexpectedly closed connection to %s", self.address)
 	    self.handleFail(retriable=1)
             self.__sock.close()
             self.__server.unregister(self) 
         except _ml.TLSError:
             if self.__state != self.__shutdownFn:
-                warn("Unexpected error: closing connection.")
+                warn("Unexpected error: closing connection to %s", 
+		     self.address)
                 self.shutdown(err=1, retriable=1)
             else:
-                warn("Error while shutting down: closing connection.")
+                warn("Error while shutting down: closing connection to %s",
+		     self.address)
                 self.__server.unregister(self)
         else:
             # We are in no state at all.
@@ -408,7 +416,7 @@
         self.__state = self.__shutdownFn
         
     def fileno(self):
-        return self.__con.fileno()
+        return self.fd
 
     def getInput(self):
         """Returns the current contents of the input buffer."""
@@ -451,19 +459,20 @@
         """Called once we're done reading the protocol string.  Either
            rejects, or sends our response.
         """
-        trace("done w/ client sendproto")
+        trace("done w/ client sendproto (fd %s)", self.fd)
         inp = self.getInput()
         m = PROTOCOL_RE.match(inp)
 
         if not m:
-            warn("Bad protocol list.  Closing connection.")
+            warn("Bad protocol list.  Closing connection to %s", self.address)
             self.shutdown(err=1)
         protocols = m.group(1).split(",")
         if "0.1" not in protocols:
-            warn("Unsupported protocol list.  Closing connection.")
+            warn("Unsupported protocol list.  Closing connection to %s",
+		 self.address)
             self.shutdown(err=1); return
         else:
-            trace("proto ok.")
+            trace("protocol ok (fd %s)", self.fd)
             self.finished = self.__sentProtocol
             self.beginWrite(PROTOCOL_STRING)
 
@@ -471,7 +480,7 @@
         """Called once we're done sending our protocol response.  Begins
            reading a packet from the line.
         """
-        trace("done w/ server sendproto")
+        trace("done w/ server sendproto (fd %s)", self.fd)
         self.finished = self.__receivedMessage
         self.expectRead(SEND_RECORD_LEN)
 
@@ -489,15 +498,18 @@
             expectedDigest = sha1(msg+"SEND")
             replyDigest = sha1(msg+"RECEIVED")
         else:
-            warn("Unrecognized command.  Closing connection.")
+            warn("Unrecognized command from %s.  Closing connection.",
+		 self.address)
             self.shutdown(err=1)
             return
         if expectedDigest != digest:
-            warn("Invalid checksum. Closing connection.")
+            warn("Invalid checksum from %s. Closing connection",
+		 self.address)
             self.shutdown(err=1)
             return
         else:
-            debug("Packet received; Checksum valid.")
+            debug("%s packet received from %s; Checksum valid.",
+		  data[:4], self.address)
             self.finished = self.__sentAck
             self.beginWrite(RECEIVED_CONTROL+replyDigest)
             self.messageConsumer(msg)
@@ -505,7 +517,7 @@
     def __sentAck(self):
         """Called once we're done sending an ACK.  Begins reading a new
            message."""
-        trace("done w/ send ack")
+        debug("Send ACK for message from %s (fd %s)", self.address, self.fd)
         #FFFF Rehandshake
         self.finished = self.__receivedMessage
         self.expectRead(SEND_RECORD_LEN)
@@ -530,7 +542,6 @@
            failCallback -- None, or a function of (msg, handle, retriable)
               to be called when messages can't be sent."""
 	
-        trace("CLIENT CON")
         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         sock.setblocking(0)
         self.keyID = keyID
@@ -550,6 +561,8 @@
         self.sentCallback = sentCallback
 	self.failCallback = failCallback
 
+        debug("Opening client connection (fd %s)", self.fd)
+
     def __setupFinished(self):
         """Called when we're done with the client side negotations.
            Begins sending the protocol string.
@@ -557,11 +570,11 @@
         keyID = sha1(self.getPeerPK().encode_key(public=1))
         if self.keyID is not None:
             if keyID != self.keyID:
-                warn("Got unexpected Key ID from %s", self.ip)
+                warn("Got unexpected Key ID from %s", self.address)
 		# This may work again in a couple of hours
                 self.shutdown(err=1,retriable=1)
             else:
-                debug("KeyID is valid")
+                debug("KeyID from %s is valid", self.address)
 
         self.beginWrite(PROTOCOL_STRING)
         self.finished = self.__sentProtocol
@@ -579,7 +592,7 @@
         """
         inp = self.getInput()
         if inp != PROTOCOL_STRING:
-            warn("Invalid protocol.  Closing connection")
+            warn("Invalid protocol.  Closing connection to %s", self.address)
 	    # This isn't retriable; we don't talk to servers we don't
 	    # understand.
             self.shutdown(err=1,retriable=0)
@@ -604,7 +617,7 @@
         """Called when we're done sending a message.  Begins reading the
            server's ACK."""
 
-        trace("message sent")
+        debug("Message delivered to %s (fd %s)", self.address, self.fd)
         self.finished = self.__receivedAck
         self.expectRead(RECEIVED_RECORD_LEN)
 
@@ -617,7 +630,7 @@
           If there are more messages to send, begins sending the next.
           Otherwise, begins shutting down.
        """
-       trace("received ack")
+       trace("received ack (fd %s)", self.fd)
        # FFFF Rehandshake
        inp = self.getInput()
        if inp != (RECEIVED_CONTROL+self.expectedDigest):
@@ -626,7 +639,7 @@
            self.shutdown(err=1,retriable=0)
            return
 
-       debug("Received valid ACK for message.")
+       debug("Received valid ACK for message from %s", self.address)
        justSent = self.messageList[0]
        justSentHandle = self.handleList[0]
        del self.messageList[0]
@@ -642,7 +655,7 @@
 	    for msg, handle in zip(self.messageList, self.handleList):
 		self.failCallback(msg,handle,retriable)
 
-LISTEN_BACKLOG = 10 # ???? Is something else more reasonable
+LISTEN_BACKLOG = 10 # ???? Is something else more reasonable?
 class MMTPServer(AsyncServer):
     """A helper class to invoke AsyncServer, MMTPServerConnection, and
        MMTPClientConnection"""

Index: Modules.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Modules.py,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -d -r1.21 -r1.22
--- Modules.py	2 Dec 2002 10:13:49 -0000	1.21
+++ Modules.py	7 Dec 2002 04:03:35 -0000	1.22
@@ -148,7 +148,6 @@
 		if result == DELIVER_OK:
 		    self.deliverySucceeded(handle)
 		elif result == DELIVER_FAIL_RETRY:
-		    # XXXX We need to drop undeliverable messages eventually!
 		    self.deliveryFailed(handle, 1)
 		else:
 		    getLog().error("Unable to deliver message")
@@ -159,13 +158,17 @@
 		self.deliveryFailed(handle, 0)
 
 class ModuleManager:
-    """A ModuleManager knows about all of the modules in the systems.
+    """A ModuleManager knows about all of the server modules in the system.
 
        A module may be in one of three states: unloaded, registered, or
        enabled.  An unloaded module is just a class in a python module.
        A registered module has been loaded, configured, and listed with
-       the ModuleManager, but will not receive messags until it has been
-       enabled."""
+       the ModuleManager, but will not receive messags until it is
+       enabled.
+
+       Because modules need to tell the ServerConfig object aboutt their
+       configuration options, initializing the ModuleManager is usually done
+       through ServerConfig.  See ServerConfig.getModuleManager()."""
     ##
     # Fields
     #    syntax: extensions to the syntax configuration in Config.py
@@ -178,8 +181,8 @@
     #    queues: a map from module name to queue (Queue objects must support
     #            queueMessage and sendReadyMessages as in DeliveryQueue.)
 
-
     def __init__(self):
+	"Create a new ModuleManager"
         self.syntax = {}
         self.modules = []
         self.enabled = {}
@@ -231,6 +234,7 @@
         pyPkg = ".".join(ids[:-1])
         pyClassName = ids[-1]
 	orig_path = sys.path[:]
+	getLog().info("Loading module %s", className)
         try:
 	    sys.path[0:0] = self.path
 	    try:
@@ -260,7 +264,8 @@
             m.configure(config, self)
 
     def enableModule(self, module):
-	"""Maps all the types for a module object."""
+	"""Sets up the module manager to deliver all messages whose exitTypes
+            are returned by <module>.getExitTypes() to the module."""
         for t in module.getExitTypes():
             if (self.typeToModule.has_key(t) and
                 self.typeToModule[t].getName() != module.getName()):
@@ -299,6 +304,8 @@
                            exitType)
 	    return
 	queue = self.queues[mod.getName()]
+	getLog().debug("Delivering message %r (type %04x) via module %s",
+		       message[:8], exitType, mod.getName())
 	try:
 	    payload = mixminion.BuildMessage.decodePayload(message, tag)
 	except MixError, _:
@@ -357,11 +364,11 @@
                  }
 
     def validateConfig(self, sections, entries, lines, contents):
-        # XXXX write this.  Parse address file.
+        # XXXX001 write this.  Parse address file.
         pass
 
     def configure(self, config, moduleManager):
-        # XXXX Check this.  Conside error handling
+        # XXXX001 Check this.  Conside error handling
 	
         self.enabled = config['Delivery/MBOX'].get("Enabled", 0)
 	if not self.enabled:
@@ -492,7 +499,7 @@
                  }
                    
     def validateConfig(self, sections, entries, lines, contents):
-        #FFFF implement
+        #FFFF001 implement
         pass
     def configure(self, config, manager):
         sec = config['Delivery/SMTP-Via-Mixmaster']
@@ -546,6 +553,7 @@
         
 #----------------------------------------------------------------------
 def sendSMTPMessage(server, toList, fromAddr, message):
+    getLog().trace("Sending message via SMTP host %s to %s", server, toList)
     con = smtplib.SMTP(server)
     try:
 	con.sendmail(fromAddr, toList, message)
@@ -564,7 +572,7 @@
 # DOCDOC
 _allChars = "".join(map(chr, range(256)))
 # DOCDOC
-# ???? Are there any nonprinting chars >= 0x7f to worry about now?
+# ????001 Are there any nonprinting chars >= 0x7f to worry about now?
 _nonprinting = "".join(map(chr, range(0x00, 0x07)+range(0x0E, 0x20)))
 def isPrintable(s):
     """Return true iff s consists only of printable characters."""

Index: Packet.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Packet.py,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -d -r1.17 -r1.18
--- Packet.py	2 Dec 2002 10:20:18 -0000	1.17
+++ Packet.py	7 Dec 2002 04:03:35 -0000	1.18
@@ -185,13 +185,13 @@
                 "routinglen=%(routinglen)r)")% self.__dict__
 
     def getExitAddress(self):
-	# XXXX SPEC This is not explicit in the spec.
+	# XXXX001 SPEC This is not explicit in the spec.
 	assert self.routingtype >= mixminion.Modules.MIN_EXIT_TYPE
 	assert len(self.routinginfo) >= TAG_LEN
 	return self.routinginfo[TAG_LEN:]
     
     def getTag(self):
-	# XXXX SPEC This is not explicit in the spec.
+	# XXXX001 SPEC This is not explicit in the spec.
 	assert self.routingtype >= mixminion.Modules.MIN_EXIT_TYPE
 	assert len(self.routinginfo) >= TAG_LEN
 	return self.routinginfo[:TAG_LEN]
@@ -268,7 +268,7 @@
 # Number of bytes taken up from OAEP padding in an encrypted forward
 # payload, minus bytes saved by spilling the RSA-encrypted block into the
 # tag, minus the bytes taken by the session key.
-# XXXX (The e2e note is off by 4.)
+# XXXX001 (The e2e note is off by 4.)
 ENC_FWD_OVERHEAD = OAEP_OVERHEAD - TAG_LEN + SECRET_LEN
 
 def parsePayload(payload):
@@ -454,7 +454,7 @@
 		self.port == other.port and self.keyinfo == other.keyinfo)
 
 #DOCDOC
-# FFFF Support subdomains and quotesd strings
+# FFFF Support subdomains and quoted strings
 _ATOM_PAT = r'[^\x00-\x20()\[\]()<>@,;:\\".\x7f-\xff]+'
 _LOCAL_PART_PAT = r"(?:%s)(?:\.(?:%s))*" % (_ATOM_PAT, _ATOM_PAT)
 _RFC822_PAT = r"\A%s@%s\Z" % (_LOCAL_PART_PAT, _LOCAL_PART_PAT)

Index: Queue.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Queue.py,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -d -r1.21 -r1.22
--- Queue.py	2 Dec 2002 03:27:52 -0000	1.21
+++ Queue.py	7 Dec 2002 04:03:35 -0000	1.22
@@ -27,11 +27,11 @@
 
 # Any inp_* files older than INPUT_TIMEOUT seconds old are assumed to be
 # trash.
-INPUT_TIMEOUT = 600
+INPUT_TIMEOUT = 6000
 
 # If we've been cleaning for more than CLEAN_TIMEOUT seconds, assume the
 # old clean is dead.
-CLEAN_TIMEOUT = 60
+CLEAN_TIMEOUT = 120
 
 class Queue:
     """A Queue is an unordered collection of files with secure remove and
@@ -205,8 +205,8 @@
            Returns 1 if a clean is already in progress; otherwise
            returns 0.
         """
-	# XXXX This is race-prone if multiple processes sometimes try to clean
-	# XXXX   the same queue.
+	# XXXX001 This is race-prone if multiple processes sometimes try to 
+	# XXXX001   clean the same queue.  Use O_EXCL, Luke.
         now = time.time()
 
         cleanFile = os.path.join(self.dir,".cleaning")
@@ -366,6 +366,7 @@
     """A TimedMixQueue holds a group of files, and returns some of them
        as requested, according to a mixing algorithm that sends a batch
        of messages every N seconds."""
+    # FFFF : interval is unused.
     def __init__(self, location, interval=600):
 	"""Create a TimedMixQueue that sends its entire batch of messages
 	   every 'interval' seconds."""
@@ -384,6 +385,7 @@
     """A CottrellMixQueue holds a group of files, and returns some of them
        as requested, according the Cottrell (timed dynamic-pool) mixing
        algorithm from Mixmaster."""
+    # FFFF : interval is unused.
     def __init__(self, location, interval=600, minPool=6, minSend=1,
 		 sendRate=.7):
 	"""Create a new queue that yields a batch of message every 'interval'

Index: ServerInfo.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ServerInfo.py,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -d -r1.21 -r1.22
--- ServerInfo.py	2 Dec 2002 03:30:07 -0000	1.21
+++ ServerInfo.py	7 Dec 2002 04:03:35 -0000	1.22
@@ -80,7 +80,7 @@
 
     def __init__(self, fname=None, string=None, assumeValid=0):
 	mixminion.Config._ConfigFile.__init__(self, fname, string, assumeValid)
-	getLog().trace("Read server %s from %s",
+	getLog().trace("Reading server descriptor %s from %s",
 		       self['Server']['Nickname'],
 		       fname or "<string>")
 
@@ -117,7 +117,10 @@
 							 identityKey):
 	    raise ConfigError("Invalid signature")
 
-	#### XXXX CHECK OTHER SECTIONS
+	#### XXXX001 CHECK OTHER SECTIONS
+
+    def getNickname(self):
+	return self['Server']['Nickname']
 
     def getAddr(self):
 	return self['Server']['IP']
@@ -448,6 +451,8 @@
     for ip in ip_set.keys():
 	if ip.startswith("127.") or ip.startswith("0."):
 	    del ip_set[ip]
+
+    # FFFF reject 192.168, 10., 176.16.x
 
     if len(ip_set) == 0:
 	raise IPGuessError("No address found")

Index: ServerMain.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ServerMain.py,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -d -r1.16 -r1.17
--- ServerMain.py	2 Dec 2002 10:13:49 -0000	1.16
+++ ServerMain.py	7 Dec 2002 04:03:35 -0000	1.17
@@ -19,7 +19,7 @@
 import mixminion.Crypto
 import mixminion.Queue
 import mixminion.MMTPServer
-from mixminion.ServerInfo import ServerKeyset, ServerInfo, _date, \
+from mixminion.ServerInfo import ServerKeyset, ServerInfo, _date, _time, \
      generateServerDescriptorAndKeys
 from mixminion.Common import getLog, MixFatalError, MixError, secureDelete, \
      createPrivateDir, previousMidnight, ceilDiv
@@ -45,7 +45,8 @@
     """A ServerKeyRing remembers current and future keys, descriptors, and
        hash logs for a mixminion server.
 
-       FFFF: We need a way to generate keys as needed
+       FFFF We need a way to generate keys as needed, not just a month's
+       FFFF worth of keys up front. 
        """
     # homeDir: server home directory
     # keyDir: server key directory
@@ -76,7 +77,10 @@
 	firstKey = sys.maxint
 	lastKey = 0
 
+	getLog().debug("Scanning server keystore at %s", self.keyDir)
+
 	if not os.path.exists(self.keyDir):
+	    getLog().info("Creating server keystore at %s", self.keyDir)
 	    createPrivateDir(self.keyDir)
 
         for dirname in os.listdir(self.keyDir):
@@ -103,6 +107,8 @@
                 t1 = inf['Server']['Valid-After']
                 t2 = inf['Server']['Valid-Until']
                 self.keyIntervals.append( (t1, t2, keysetname) )
+		getLog().debug("Found key %s (valid from %s to %s)",
+			       dirname, _date(t1), _date(t2))
 	    else:
 		getLog().warn("No server descriptor found for key %s"%dirname)
 
@@ -258,7 +264,7 @@
         return self.nextKeyRotation
 
     def getServerKeyset(self):
-	"""Return a ServerKeyset object for  the currently live key."""
+	"""Return a ServerKeyset object for the currently live key."""
 	# FFFF Support passwords on keys
 	_, _, name = self._getLiveKey()
 	keyset = ServerKeyset(self.keyDir, name, self.hashDir)
@@ -275,6 +281,9 @@
             getLog().info("Generating Diffie-Helman parameters for TLS...")
             mixminion._minionlib.generate_dh_parameters(dhfile, verbose=0)
             getLog().info("...done")
+	else:
+	    getLog().debug("Using existing Diffie-Helman parameter from %s",
+			   dhfile)
 
         return dhfile
 
@@ -311,6 +320,7 @@
 
     def queueMessage(self, msg):
 	"""Add a message for delivery"""
+	getLog().trace("Inserted message %r into incoming queue", msg[:8])
 	self.queueDeliveryMessage(None, msg)
 
     def _deliverMessages(self, msgList):
@@ -321,18 +331,22 @@
 		res = ph.processMessage(message)
 		if res is None:
 		    # Drop padding before it gets to the mix.
-		    getLog().info("Padding message dropped")
+		    getLog().debug("Padding message %r dropped", 
+				   message[:8])
 		else:
+		    getLog().debug("Processed message %r; inserting into pool",
+				   message[:8])
 		    self.mixPool.queueObject(res)
 		    self.deliverySucceeded(handle)
 	    except mixminion.Crypto.CryptoError, e:
-		getLog().warn("Invalid PK or misencrypted packet header:"+str(e))
+		getLog().warn("Invalid PK or misencrypted packet header: %s",
+			      e)
 		self.deliveryFailed(handle)
 	    except mixminion.Packet.ParseError, e:
-		getLog().warn("Malformed message dropped:"+str(e))
+		getLog().warn("Malformed message dropped: %s", e)
 		self.deliveryFailed(handle)
 	    except mixminion.PacketHandler.ContentError, e:
-		getLog().warn("Discarding bad packet:"+str(e))
+		getLog().warn("Discarding bad packet: %s", e)
 		self.deliveryFailed(handle)
 
 class MixPool:
@@ -348,6 +362,10 @@
 	"""Insert an object into the queue."""
 	self.queue.queueObject(obj)
 
+    def count(self):
+	"Return the number of messages in the queue"
+	return self.queue.count()
+
     def connectQueues(self, outgoing, manager):
 	"""Sets the queue for outgoing mixminion packets, and the
   	   module manager for deliverable messages."""
@@ -358,15 +376,20 @@
 	"""Get a batch of messages, and queue them for delivery as
 	   appropriate."""
 	handles = self.queue.getBatch()
-	getLog().trace("Mixing %s messages", len(handles))
+	getLog().debug("Mixing %s messages out of %s", 
+		       len(handles), self.queue.count())
 	for h in handles:
 	    tp, info = self.queue.getObject(h)
 	    if tp == 'EXIT':
 		rt, ri, app_key, tag, payload = info
+		getLog().debug("  (sending message %r to exit modules)", 
+			       payload[:8])
 		self.moduleManager.queueMessage(payload, tag, rt, ri)
 	    else:
 		assert tp == 'QUEUE'
 		ipv4, msg = info
+		getLog().debug("  (sending message %r to MMTP server)", 
+			       msg[:8])
 		self.outgoingQueue.queueDeliveryMessage(ipv4, msg)
 	    self.queue.removeMessage(h)
 
@@ -418,6 +441,7 @@
        all timed events."""
     def __init__(self, config):
 	"""Create a new server from a ServerConfig."""
+	getLog().debug("Initializing server")
 	self.config = config
 	self.keyring = ServerKeyring(config)
 	if self.keyring._getLiveKey() is None:
@@ -428,27 +452,42 @@
 	    nKeys = ceilDiv(30*24*60*60, keylife)
 	    self.keyring.createKeys(nKeys)
 
+	getLog().trace("Initializing packet handler")
 	self.packetHandler = self.keyring.getPacketHandler()
+	getLog().trace("Initializing TLS context")
 	tlsContext = self.keyring.getTLSContext()
+	getLog().trace("Initializing MMTP server")
 	self.mmtpServer = _MMTPServer(config, tlsContext)
 
 	# FFFF Modulemanager should know about async so it can patch in if it
 	# FFFF needs to.
+	getLog().trace("Initializing delivery module")
 	self.moduleManager = config.getModuleManager()
+	self.moduleManager.configure(config)
 
 	homeDir = config['Server']['Homedir']
 	queueDir = os.path.join(homeDir, 'work', 'queues')
 
 	incomingDir = os.path.join(queueDir, "incoming")
+	getLog().trace("Initializing incoming queue")
 	self.incomingQueue = IncomingQueue(incomingDir, self.packetHandler)
+	getLog().trace("Found %d pending messages in incoming queue", 
+		       self.incomingQueue.count())
 
 	mixDir = os.path.join(queueDir, "mix")
 	# FFFF The choice of mix algorithm should be configurable
+	getLog().trace("Initializing Mix pool")
 	self.mixPool = MixPool(mixminion.Queue.TimedMixQueue(mixDir, 60))
+	getLog().trace("Found %d pending messages in Mix pool",
+		       self.mixPool.count())
 
 	outgoingDir = os.path.join(queueDir, "outgoing")
+	getLog().trace("Initializing outgoing queue")
 	self.outgoingQueue = OutgoingQueue(outgoingDir)
+	getLog().trace("Found %d pending messages in outgoing queue",
+		       self.outgoingQueue.count())
 
+	getLog().trace("Connecting queues")
 	self.incomingQueue.connectQueues(mixPool=self.mixPool)
 	self.mixPool.connectQueues(outgoing=self.outgoingQueue,
 				   manager=self.moduleManager)
@@ -458,7 +497,8 @@
 
     def run(self):
 	"""Run the server; don't return unless we hit an exception."""
-	# FFFF Use heapq to schedule events?
+	# FFFF Use heapq to schedule events? [I don't think so; there are only
+	# FFFF   two events, after all!]
 	now = time.time()
 	MIX_INTERVAL = 20  # FFFF Configurable!
 	nextMix = now + MIX_INTERVAL
@@ -466,6 +506,7 @@
 	#FFFF Unused
 	#nextRotate = self.keyring.getNextKeyRotation()
 	while 1:
+	    getLog().trace("Next mix at %s", _time(nextMix))
 	    while time.time() < nextMix:
 		# Handle pending network events
 		self.mmtpServer.process(1)
@@ -492,7 +533,7 @@
 
 	    if now > nextShred:
 		# FFFF Configurable shred interval
-		getLog().trace("Expunging queues")
+		getLog().trace("Expunging deleted messages from queues")
 		self.incomingQueue.cleanQueue()
 		self.mixPool.queue.cleanQueue()
 		self.outgoingQueue.cleanQueue()
@@ -544,13 +585,12 @@
 	getLog().debug("Configuring server")
 	mixminion.Common.configureShredCommand(config)
 	mixminion.Crypto.init_crypto(config)
-	config.getModuleManager().configure(config)
 
 	server = MixminionServer(config)
     except:
 	getLog().fatal_exc(sys.exc_info(),"Exception while configuring server")
 	print >>sys.stderr, "Shutting down because of exception"
-        #XXXX print stack trace
+        #XXXX print stack trace as well as logging?
 	sys.exit(1)
 
     getLog().info("Starting server")
@@ -560,7 +600,7 @@
 	pass
     except:
 	getLog().fatal_exc(sys.exc_info(),"Exception while running server")
-        #XXXX print stack trace
+        #XXXX print stack trace as well as logging?
     getLog().info("Server shutting down")
     server.close()
     getLog().info("Server is shut down")

Index: test.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/test.py,v
retrieving revision 1.41
retrieving revision 1.42
diff -u -d -r1.41 -r1.42
--- test.py	2 Dec 2002 20:18:44 -0000	1.41
+++ test.py	7 Dec 2002 04:03:35 -0000	1.42
@@ -868,6 +868,7 @@
         self.key = key
         self.keyid = keyid
 
+    def getNickname(self): return "N(%s:%s)"%(self.addr,self.port)
     def getAddr(self): return self.addr
     def getPort(self): return self.port
     def getPacketKey(self): return self.key
@@ -930,18 +931,18 @@
 		self.assert_(BuildMessage._checkPayload(pld))
 		self.assertEquals(len(comp), ord(pld[0])*256+ord(pld[1]))
 		self.assertEquals(0, ord(pld[0])&0x80)
-		self.assertEquals(m, BuildMessage.decodePayloadImpl(pld))
+		self.assertEquals(m, BuildMessage._decodePayloadImpl(pld))
 
-	self.failUnlessRaises(MixError, BuildMessage.decodePayloadImpl, b)
+	self.failUnlessRaises(MixError, BuildMessage._decodePayloadImpl, b)
 
 	# Check fragments (not yet supported)
 	pldFrag = chr(ord(pld[0])|0x80)+pld[1:]
-	self.failUnlessRaises(MixError, BuildMessage.decodePayloadImpl,pldFrag)
+	self.failUnlessRaises(MixError,BuildMessage._decodePayloadImpl,pldFrag)
 
 	# Check impossibly long messages
 	pldSize = "\x7f\xff"+pld[2:] #sha1(pld[22:])+pld[22:]
 	self.failUnlessRaises(ParseError,
-			      BuildMessage.decodePayloadImpl,pldSize)
+			      BuildMessage._decodePayloadImpl,pldSize)
 
     def test_buildheader_1hop(self):
         bhead = BuildMessage._buildHeader
@@ -1187,7 +1188,7 @@
             p = lioness_decrypt(p,ks.getLionessKeys(PAYLOAD_ENCRYPT_MODE))
 
 	if decoder is None:
-	    p = BuildMessage.decodeForwardPayload(p)
+	    p = BuildMessage._decodeForwardPayload(p)
 	else:
 	    p = decoder(p, tag)
 
@@ -1221,7 +1222,7 @@
 
 	def decoder0(p,t,messages=messages):
 	    messages['fwd'] = (p,t)
-	    return BuildMessage.decodeForwardPayload(p)
+	    return BuildMessage._decodeForwardPayload(p)
 
         self.do_message_test(m,
                              ( (self.pk1,), None,
@@ -1243,7 +1244,7 @@
 		     rsakey)
 	    def decoder(p,t,key=rsakey,messages=messages):
 		messages['efwd'+str(key.get_modulus_bytes())] = (p,t)
-		return BuildMessage.decodeEncryptedForwardPayload(p,t,key)
+		return BuildMessage._decodeEncryptedForwardPayload(p,t,key)
 
 	    self.do_message_test(m,
 				 ( (self.pk1, self.pk2), None,
@@ -1310,7 +1311,7 @@
 	messages = {}
 	def decoder(p,t,secrets=secrets_1,messages=messages):
 	    messages['repl'] = p,t
-	    return BuildMessage.decodeReplyPayload(p,secrets)
+	    return BuildMessage._decodeReplyPayload(p,secrets)
 
         self.do_message_test(m,
                              ((self.pk3, self.pk1), None,
@@ -1351,7 +1352,7 @@
 
 	def decoder2(p,t,messages=messages):
 	    messages['srepl'] = p,t
-	    return BuildMessage.decodeStatelessReplyPayload(p,t,
+	    return BuildMessage._decodeStatelessReplyPayload(p,t,
 							 "Tyrone Slothrop")
         self.do_message_test(m,
                              ((self.pk3, self.pk1), None,
@@ -1402,7 +1403,7 @@
 	encoded1 = (comp+ "RWE/HGW"*4096)[:28*1024-22]
 	encoded1 = '\x00\x6D'+sha1(encoded1)+encoded1
 	# Forward message.
-	self.assertEquals(payload, BuildMessage.decodeForwardPayload(encoded1))
+	self.assertEquals(payload, BuildMessage._decodeForwardPayload(encoded1))
 
 	# Encoded forward message
 	efwd = (comp+"RWE/HGW"*4096)[:28*1024-22-38]
@@ -1414,7 +1415,7 @@
 	efwd_t = efwd_rsa[:20]
 	efwd_p = efwd_rsa[20:]+efwd_lioness
 	self.assertEquals(payload,
-	     BuildMessage.decodeEncryptedForwardPayload(efwd_p,efwd_t,rsa1))
+	     BuildMessage._decodeEncryptedForwardPayload(efwd_p,efwd_t,rsa1))
 
 	# Stateful reply
 	secrets = [ "Is that you, Des","troyer?Rinehart?" ]
@@ -1423,7 +1424,7 @@
 	m = lioness_decrypt(encoded1, ks.getLionessKeys(PAYLOAD_ENCRYPT_MODE))
 	ks = Keyset(secrets[0])
 	m = lioness_decrypt(m, ks.getLionessKeys(PAYLOAD_ENCRYPT_MODE))
-	self.assertEquals(payload, BuildMessage.decodeReplyPayload(m,secrets))
+	self.assertEquals(payload, BuildMessage._decodeReplyPayload(m,secrets))
 	repl1 = m
 
 	# Stateless reply
@@ -1439,7 +1440,7 @@
 	    key = Keyset(k).getLionessKeys(PAYLOAD_ENCRYPT_MODE)
 	    m = lioness_decrypt(m,key)
 	self.assertEquals(payload,
-		     BuildMessage.decodeStatelessReplyPayload(m,tag,passwd))
+		     BuildMessage._decodeStatelessReplyPayload(m,tag,passwd))
 	repl2, repl2tag = m, tag
 
 	#
@@ -1498,7 +1499,7 @@
 	# Bad efwd
 	efwd_pbad = efwd_p[:-1] + chr(ord(efwd_p[-1])^0xaa)
 	self.failUnlessRaises(MixError,
-			      BuildMessage.decodeEncryptedForwardPayload,
+			      BuildMessage._decodeEncryptedForwardPayload,
 			      efwd_pbad, efwd_t, self.pk1)
 	for d in (sdict, None):
 	    for p in (passwd, None):
@@ -1516,7 +1517,7 @@
 			 decodePayload, repl1_bad, "tag1"*5, pk, sd, p)
 		sd = sdict.copy()
 		self.failUnlessRaises(MixError,
-			 BuildMessage.decodeReplyPayload, repl1_bad,
+			 BuildMessage._decodeReplyPayload, repl1_bad,
 				      sd["tag1"*5])
 	# Bad srepl
 	repl2_bad = repl2[:-1] + chr(ord(repl2[-1])^0xaa)
@@ -1576,7 +1577,7 @@
 
 		#tag = res[1][3]
 		p = res[1][4]
-		p = BuildMessage.decodeForwardPayload(p)
+		p = BuildMessage._decodeForwardPayload(p)
                 self.assert_(p.startswith(payload))
                 break
 
@@ -3184,24 +3185,13 @@
 	# Test getPacketHandler
 	_ = keyring.getPacketHandler()
 
-    def testIncomingQueue(self):
-	# Test _deliverMessage.
-	pass
-
-    def testMixPool(self):
-	# Test 'mix' method
-	pass
-
-    def testOutgoingQueue(self):
-	# Test _deliverMessage
-	pass
-
 #----------------------------------------------------------------------
 
 _EXAMPLE_DESCRIPTORS = {} # name->list of str
 EX_SERVER_CONF_TEMPLATE = """
 [Server]
 Mode: relay
+Homedir: %(homedir)s
 EncryptIdentityKey: No
 PublicKeyLifetime: %(lifetime)s days
 IdentityKeyBits: 2048
@@ -3216,12 +3206,12 @@
 
 _EXAMPLE_DESCRIPTORS_INP = [
     # name        days         ip?        validAt
-    [ "Fred",	  "10 days", "10.0.0.6", (-19,-9,1,11) ],
-    [ "Lola",	  "5 days",  "10.0.0.7", (-2,0,5) ],
-    [ "Joe",	  "20 days", "10.0.0.8", (-15,5,25) ],
-    [ "Alice",	  "8 days",  "10.0.0.9", (-3,5,13) ],
-    [ "Bob",	  "11 days", "10.0.0.10", (-10,-1,6) ],
-    [ "Lisa",	  "3 days",  "10.0.0.11", (-10,-1,5) ],
+    [ "Fred",	  "10 days", "10.0.0.6", (-19,-9,1,11), () ],
+    [ "Lola",	  "5 days",  "10.0.0.7", (-2,0,5),      (MBOX_TYPE,) ],
+    [ "Joe",	  "20 days", "10.0.0.8", (-15,5,25),    (SMTP_TYPE,) ],
+    [ "Alice",	  "8 days",  "10.0.0.9", (-3,5,13),     () ],
+    [ "Bob",	  "11 days", "10.0.0.10", (-10,-1,6),   () ],
+    [ "Lisa",	  "3 days",  "10.0.0.11", (-10,-1,5),   () ],
 ]
 
 _EXAMPLE_DESCRIPTORS_TIME = 0
@@ -3236,13 +3226,26 @@
 
     sys.stdout.flush()
 
-    for (nickname, lifetime, ip, starting) in _EXAMPLE_DESCRIPTORS_INP:
+    for (nickname, lifetime, ip, starting, types) in _EXAMPLE_DESCRIPTORS_INP:
+	homedir = mix_mktemp()
 	conf = EX_SERVER_CONF_TEMPLATE % locals()
+	for t in types:
+	    if t == MBOX_TYPE:
+		addrf = mix_mktemp()
+		writeFile(addrf,"")
+		conf += ("[Delivery/MBOX]\nEnabled: yes\nAddressFile: %s\n"+
+			 "ReturnAddress: a@b.c\nRemoveContact: b@c.d\n") %(
+		    addrf)
+	    elif t == SMTP_TYPE:
+		conf += ("[Delivery/SMTP-Via-Mixmaster]\nEnabled: yes\n"+
+			 "MixCommand: /bin/ls\nServer: foobar\n")
 	try:
 	    suspendLog()
 	    conf = mixminion.Config.ServerConfig(string=conf)
+	    conf.getModuleManager().configure(conf)
 	finally:
 	    resumeLog()
+	    pass
 
 	_EXAMPLE_DESCRIPTORS[nickname] = []
 	for n in xrange(len(starting)):
@@ -3365,6 +3368,140 @@
 	parseFails("0x9999")
 	parseFails("0xFFFFF:zymurgy")
 
+    def testMixminionClient(self):
+	parseAddress = mixminion.ClientMain.parseAddress
+	userdir = mix_mktemp()
+	usercfgstr = "[User]\nUserDir: %s\n[DirectoryServers]\n"%userdir
+	usercfg = mixminion.Config.ClientConfig(string=usercfgstr)
+	client = mixminion.ClientMain.MixminionClient(usercfg)
+	
+	# Make sure client sets its directories up correctly.
+	serverdir = os.path.join(userdir, 'servers')
+	self.assert_(os.path.exists(serverdir))
+	self.assertEquals([], os.listdir(serverdir))
+
+	# Now try with some servers...
+	edesc = getExampleServerDescriptors()
+	writeFile(os.path.join(serverdir,"lola1"), edesc["Lola"][1])
+	writeFile(os.path.join(serverdir,"joe1"), edesc["Joe"][0])
+	writeFile(os.path.join(serverdir,"alice1"), edesc["Alice"][0])
+
+	# ... and for now, we need to restart the client.
+	client = mixminion.ClientMain.MixminionClient(usercfg)
+
+	##  Test generateForwardMessage.
+	# We replace 'buildForwardMessage' to make this easier to test.
+	replaceFunction(mixminion.BuildMessage, "buildForwardMessage", 
+			lambda *a:"X")
+	try:
+	    getCalls = getReplacedFunctionCallLog
+	    clearCalls = clearReplacedFunctionCallLog
+	    # First, two forward messages that end with 'joe' and go via
+	    # SMTP
+	    payload = "Hey Joe, where you goin' with that gun in your hand?"
+	    m1 = client.generateForwardMessage(
+		parseAddress("joe@cledonism.net"),
+		payload,
+		path1=["Lola", "Joe"], path2=["Alice", "Joe"])
+	    m2 = client.generateForwardMessage(
+		parseAddress("smtp:joe@cledonism.net"),
+		"Hey Joe, where you goin' with that gun in your hand?",
+		path1=["Lola", "Joe"], path2=["Alice", "Joe"])
+
+	    for fn, args, kwargs in getCalls():
+		self.assertEquals(fn, "buildForwardMessage")
+		self.assertEquals(args[0:3], 
+				  (payload, SMTP_TYPE, "joe@cledonism.net"))
+		self.assert_(len(args[3]) == len(args[4]) == 2)
+		self.assertEquals(["Lola", "Joe", "Alice", "Joe"],
+		     [x['Server']['Nickname'] for x in args[3]+args[4]])
+	    clearCalls()
+	    
+	    # Now try an mbox message, with an explicit last hop.
+	    payload = "Hey, Lo', where you goin' with that pun in your hand?"
+	    m1 = client.generateForwardMessage(
+		parseAddress("mbox:granola"),
+		payload,
+		path1=["Lola", "Joe"], path2=["Alice", "Lola"])
+	    # And an mbox message with a last hop implicit in the address
+	    m1 = client.generateForwardMessage(
+		parseAddress("mbox:granola@Lola"),
+		payload,
+		path1=["Lola", "Joe"], path2=["Alice"])	    
+
+	    for fn, args, kwargs in getCalls():
+		self.assertEquals(fn, "buildForwardMessage")
+		self.assertEquals(args[0:3], 
+				  (payload, MBOX_TYPE, "granola"))
+		self.assert_(len(args[3]) == len(args[4]) == 2)
+		self.assertEquals(["Lola", "Joe", "Alice", "Lola"],
+		     [x['Server']['Nickname'] for x in args[3]+args[4]])
+	    clearCalls()
+	finally:
+	    undoReplacedAttributes()
+	    clearCalls()
+
+	### Now try some failing cases for generateForwardMessage:
+	# Empty path...
+	self.assertRaises(MixError, 
+			  client.generateForwardMessage, 
+			  parseAddress("0xFFFF:zed"),
+			  "Z", [], ["Alice"])
+	# Nonexistant servers...
+	self.assertRaises(MixError, 
+			  client.generateForwardMessage,
+			  parseAddress("0xFFFF:zed"),
+			  "Z", ["Marvin"], ["Fred"])
+	# Lola doesn't support SMTP...
+	self.assertRaises(MixError, 
+			  client.generateForwardMessage,
+			  parseAddress("smtp:joe@cledonism.net"),
+			  "Z", ["Joe"], ["Lola"])
+	# Joe doesn't support MBOX...
+	self.assertRaises(MixError, 
+			  client.generateForwardMessage,
+			  parseAddress("mbox:wahoo"),
+			  "Z", ["Lola"], ["Joe"])
+	
+	
+	# Temporarily replace BlockingClientConnection so we can try the client
+	# without hitting the network.
+	class FakeBCC:
+	    def __init__(self, addr, port, keyid):
+		global BCC_INSTANCE
+		BCC_INSTANCE = self
+		self.addr = addr
+		self.port = port
+		self.keyid = keyid
+		self.packets = []
+		self.connected = 0
+	    def connect(self):
+		self.connected = 1
+	    def sendPacket(self, msg):
+		assert self.connected
+		self.packets.append(msg)
+	    def shutdown(self):
+		self.connected = 0
+
+	replaceAttribute(mixminion.MMTPClient, "BlockingClientConnection",
+			 FakeBCC)
+	try:
+	    client.sendForwardMessage(
+		parseAddress("mbox:granola@Lola"),
+		"You only give me your information.",
+		["Alice", "Lola", "Joe", "Alice"], ["Joe", "Alice"])
+	    bcc = BCC_INSTANCE
+	    # first hop is alice
+	    self.assertEquals(bcc.addr, "10.0.0.9")
+	    self.assertEquals(bcc.port, 48099)
+	    self.assertEquals(0, bcc.connected)
+	    self.assertEquals(1, len(bcc.packets))
+	    self.assertEquals(32*1024, len(bcc.packets[0]))
+
+	finally:
+	    undoReplacedAttributes()
+	    clearCalls()
+
     def isSameServerDesc(self, s1, s2):
 	"""s1 and s2 are either ServerInfo objects or strings containing server
 	   descriptors. Returns 1 iff their digest fields match"""
@@ -3380,12 +3517,13 @@
 
 #----------------------------------------------------------------------
 def testSuite():
+    """Return a PyUnit test suite containing all the unit test cases."""
     suite = unittest.TestSuite()
     loader = unittest.TestLoader()
     tc = loader.loadTestsFromTestCase
 
-    suite.addTest(tc(ModuleTests))
     if 0:
+	suite.addTest(tc(ClientMainTests))
 	return suite
 
     suite.addTest(tc(MiscTests))
@@ -3399,6 +3537,7 @@
     suite.addTest(tc(BuildMessageTests))
     suite.addTest(tc(PacketHandlerTests))
     suite.addTest(tc(QueueTests))
+    suite.addTest(tc(ModuleTests))
 
     suite.addTest(tc(ClientMainTests))
     suite.addTest(tc(ServerMainTests))