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

[minion-cvs] Lots of documentation work and code cleanup.



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

Modified Files:
	ClientMain.py Common.py Main.py Packet.py ServerInfo.py 
	test.py 
Log Message:
Lots of documentation work and code cleanup.

ClientMain:
- Refactor path selection so it's easier understand.

Main: 
- Add a new banner.

ServerInbox:
- Actually use ServerQueuedException as documented.

ServerMain:
- Generate keys when keygen is needed, not when key rotation is needed!
- Flush hashlogs on SIGHUP.
- Simplify USAGE code.
- Remove last vestiges of 'stop-server' and 'reload-server' homonyms.

ServerQueue:
- Put off memory-saving work until 0.0.5; it's too invasive for now.

*.py:
- Comment code, resolve XXXXs and DOCDOCs.



Index: ClientMain.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ClientMain.py,v
retrieving revision 1.89
retrieving revision 1.90
diff -u -d -r1.89 -r1.90
--- ClientMain.py	5 Jun 2003 18:41:40 -0000	1.89
+++ ClientMain.py	6 Jun 2003 06:04:57 -0000	1.90
@@ -1,5 +1,5 @@
 # Copyright 2002-2003 Nick Mathewson.  See LICENSE for licensing information.
-# $Id$
+# Id: ClientMain.py,v 1.89 2003/06/05 18:41:40 nickm Exp $
 
 """mixminion.ClientMain
 
@@ -636,7 +636,6 @@
 
         # Now figure out which relays we haven't used yet.
         used = filter(None, servers)
-        nNeeded = len([info for info in servers if info is None])
         relays = self.__find(self.byCapability['relay'], startAt, endAt)
         if not relays:
             raise UIError("No relays known")
@@ -745,42 +744,51 @@
        path, nHops must equal the path length; and if nHops is used _with_ a
        star on the path, nHops must be >= the path length.
     """
-    #DOCDOC comment this.
     if not path:
         path = '*'
-    explicitSwap = 0
+    # Break path into a list of entries of the form:
+    #        Nickname
+    #     or "<swap>"
+    #     or "?"
     p = path.replace(":", ",<swap>,").split(",")
     path = []
     for ent in p:
         if re.match(r'\*(\d+)', ent):
-            path.extend(("?")* int(ent[1:]))
+            path.extend(["?"]*int(ent[1:]))
         else:
             path.append(ent)
 
-    starPos = None
-    colonPos = None
-    for i in xrange(len(path)):
-        if path[i] == '<swap>':
-            if colonPos is not None:
-                raise UIError("Can't specify swap point twise")
-            colonPos = i
-            explicitSwap = 1
-        elif path[i] == '*':
-            if starPos is not None:
-                raise UIError("Can't have two variable-length wildcards in a path")
-            starPos = i
+    # set explicitSwap to true iff the user specified a swap point.
+    explicitSwap = path.count("<swap>")
+    # set colonPos to the index of the explicit swap point, if any.
+    if path.count("<swap>") > 1:
+        raise UIError("Can't specify swap point twise")
 
-    myNHops = nHops or defaultNHops or 6
+    # set starPos to the index of the var-length wildcard, if any.
+    if path.count("*") > 1:
+        raise UIError("Can't have two variable-length wildcards in a path")
+    elif path.count("*") == 1:
+        starPos = path.index("*")
+    else:
+        starPos = None
+
+    # If there's a variable-length wildcard...
     if starPos is not None:
+        # Find out how many hops we should have.
+        myNHops = nHops or defaultNHops or 6
+        # Figure out how many nodes we need to add.
         haveHops = len(path) - 1
-        if colonPos is not None:
+        # A colon will throw the count off.
+        if explicitSwap:
             haveHops -= 1
         path[starPos:starPos+1] = ["?"]*max(0,myNHops-haveHops)
 
-    if colonPos is not None:
+    # Figure out how long the first leg should be.
+    if explicitSwap:
+        # Calculate colon position
+        colonPos = path.index("<swap>")
         if halfPath:
             raise UIError("Can't specify swap point with replies")
