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

[minion-cvs] Resolve XXXXs where possible; clear small memory leak i...



Update of /home/minion/cvsroot/src/minion/lib/mixminion
In directory moria.seul.org:/tmp/cvs-serv28031/lib/mixminion

Modified Files:
	Common.py Config.py Crypto.py MMTPClient.py MMTPServer.py 
	Modules.py PacketHandler.py Queue.py test.py 
Log Message:
Resolve XXXXs where possible; clear small memory leak in c code.


Index: Common.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Common.py,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -d -r1.13 -r1.14
--- Common.py	19 Aug 2002 15:33:55 -0000	1.13
+++ Common.py	19 Aug 2002 20:27:02 -0000	1.14
@@ -52,12 +52,12 @@
     return divmod(a-1,b)[0]+1
 
 #----------------------------------------------------------------------
-def createPrivateDir(d):
+def createPrivateDir(d, nocreate=0):
     """Create a directory, and all parent directories, checking permissions
-       as we go along.  All superdirectories must be owned by root or us.
-       
-       XXXX we don't check permissions properly yet."""
+       as we go along.  All superdirectories must be owned by root or us."""
     if not os.path.exists(d):
+	if nocreate:
+	    raise MixFatalError("Nonexistant directory %s"%d)
 	try:
 	    os.makedirs(d, 0700)
 	except OSError, e:
@@ -67,11 +67,29 @@
         getLog().fatal("%s is not a directory"%d)
         raise MixFatalError()
     else:
-        m = os.stat(d)[stat.ST_MODE]
-        # check permissions
-        if m & 0077:
-            getLog().fatal("Directory %s must be mode 0700" %d)
-            raise MixFatalError()
+	m = os.stat(d)[stat.ST_MODE]
+	# check permissions
+	if m & 0077:
+	    getLog().fatal("Directory %s must be mode 0700" %d)
+	    raise MixFatalError()
+
+    # Check permissions on parents.
+    me = os.getuid()
+    while 1:
+	st = os.stat(d)
+	mode = st[stat.ST_MODE]
+	owner = st[stat.ST_UID]
+	if owner not in (0, me):
+	    getLog().fatal("Bad owner (uid=%s) on directory %s", owner, d)
+	    raise MixFatalError()
+	# FFFF Check group permissions
+	if (mode & 02) and not (mode & stat.S_ISVTX):
+	    getLog().fatal("Bad mode (%o) on directory %s", mode, d)
+
+	parent, _ = os.path.split(d)
+	if parent == d:
+	    return
+	d = parent
 
 #----------------------------------------------------------------------
 # Secure filesystem operations.
@@ -106,7 +124,7 @@
        return until the remove is complete.  If blocking=0, returns
        immediately, and returns the PID of the process removing the
        files.  (Returns None if this process unlinked the files
-       itself.) XXXX Clarify this.
+       itself.) 
 
        XXXX Securely deleting files only does so much good.  Metadata on
        XXXX the file system, such as atime and dtime, can still be used

Index: Config.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Config.py,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
--- Config.py	19 Aug 2002 15:33:55 -0000	1.9
+++ Config.py	19 Aug 2002 20:27:02 -0000	1.10
@@ -385,17 +385,22 @@
        avoidably longer than 'w' characters, and with continuation lines
        indented by 'ind' spaces.
     """
-    ind = " "*ind
+    ind_s = " "*(ind-1)
     if len(str(val))+len(key)+2 <= 79:
         return "%s: %s\n" % (key,val)
 
-    lines = [ "%s: " %key ]
-    #XXXX Bad implementation.
+    lines = [  ]
+    linecontents = [ "%s:" % key ]
+    linelength = len(linecontents[0])
     for v in val.split(" "):
-        if len(lines[-1])+1+len(v) <= w:
-            lines[-1] = "%s %s" % (lines[-1],v)
+        if linelength+1+len(v) <= w:
+            linescontents.append(v)
+	    linelength += 1+len(v)
         else:
-            lines.append(ind+v)
+	    lines.append(" ".join(linecontents))
+	    linecontents = [ ind_s, v ]
+	    linelength = ind+len(v)
+    lines.append(" ".join(linecontents))
     lines.append("") # so the last line ends with \n
     return "\n".join(lines)
 
@@ -574,13 +579,6 @@
                 self_sectionEntries[secName] = {}
                 
         if not self.assumeValid:
-            # Make sure that sectionEntries is correct (sanity check)
-            #XXXX remove this
-            for s in self_sectionNames:
-                for k,v in self_sectionEntries[s]:
-                    assert (v==self_sections[s][k] or
-                            v in self_sections[s][k])
-                    
             # Call our validation hook.
             self.validate(self_sections, self_sectionEntries, 
                           sectionEntryLines, fileContents)