-        colonPos = path.index("<swap>")
         firstLegLen = colonPos
         del path[colonPos]
     elif halfPath:
@@ -788,13 +796,16 @@
     else:
         firstLegLen = ceilDiv(len(path), 2)
 
+    # Do we have the right # of hops?
     if nHops is not None and len(path) != nHops:
         raise UIError("Mismatch between specified path lengths")
 
+    # Replace all '?'s in path with [None].
     for i in xrange(len(path)):
         if path[i] == '?': path[i] = None
 
-    # DOCDOC Remark: why do this now
+    # Figure out what capability we need in our exit node, so that
+    # we can tell the directory.
     if address is None:
         rt, ri, exitNode = None, None, None
         exitCap = 'relay'
@@ -807,9 +818,11 @@
         else:
             exitCap = None
 
+    # If we have an explicit exit node from the address, append it.
     if exitNode is not None:
         path.append(exitNode)
 
+    # Get a list of serverinfo.
     path = directory.getPath(endCap=exitCap,
                              template=path, startAt=startAt, endAt=endAt)
 
@@ -827,16 +840,19 @@
                       % (path[-1].getNickname(), exitCap))
 
 
+    # Split the path into 2 legs.
     path1, path2 = path[:firstLegLen], path[firstLegLen:]
     if not halfPath and len(path1)+len(path2) < 2:
         raise UIError("Path is too short")
     if not halfPath and (not path1 or not path2):
         raise UIError("Each leg of the path must have at least 1 hop")
 
+    # Make sure the path can fit into the headers.
     mixminion.BuildMessage.checkPathLength(path1, path2,
                                            rt,ri,
                                            explicitSwap)
 