@@ -685,6 +683,10 @@
         'Outgoing/MMTP' : { 'Enabled' : ('REQUIRE', _parseBoolean, "no"),
                             'Allow' : ('ALLOW*', _parseAddressSet_allow, None),
                             'Deny' : ('ALLOW*', _parseAddressSet_deny, None) },
+	# FFFF Missing: Queue-Size / Queue config options
+	# FFFF         timeout options
+	# FFFF         listen timeout??
+	# FFFF         Retry options
         }
 
 class ServerConfig(_ConfigFile):
@@ -693,10 +695,6 @@
     #   moduleManager
     #
     _restrictFormat = 0
-    # XXXX Missing: Queue-Size / Queue config options
-    # XXXX         timeout options
-    # XXXX         listen timeout??
-    # XXXX         Retry options
     def __init__(self, fname=None, string=None, moduleManager=None):
 	# We use a copy of SERVER_SYNTAX, because the ModuleManager will
 	# mess it up.

Index: Crypto.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Crypto.py,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -r1.14 -r1.15
--- Crypto.py	19 Aug 2002 15:33:56 -0000	1.14
+++ Crypto.py	19 Aug 2002 20:27:02 -0000	1.15
@@ -11,8 +11,6 @@
 import os
 import sys
 import stat
-# XXXX is this used?
-import struct
 from types import StringType
 
 import mixminion.Config
@@ -138,7 +136,7 @@
                  "must guard even his enemy from oppression."
 
 def pk_encrypt(data,key):
-    """Returns the RSA encryption of OAEP-padded data, using the public key
+    """Return the RSA encryption of OAEP-padded data, using the public key
        in key.
     """
     bytes = key.get_modulus_bytes()
@@ -147,14 +145,15 @@
     return key.crypt(data, 1, 1)
 
 def pk_sign(data, key):
-    """XXXX"""
+    """Return the RSA signature of OAEP-padded data, using the public key
+       in key."""
     bytes = key.get_modulus_bytes()
     data = add_oaep(data,OAEP_PARAMETER,bytes)
     return key.crypt(data, 0, 1)
 
 def pk_decrypt(data,key):
-    """Returns the unpadded RSA decryption of data, using the private key in\n
-       key
+    """Returns the unpadded RSA decryption of data, using the private key in
+       key.
     """
     bytes = key.get_modulus_bytes()
     # private key decrypt
@@ -162,7 +161,9 @@
     return check_oaep(data,OAEP_PARAMETER,bytes)
 
 def pk_check_signature(data, key):
-    """XXXX"""
+    """If data holds the RSA signature of some OAEP-padded data, check the
+       signature using public key 'key', and return the orignal data.  
+       Throw CryptoError on failure. """
     bytes = key.get_modulus_bytes()
     # private key decrypt
     data = key.crypt(data, 1, 0)
@@ -363,8 +364,6 @@
 def lioness_keys_from_payload(payload):
     '''Given a payload, returns the LIONESS keys to encrypt the off-header
        at the swap point.''' 
-    
-    # XXXX Temporary method till George and I agree on a key schedule.
     digest = sha1(payload)
     return Keyset(digest).getLionessKeys(HIDE_HEADER_MODE)
 

Index: MMTPClient.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPClient.py,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- MMTPClient.py	6 Aug 2002 16:09:21 -0000	1.6
+++ MMTPClient.py	19 Aug 2002 20:27:02 -0000	1.7
@@ -12,11 +12,8 @@
    introduce messages into the system, and [B] so that we've got an
    easy-to-verify reference implementation of the protocol.)
 
-   XXXX We don't yet check for the correct keyid.
-
-   XXXX: As yet unsupported are: Session resumption and key renegotiation.
-
-   XXXX: Also unsupported: timeouts."""
+   FFFF: As yet unsupported are: Session resumption and key renegotiation.
+   FFFF: Also unsupported: timeouts."""
 
 import socket
 import mixminion._minionlib as _ml

Index: MMTPServer.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPServer.py,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -d -r1.11 -r1.12
--- MMTPServer.py	19 Aug 2002 15:33:56 -0000	1.11
+++ MMTPServer.py	19 Aug 2002 20:27:02 -0000	1.12
@@ -9,10 +9,8 @@
 
    If you just want to send messages into the system, use MMTPClient.
 