+    # Return the two legs of the path.
     return path1, path2
 
 def parsePathLeg(directory, config, path, nHops, address=None,
@@ -867,7 +883,8 @@
     """
     ## Fields:
     # keyDir: The directory where we store our keys.
-    # keyring: DICT DOCDOC
+    # keyring: Dict to map from strings of the form "SURB-keyname" to SURB
+    #     secrets.
     # keyringPassword: The password for our encrypted keyfile
     ## Format:
     # We store keys in a file holding:
@@ -876,7 +893,10 @@
     #  variable         ENCRYPTED DATA:KEY=sha1(salt+password+salt)
     #                                  DATA=encrypted_pickled_data+
     #                                                   sha1(data+salt+magic)
-
+    
+    # XXXX There needs to be some way to rotate and expire SURB secrets.
+    # XXXX Otherwise, we're very vulnerable to compromise.
+    
     def __init__(self, keyDir):
         """Create a new ClientKeyring to store data in keyDir"""
         self.keyDir = keyDir
@@ -885,6 +905,13 @@
         self.keyringPassword = None
 
     def getKey(self, keyid, create=0, createFn=None, password=None):
+        """Helper function. Return a key for a given keyid.
+
+           keyid -- the name of the key.
+           create -- If true, create a new key if none is found.
+           createFn -- a callback to return a new key.
+           password -- Optionally, a password for the keyring.
+        """
         if self.keyring is None:
             self.getKeyring(create=create,password=password)
             if self.keyring is None:
@@ -902,6 +929,11 @@
                 return key
 
     def getKeyring(self, create=0, password=None):
+        """Return a the current keyring, loading it if necessary.
+
+           create -- if true, create a new keyring if none is found.
+           password -- optionally, a password for the keyring.
+        """
         if self.keyring is not None:
             return self.keyring
         fn = os.path.join(self.keyDir, "keyring")
@@ -945,6 +977,7 @@
             return {}
 
     def _saveKeyring(self):
+        """Save the current keyring to disk."""
         assert self.keyringPassword is not None
         fn = os.path.join(self.keyDir, "keyring")
         LOG.trace("Saving keyring to %s", fn)
@@ -954,6 +987,7 @@
         os.rename(fn+"_tmp", fn)
 
     def getSURBKey(self, name="", create=0, password=None):
+        """Return the key for a given SURB identity."""
         k = self.getKey("SURB-"+name,
                         create=create, createFn=lambda: trng(20),
                         password=password)
@@ -962,6 +996,8 @@
         return k
 
     def getSURBKeys(self,password=None):
+        """Return the keys for _all_ SURB identities as a map from name
+           to key."""
         self.getKeyring(create=0,password=password)
         if not self.keyring: return {}
         r = {}
@@ -1819,7 +1855,7 @@
                         "Unrecognized value for %s. Expected 'yes' or 'no'"%o)
                 if self.download not in (None, dl):
                     raise UIError(
-                        "Value of %s for %o conflicts with earlier value" %
+                        "Value of %s for %s conflicts with earlier value" %
                         (v, o))
                 self.download = dl
             elif o in ('-t', '--to'):
@@ -2067,6 +2103,8 @@
 """.strip()
 
 def sendUsageAndExit(cmd, error=None):
+    """Print a usage message for the mixminion send command (and family)
+       and exit."""
     if error:
         print >>sys.stderr, "ERROR: %s"%error
         print >>sys.stderr, "For usage, run 'mixminion send --help'"
@@ -2082,7 +2120,10 @@
     sys.exit(0)
 
 def runClient(cmd, args):
-    #DOCDOC Comment this function
+    """[Entry point]  Generate an outgoing mixminion message and possibly
+       send it.  Implements 'mixminion send' and 'mixminion queue'."""
+
+    # Are we queueing?
     queueMode = 0
     if cmd.endswith(" queue"):
         queueMode = 1
@@ -2090,6 +2131,8 @@
         LOG.warn("The 'pool' command is deprecated.  Use 'queue' instead.")
         queueMode = 1
 
+    ###
+    # Parse and validate our options.
     #XXXX005 remove obsolete swap-at option.
     options, args = getopt.getopt(args, "hvf:D:t:H:P:R:i:",
              ["help", "verbose", "config=", "download-directory=",
@@ -2135,6 +2178,7 @@
     path1, path2 = parser.getForwardPath()
     address = parser.address
 
+    # Get our surb, if any.
     if parser.usingSURBList and inFile in ('-', None):
         # We check to make sure that we have a valid SURB before reading
         # from stdin.
@@ -2142,7 +2186,7 @@
         try:
             s = surblog.findUnusedSURB(parser.path2)
             if s is None:
-                raise UIError("No unused, unexpired reply blocks found.")
+                raise UIError("No unused and unexpired reply blocks found.")
         finally:
             surblog.close()
 
@@ -2186,8 +2230,7 @@
                             fresh directory.
 """
 def runPing(cmd, args):
-    #DOCDOC comment me
-
+    """[Entry point] Send link padding to servers to see if they're up."""
     if len(args) == 1 and args[0] in ('-h', '--help'):
         print _PING_USAGE
         sys.exit(0)
@@ -2241,6 +2284,7 @@
 """.strip()
 
 def importServer(cmd, args):
+    """[Entry point] Manually add a server to the client directory."""
     options, args = getopt.getopt(args, "hf:v", ['help', 'config=', 'verbose'])
 
     try:
@@ -2283,11 +2327,13 @@
 """.strip()
 
 def listServers(cmd, args):
+    """[Entry point] Print info about """
     options, args = getopt.getopt(args, "hf:D:v",
                                   ['help', 'config=', "download-directory=",
                                    'verbose'])
     try:
-        parser = CLIArgumentParser(options, wantConfig=1, wantClientDirectory=1,
+        parser = CLIArgumentParser(options, wantConfig=1,
+                                   wantClientDirectory=1,
                                    wantLog=1, wantDownload=1)
     except UsageError, e:
         e.dump()
@@ -2354,7 +2400,7 @@
 """.strip()
 
 def clientDecode(cmd, args):
-    #DOCDOC Comment me
+    """[Entry point] Decode a message."""
     options, args = getopt.getopt(args, "hvf:o:Fi:",
           ['help', 'verbose', 'config=',
            'output=', 'force', 'input='])
@@ -2459,7 +2505,6 @@
 """.strip()
 
 def generateSURB(cmd, args):
-    #DOCDOC Comment me
     options, args = getopt.getopt(args, "hvf:D:t:H:P:o:bn:",
           ['help', 'verbose', 'config=', 'download-directory=',
            'to=', 'hops=', 'path=', 'lifetime=',
@@ -2624,7 +2669,7 @@
 
 def listQueue(cmd, args):
     options, args = getopt.getopt(args, "hvf:",
-             ["help", "verbose", "config=", ])
+                                  ["help", "verbose", "config=", ])
     try:
         parser = CLIArgumentParser(options, wantConfig=1, wantLog=1,
                                    wantClient=1)

Index: Common.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Common.py,v
retrieving revision 1.88
retrieving revision 1.89
diff -u -d -r1.88 -r1.89
--- Common.py	5 Jun 2003 18:41:40 -0000	1.88
+++ Common.py	6 Jun 2003 06:04:57 -0000	1.89
@@ -353,7 +353,8 @@
             raise MixFatalError("Bad owner (uid=%s) on directory %s"
                                 % (owner, d))
         if (mode & 02) and not (mode & stat.S_ISVTX):
-            raise MixFatalError("Bad mode (%o) on directory %s" %(mode, d))
+            raise MixFatalError("Bad mode (%o) on directory %s" %
+                                (mode&0777, d))
 
         if (mode & 020) and not (mode & stat.S_ISVTX):
             # FFFF We may want to give an even stronger error here.

Index: Main.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Main.py,v
retrieving revision 1.47
retrieving revision 1.48
diff -u -d -r1.47 -r1.48
--- Main.py	5 Jun 2003 18:41:40 -0000	1.47
+++ Main.py	6 Jun 2003 06:04:57 -0000	1.48
@@ -205,13 +205,9 @@
         printUsage()
         sys.exit(1)
 
-#    if args[1] not in ('unittests', 'benchmarks'):
-#        print "==========================================================="
-#        print "                     TURN  BACK  NOW  !!!"
-#        print "This version of Mixminion (0.0.4alpha3) is compatible with no"
-#        print "other version.  Go check out the maintenance branch if you"
-#        print "want to use this software to run a server or send messages."
-#        print "==========================================================="
+    if args[1] not in ('unittests', 'benchmarks'):
+        print "This software is for testing purposes only."\
+              "  Anonymity is not guaranteed."
 
     # Read the 'common' module to get the UIError class.  To simplify
     # command implementation code, we catch all UIError exceptions here.

Index: Packet.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Packet.py,v
retrieving revision 1.45
retrieving revision 1.46
diff -u -d -r1.45 -r1.46
--- Packet.py	5 Jun 2003 05:24:23 -0000	1.45
+++ Packet.py	6 Jun 2003 06:04:57 -0000	1.46
@@ -403,7 +403,9 @@
     return block
 
 def _parseReplyBlock(s):
-    """DOCDOC"""
+    """Helper function: Given a string containing one or more (binary) reply
+       blocks, extract the first one.  On success, return a tuple containing
+       a ReplyBlock, and the length of 's' taken up by the reply block."""
     if len(s) < MIN_RB_LEN:
         raise ParseError("Reply block too short")
     try:
@@ -556,19 +558,23 @@
 #----------------------------------------------------------------------
 # Ascii-encoded packets
 #
-# The format is HeaderLine, TagLine?, Body, FooterLine.
-#     TagLine is one of /Message-type: (overcompressed|binary)/
-#                    or /Decoding-handle: (base64-encoded-stuff)/.
-#DOCDOC
+# A packet is an ASCII-armored object, with the following headers:
+#     Message-type: (plaintext|encrypted|binary|overcompressed)
+#    [Decoding-handle: base64-encoded-stuff]
+#
+# If Message-type is 'plaintext', the body is dash-escaped.  Otherwise,
+# the body is base64-encoded.
 
 MESSAGE_ARMOR_NAME = "TYPE III ANONYMOUS MESSAGE"
  
 def parseTextEncodedMessages(msg,force=0):
-    """Given a text-encoded Type III packet, return a TextEncodedMessage
-       object or raise ParseError. DOCDOC
+
+    """Given a text-encoded Type III packet, return a list of
+       TextEncodedMessage objects or raise ParseError.
+       
           force -- uncompress the message even if it's overcompressed.
-          idx -- index within <msg> to search.
     """
+    
     def isBase64(t,f):
         for k,v in f:
             if k == "Message-type":

Index: ServerInfo.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/ServerInfo.py,v
retrieving revision 1.47
retrieving revision 1.48
diff -u -d -r1.47 -r1.48
--- ServerInfo.py	5 Jun 2003 18:41:40 -0000	1.47
+++ ServerInfo.py	6 Jun 2003 06:04:57 -0000	1.48
@@ -309,7 +309,11 @@
        become very inefficient when directories get big, but we won't have
        that problem for a while."""
     ##Fields:
-    # servers: list of validated ServerInfo objects, in no particular order.
+    # allServers: list of validated ServerInfo objects, in no particular order.
+    # servers: sub-list of self.allServers, containing all of the servers
+    #    that are recommended.
+    # goodServerNames: list of lowercased nicknames for the recommended
+    #    servers in this directory.
     # header: a _DirectoryHeader object for the non-serverinfo part of this
     #    directory.
     def __init__(self, string=None, fname=None, validatedDigests=None):
@@ -338,26 +342,28 @@
         servercontents = [ "[Server]\n%s"%s for s in sections[1:] ]
 
         self.header = _DirectoryHeader(headercontents, digest)
-        #DOCDOC
-        self.goodServers = [name.strip().lower() for name in
+        self.goodServerNames = [name.strip().lower() for name in
                    self.header['Directory']['Recommended-Servers'].split(",")]
         servers = [ ServerInfo(string=s,
                                validatedDigests=validatedDigests)
                     for s in servercontents ]
-        self.allServers = servers[:] #DOCDOC
+        self.allServers = servers[:]
         self.servers = [ s for s in servers
-                         if s.getNickname().lower() in self.goodServers ]
+                         if s.getNickname().lower() in self.goodServerNames ]
 
     def getServers(self):
-        """Return a list of ServerInfo objects in this directory"""
+        """Return a list of recommended ServerInfo objects in this directory"""
         return self.servers
 
     def getAllServers(self):
-        """DOCDOC"""
+        """Return a list of all (even unrecommended) ServerInfo objects in
+           this directory."""
         return self.allServers
 
     def getRecommendedNicknames(self):
-        return self.goodServers
+        """Return a list of the (lowercased) nicknames of all of the
+           recommended servers in this directory."""
+        return self.goodServerNames
 
     def __getitem__(self, item):
         return self.header[item]

Index: test.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/test.py,v
retrieving revision 1.118
retrieving revision 1.119
diff -u -d -r1.118 -r1.119
--- test.py	5 Jun 2003 18:41:40 -0000	1.118
+++ test.py	6 Jun 2003 06:04:58 -0000	1.119
@@ -5864,8 +5864,9 @@
     LOG.setMinSeverity("FATAL")
     mixminion.Common.secureDelete([],1)
 
-    #DOCDOC
+    # Don't complain about owner on /tmp, no matter who is is.
     mixminion.Common._VALID_DIRECTORIES["/tmp"] = 1
+    mixminion.Common._VALID_DIRECTORIES["/var/tmp"] = 1
 
     # Disable TRACE and DEBUG log messages, unless somebody overrides from
     # the environment.