-   XXXX As yet unsupported are: Session resumption, key renegotiation,
-   XXXX checking KeyID.
-
-   XXXX: Also unsupported: timeouts."""
+   FFFF As yet unsupported are: Session resumption, key renegotiation,
+   FFFF: Also unsupported: timeouts."""
 
 # NOTE FOR THE CURIOUS: The 'asyncore' module in the standard library
 #    is another general select/poll wrapper... so why are we using our
@@ -430,7 +428,7 @@
         """
         trace("done w/ client sendproto")
         inp = self.getInput()
-        m =PROTOCOL_RE.match(inp)
+        m = PROTOCOL_RE.match(inp)
 
         if not m:
             warn("Bad protocol list.  Closing connection.")
@@ -438,7 +436,7 @@
         protocols = m.group(1).split(",")
         if "1.0" not in protocols:
             warn("Unsupported protocol list.  Closing connection.")
-            self.shutdown(err=1); return #XXXX
+            self.shutdown(err=1); return
         else:
             trace("proto ok.")
             self.finished = self.__sentProtocol
@@ -483,13 +481,13 @@
         """Called once we're done sending an ACK.  Begins reading a new
            message."""
         trace("done w/ send ack")
-        #XXXX Rehandshake
+        #FFFF Rehandshake
         self.finished = self.__receivedMessage
         self.expectRead(SEND_RECORD_LEN)
 
 #----------------------------------------------------------------------
-        
-# XXXX We need logic to do retires.
+
+# FFFF We need to note retriable situations better.
 class MMTPClientConnection(SimpleTLSConnection):
     def __init__(self, context, ip, port, keyID, messageList, handleList,
                  sentCallback=None, failCallback=None):
@@ -592,7 +590,7 @@
           Otherwise, begins shutting down.
        """
        trace("received ack")
-       #XXXX Rehandshake
+       # FFFF Rehandshake
        inp = self.getInput()
        if inp != (RECEIVED_CONTROL+self.expectedDigest):
 	   # We only get bad ACKs if an adversary somehow subverts TLS's
@@ -623,7 +621,6 @@
         self.context = config.getTLSContext(server=1)
 	# FFFF Don't always listen; don't always retransmit!
 	# FFFF Support listening on specific IPs
-	# FFFF File
         self.listener = ListenConnection("127.0.0.1",
                                          config['Outgoing/MMTP']['Port'],
 					 LISTEN_BACKLOG,
@@ -634,7 +631,7 @@
     def _newMMTPConnection(self, sock):
 	"""helper method.  Creates and registers a new server connection when
 	   the listener socket gets a hit."""
-        # XXXX Check whether incoming IP is allowed!
+        # FFFF Check whether incoming IP is allowed!
         tls = self.context.sock(sock, serverMode=1)
         sock.setblocking(0)
         con = MMTPServerConnection(sock, tls, self.onMessageReceived)

Index: Modules.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Modules.py,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -d -r1.5 -r1.6
--- Modules.py	19 Aug 2002 15:33:56 -0000	1.5
+++ Modules.py	19 Aug 2002 20:27:02 -0000	1.6
@@ -160,7 +160,7 @@
     # Fields
     #    syntax: extensions to the syntax configuration in Config.py
     #    modules: a list of DeliveryModule objects
-    #    nameToModule: XXXX Docdoc
+    #    nameToModule: Map from module name to module
     #    typeToModule: a map from delivery type to enabled deliverymodule.
     #    path: search path for python modules.
     #    queueRoot: directory where all the queues go.
@@ -301,8 +301,8 @@
 
 #----------------------------------------------------------------------
 class MBoxModule(DeliveryModule):
-    # XXXX This implementation can stall badly if we don't have a fast
-    # XXXX local MTA.
+    # FFFF This implementation can stall badly if we don't have a fast
+    # FFFF local MTA.
     def __init__(self):
         DeliveryModule.__init__(self)
         self.command = None
@@ -323,7 +323,7 @@
         pass
 
     def configure(self, config, moduleManager):
-        # XXXX Check this.  error handling
+        # XXXX Check this.  Conside error handling
 	
         self.enabled = config['Delivery/MBOX'].get("Enabled", 0)
 	if not self.enabled:
@@ -417,7 +417,7 @@
 	res = DELIVER_OK
     except smtplib.SMTPException, e:
 	getLog().warn("Unsuccessful smtp: "+str(e))
-	res = DELIVER_FAIL_RETRY #????
+	res = DELIVER_FAIL_RETRY
 
     con.quit()
     con.close()

Index: PacketHandler.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/PacketHandler.py,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -d -r1.5 -r1.6
--- PacketHandler.py	1 Jul 2002 18:03:05 -0000	1.5
+++ PacketHandler.py	19 Aug 2002 20:27:02 -0000	1.6
@@ -30,7 +30,6 @@
            though: PK decryption is expensive.  Also, a hashlog must be
            provided for each private key.
         """
-        # ???? Any way to support multiple keys in protocol?
         try:
             # Check whether we have a key or a tuple of keys.
             _ = privatekey[0]

Index: Queue.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Queue.py,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -d -r1.11 -r1.12
--- Queue.py	19 Aug 2002 15:33:56 -0000	1.11
+++ Queue.py	19 Aug 2002 20:27:02 -0000	1.12
@@ -78,18 +78,7 @@
         if os.path.exists(location) and not os.path.isdir(location):
             raise MixFatalError("%s is not a directory" % location)
 
-        if not os.path.exists(location):
-            if create:
-                getLog().info("Trying to create queue %s", location)
-		createPrivateDir(location)
-            else:
-                raise MixFatalError("No directory for queue %s" % location)
- 
-        # Check permissions
-        mode = os.stat(location)[stat.ST_MODE]
-        if mode & 0077:
-            # XXXX be more Draconian.
-            getLog().warn("Worrisome mode %o on directory %s", mode, location)
+	createPrivateDir(location, nocreate=(not create))
 
         if scrub:
             self.cleanQueue()

Index: test.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/test.py,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -d -r1.19 -r1.20
--- test.py	19 Aug 2002 15:33:56 -0000	1.19
+++ test.py	19 Aug 2002 20:27:02 -0000	1.20
@@ -57,14 +57,28 @@
 	    print "Couldn't create temp dir %r" %temp
 	    sys.exit(1)
 	st = os.stat(temp)
-	if paranoia and st[stat.ST_MODE] & 077:
-	    print "Couldn't create temp dir %r with secure permissions" %temp
-	    sys.exit(1)
-	if paranoia and st[stat.ST_UID] != os.getuid():
-	    print "The wrong user owns temp dir %r"%temp
-	    sys.exit(1)
-	# XXXX If we're on a bad system, the permissions on /tmp
-	# XXXX might be faulty.  We don't bother checking for that.
+	if paranoia:
+	    if st[stat.ST_MODE] & 077:
+		print "Couldn't make temp dir %r with secure permissions" %temp
+		sys.exit(1)
+	    if st[stat.ST_UID] != os.getuid():
+		print "The wrong user owns temp dir %r"%temp
+		sys.exit(1)
+	    parent = temp
+	    while 1:
+		p = os.path.split(parent)[0]
+		if parent == p:
+		    break
+		parent = p
+		st = os.stat(parent)
+		m = st[stat.ST_MODE]
+		if m & 02 and not (m & stat.S_ISVTX):
+		    print "Directory %s has fishy permissions %o" %(parent,m)
+		    sys.exit(1)
+		if st[stat.ST_UID] not in (0, os.getuid()):
+		    print "Directory %s has bad owner %s" % st[stat.UID]
+		    sys.exit(1)
+		    
 	_MM_TESTING_TEMPDIR = temp
 	if _MM_TESTING_TEMPDIR_REMOVE_ON_EXIT:
 	    import atexit
@@ -953,7 +967,7 @@
             ks = Keyset(s)
             p = lioness_decrypt(p,ks.getLionessKeys(PAYLOAD_ENCRYPT_MODE))
 
-        # ???? Need to do something about size encoding.
+        # FFFF Need to do something about size encoding.
         self.assertEquals(payload, p[:len(payload)])
 
 
@@ -1887,11 +1901,13 @@
         self.assertEquals(pa("192.168.0.1",0),
                           ("192.168.0.1", "255.255.255.255", 0, 65535))
 
-        # XXXX This won't work on Windows.
-        self.assertEquals(C._parseCommand("ls -l"), ("/bin/ls", ['-l']))
-        self.assertEquals(C._parseCommand("rm"), ("/bin/rm", []))
-        self.assertEquals(C._parseCommand("/bin/ls"), ("/bin/ls", []))
-        self.failUnless(C._parseCommand("python")[0] is not None)
+	if not sys.platform == 'win32':
+	    # XXXX This should get implemented for Windows.
+	    self.assertEquals(C._parseCommand("ls -l"), ("/bin/ls", ['-l']))
+	    self.assertEquals(C._parseCommand("rm"), ("/bin/rm", []))
+	    self.assertEquals(C._parseCommand("/bin/ls"), ("/bin/ls", []))
+	    self.failUnless(C._parseCommand("python")[0] is not None)
+
 	self.assertEquals(C._parseBase64(" YW\nJj"), "abc")
 	self.assertEquals(C._parseHex(" C0D0"), "\xC0\xD0")
 	tm = C._parseDate("30/05/2002")