[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[minion-cvs] A few misc hacks,tests,cleanups.
Update of /home/minion/cvsroot/src/minion/lib/mixminion
In directory moria.seul.org:/tmp/cvs-serv1689/lib/mixminion
Modified Files:
Common.py Config.py Crypto.py MMTPServer.py Modules.py
Packet.py Queue.py test.py
Log Message:
A few misc hacks,tests,cleanups.
TODO: Finish applying changes
Config:
No longer lowercase commands.
Remove dead-code from the old incarnation of configuration
Use socket.inet_aton for parsing IPs.
Common:
Refactor and debug createPrivateDir
Add 'log-that-exception!' method to Log.
Crypto, Queue, Modules:
Mark abstract methods with NotImplementedError
MMTPServer:
Listen on 0.0.0.0, not 127.0.0.1.
Modules:
Use Log.error_exc method
Packet:
Use socket.inet_ntoa and socket.inet_aton in place of _packIP and
_unpackIP.
test:
Add tests for Log.error_exc
Add tests for create/checkPrivateDir
Reorder tests to put slowest tests last
Suppress the you-don't-have-shred error while testing.
Index: Common.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Common.py,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -r1.14 -r1.15
--- Common.py 19 Aug 2002 20:27:02 -0000 1.14
+++ Common.py 21 Aug 2002 19:09:48 -0000 1.15
@@ -14,6 +14,7 @@
import sys
import time
import stat
+import traceback
from types import StringType
class MixError(Exception):
@@ -46,7 +47,6 @@
"Compute floor(a / b). See comments for portability notes."
return divmod(a,b)[0]
-
def ceilDiv(a,b):
"Compute ceil(a / b). See comments for portability notes."
return divmod(a-1,b)[0]+1
@@ -57,39 +57,57 @@
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)
+ raise MixFatalError("Nonexistent directory %s" % d)
try:
os.makedirs(d, 0700)
except OSError, e:
- getLog().fatal("Unable to create directory %s"%d)
- raise MixFatalError()
- elif not os.path.isdir(d):
- 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()
+ raise MixFatalError("Unable to create directory %s" % d)
- # Check permissions on parents.
+ checkPrivateDir(d)
+
+def checkPrivateDir(d, recurse=1):
+ """Return true iff d is a directory owned by this uid, set to mode
+ 0700. All of d's parents must not be writable or owned by anybody but
+ this uid and uid 0. If any of these conditions are unmet, raise
+ MixFatalErrror. Otherwise, return None."""
me = os.getuid()
+
+ if not os.path.exists(d):
+ raise MixFatalError("Directory %s does not exist" % d)
+ if not os.path.isdir(d):
+ raise MixFatalError("%s is not a directory" % d)
+
+ st = os.stat(d)
+ # check permissions
+ if st[stat.ST_MODE] & 0777 != 0700:
+ raise MixFatalError("Directory %s must be mode 0700" % d)
+
+ if st[stat.ST_UID] != me:
+ raise MixFatalError("Directory %s has must have owner %s" %(d, me))
+
+ if not recurse:
+ return
+
+ # Check permissions on parents.
while 1:
+ parent = os.path.split(d)[0]
+ if parent == d:
+ return
+ d = parent
+
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
+ raise MixFatalError("Bad owner (uid=%s) on directory %s"
+ % (owner, d))
if (mode & 02) and not (mode & stat.S_ISVTX):
- getLog().fatal("Bad mode (%o) on directory %s", mode, d)
+ raise MixFatalError("Bad mode (%o) on directory %s" %(mode, d))
- parent, _ = os.path.split(d)
- if parent == d:
- return
- d = parent
+ if (mode & 020) and not (mode & stat.S_ISVTX):
+ # FFFF We may want to give an even stronger error here.
+ getLog().warn("Iffy mode %o on directory %s (Writable by gid %s)",
+ mode, d, st[stat.ST_GID])
#----------------------------------------------------------------------
# Secure filesystem operations.
@@ -276,6 +294,9 @@
h.close()
def log(self, severity, message, *args):
+ self._log(severity, message, args)
+
+ def _log(self, severity, message, args):
if _SEVERITIES.get(severity, 100) < self.severity:
return
m = message % args
@@ -294,6 +315,21 @@
self.log("ERROR", message, *args)
def fatal(self, message, *args):
self.log("FATAL", message, *args)
+ def error_exc(self, (exclass, ex, tb), message=None, *args):
+ if message is not None:
+ self.log("ERROR", message, *args)
+ elif tb is not None:
+ filename = tb.tb_frame.f_code.co_filename
+ self.log("ERROR", "Unexpected exception in %s", filename)
+ else:
+ self.log("ERROR", "Unexpected exception")
+
+ formatted = traceback.format_exception(exclass, ex, tb)
+ formatted[1:] = [ " %s" % line for line in formatted[1:] ]
+ indented = "".join(formatted)
+ if indented.endswith('\n'):
+ indented = indented[:-1]
+ self._log("ERROR", indented, ())
_THE_LOG = None
def getLog():
Index: Config.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Config.py,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -d -r1.10 -r1.11
--- Config.py 19 Aug 2002 20:27:02 -0000 1.10
+++ Config.py 21 Aug 2002 19:09:48 -0000 1.11
@@ -53,6 +53,7 @@
import binascii
import time
import copy
+import socket # for inet_aton and error
from cStringIO import StringIO
import mixminion.Common
@@ -60,32 +61,6 @@
import mixminion.Packet
import mixminion.Crypto
-#----------------------------------------------------------------------
-
-# global variable to hold the configuration object for this process.
-_theConfiguration = None
-
-def loadConfig(fname,server=0):
- """Load the configuration file for this process. Takes a
- filename, and a flag to determine whether we're running as a
- client or a server.
-
- Registers the configuration object to be reloaded on SIGHUP."""
- global _theConfiguration
- assert _theConfiguration is None
-
- if server:
- _theConfiguration = ServerConfig(fname)
- else:
- assert fname is not None
- _theConfiguration = ClientConfig(fname)
-
-def getConfig():
- """Return the configuration object for this process, or None if we haven't
- been configured yet."""
- return _theConfiguration
-#----------------------------------------------------------------------
-
class ConfigError(MixError):
"""Thrown when an error is found in a configuration file."""
pass
@@ -156,13 +131,21 @@
except ValueError, e:
raise ConfigError("Expected an integer but got %r" % (integer))
+_ip_re = re.compile(r'\d+\.\d+\.\d+\.\d+')
+
def _parseIP(ip):
"""Validation function. Converts a config value to an IP address.
Raises ConfigError on failure."""
- i = ip.strip().lower()
+ i = ip.strip()
+
+ # inet_aton is a bit more permissive about spaces and incomplete
+ # IP's than we want to be. Thus we use a regex to catch the cases
+ # it doesn't.
+ if not _ip_re.match(i):
+ raise ConfigError("Invalid IP %r" % i)
try:
- f = mixminion.Packet._packIP(i)
- except mixminion.Packet.ParseError, p:
+ f = socket.inet_aton(i)
+ except socket.error, ex:
raise ConfigError("Invalid IP %r" % i)
return i
@@ -213,7 +196,7 @@
def _parseCommand(command):
"""Validation function. Converts a config value to a shell command of
the form (fname, optionslist). Raises ConfigError on failure."""
- c = command.strip().lower().split()
+ c = command.strip().split()
if not c:
raise ConfigError("Invalid command %r" %command)
cmd, opts = c[0], c[1:]
@@ -675,6 +658,7 @@
'Publish' : ('ALLOW', _parseBoolean, "no"),
'MaxSkew' : ('ALLOW', _parseInterval,
"10 minutes",) },
+ # FFFF Generic multi-port listen/publish options.
'Incoming/MMTP' : { 'Enabled' : ('REQUIRE', _parseBoolean, "no"),
'IP' : ('ALLOW', _parseIP, None),
'Port' : ('ALLOW', _parseInt, "48099"),
Index: Crypto.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Crypto.py,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -d -r1.15 -r1.16
--- Crypto.py 19 Aug 2002 20:27:02 -0000 1.15
+++ Crypto.py 21 Aug 2002 19:09:48 -0000 1.16
@@ -452,7 +452,7 @@
def _prng(self, n):
"""Abstract method: Must be overridden to return n bytes of fresh
entropy."""
- raise MixFatalError()
+ raise NotImplementedError("_prng")
class AESCounterPRNG(RNG):
'''Pseudorandom number generator that yields an AES counter-mode cipher'''
Index: MMTPServer.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/MMTPServer.py,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -d -r1.12 -r1.13
--- MMTPServer.py 19 Aug 2002 20:27:02 -0000 1.12
+++ MMTPServer.py 21 Aug 2002 19:09:48 -0000 1.13
@@ -621,7 +621,7 @@
self.context = config.getTLSContext(server=1)
# FFFF Don't always listen; don't always retransmit!
# FFFF Support listening on specific IPs
- self.listener = ListenConnection("127.0.0.1",
+ self.listener = ListenConnection("0.0.0.0",
config['Outgoing/MMTP']['Port'],
LISTEN_BACKLOG,
self._newMMTPConnection)
@@ -654,3 +654,4 @@
def onMessageSent(self, msg, handle):
pass
+
Index: Modules.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Modules.py,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- Modules.py 19 Aug 2002 20:27:02 -0000 1.6
+++ Modules.py 21 Aug 2002 19:09:48 -0000 1.7
@@ -55,7 +55,7 @@
def getConfigSyntax(self):
"""Return a map from section names to section syntax, as described
in Config.py"""
- pass
+ raise NotImplementedError("getConfigSyntax")
def validateConfig(self, sections, entries, lines, contents):
"""See mixminion.Config.validate"""
@@ -64,21 +64,21 @@
def configure(self, config, manager):
"""Configure this object using a given Config object, and (if
required) register it with the module manager."""
- pass
+ raise NotImplementedError("configure")
def getServerInfoBlock(self):
"""Return a block for inclusion in a server descriptor."""
- pass
+ raise NotImplementedError("getServerInfoBlock")
def getName(self):
"""Return the name of this module. This name may be used to construct
directory paths, so it shouldn't contain any funny characters."""
- pass
+ raise NotImplementedError("getName")
def getExitTypes(self):
"""Return a sequence of numeric exit types that this module can
handle."""
- pass
+ raise NotImplementedError("getExitTypes")
def createDeliveryQueue(self, queueDir):
"""Return a DeliveryQueue object suitable for delivering messages
@@ -94,7 +94,7 @@
DELIVER_FAIL_RETRY (if the message wasn't delivered, but might be
deliverable later), or
DELIVER_FAIL_NORETRY (if the message shouldn't be tried later)."""
- pass
+ raise NotImplementedError("processMessage")
class _ImmediateDeliveryQueue:
"""Helper class usable as delivery queue for modules that don't
@@ -113,12 +113,11 @@
else:
getLog().error("Unable to deliver message")
except:
- _, e, tb = sys.exc_info()
- getLog().error(
- "Exception delivering message: %s at line %s of %s",
- e, tb.tb_lineno, tb.tb_frame.f_code.co_name)
+ getLog().error_exc(sys.exc_info(),
+ "Exception delivering message")
def sendReadyMessages(self):
+ # We do nothing here; we already delivered the messages
pass
class _SimpleModuleDeliveryQueue(mixminion.Queue.DeliveryQueue):
@@ -142,10 +141,8 @@
getLog().error("Unable to deliver message")
self.deliveryFailed(handle, 0)
except:
- _, e, tb = sys.exc_info()
- getLog().error(
- "Exception delivering message: %s at line %s of %s",
- e, tb.tb_lineno, tb.tb_frame.f_code.co_name)
+ getLog().error_exc(sys.exc_info(),
+ "Exception delivering message")
self.deliveryFailed(handle, 0)
class ModuleManager:
Index: Packet.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Packet.py,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -d -r1.7 -r1.8
--- Packet.py 11 Aug 2002 07:50:34 -0000 1.7
+++ Packet.py 21 Aug 2002 19:09:48 -0000 1.8
@@ -15,6 +15,7 @@
'SECRET_LEN']
import struct
+from socket import inet_ntoa, inet_aton
from mixminion.Common import MixError, floorDiv
# Major and minor number for the understood packet format.
@@ -272,26 +273,6 @@
self.header, len(self.routingInfo),
self.routingType)+self.routingInfo
-def _packIP(s):
- """Helper method: convert a dotted-decimal IPv4 address into a
- four-byte encoding. Raises ParseError if the input is not a
- dotted quad of 0..255"""
- addr = s.split(".")
- if len(addr) != 4:
- raise ParseError("Malformed IP address")
- try:
- addr = map(int, addr)
- except ValueError:
- raise ParseError("Malformed IP address")
- for i in addr:
- if not (0 <= i <= 255): raise ParseError("Malformed IP address")
- return struct.pack("!BBBB", *addr)
-
-def _unpackIP(s):
- """Helper method: convert a four-byte IPv4 address into a dotted quad."""
- if len(s) != 4: raise ParseError("Malformed IP")
- return ".".join(map(str, struct.unpack("!BBBB", s)))
-
# An IPV4 address (Used by SWAP_FWD and FWD) is packed as: four bytes
# of IP address, a short for the portnum, and DIGEST_LEN bytes of keyid.
IPV4_PAT = "!4sH%ds" % DIGEST_LEN
@@ -305,7 +286,7 @@
ip, port, keyinfo = struct.unpack(IPV4_PAT, s)
except struct.error:
raise ParseError("Misformatted IPV4 routing info")
- ip = _unpackIP(ip)
+ ip = inet_ntoa(ip)
return IPV4Info(ip, port, keyinfo)
class IPV4Info:
@@ -323,7 +304,8 @@
def pack(self):
"""Return the routing info for this address"""
assert len(self.keyinfo) == DIGEST_LEN
- return struct.pack(IPV4_PAT, _packIP(self.ip), self.port, self.keyinfo)
+ return struct.pack(IPV4_PAT, inet_aton(self.ip),
+ self.port, self.keyinfo)
def __hash__(self):
return hash(self.pack())
Index: Queue.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/Queue.py,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -d -r1.12 -r1.13
--- Queue.py 19 Aug 2002 20:27:02 -0000 1.12
+++ Queue.py 21 Aug 2002 19:09:48 -0000 1.13
@@ -325,7 +325,8 @@
# We could implement this as a single deliverMessage(h,addr,m,n)
# method, but that wouldn't allow implementations to batch
# messages being sent to the same address.
- assert 0
+
+ raise NotImplementedError("deliverMessages")
def deliverySucceeded(self, handle):
"""Removes a message from the outgoing queue. This method
Index: test.py
===================================================================
RCS file: /home/minion/cvsroot/src/minion/lib/mixminion/test.py,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -d -r1.20 -r1.21
--- test.py 19 Aug 2002 20:27:02 -0000 1.20
+++ test.py 21 Aug 2002 19:09:48 -0000 1.21
@@ -497,7 +497,7 @@
import mixminion.Packet
from mixminion.Packet import *
-class FormatTests(unittest.TestCase):
+class PacketTests(unittest.TestCase):
def test_subheader(self):
s = Subheader(3,0,"abcdeabcdeabcdef",
"ABCDEFGHIJABCDEFGHIJ",
@@ -1302,7 +1302,7 @@
self.d3 = mix_mktemp("q3")
def testCreateQueue(self):
- # Nonexistant dir.
+ # Nonexistent dir.
self.failUnlessRaises(MixFatalError, Queue, self.d1)
# File in place of dir
f = open(self.d1, 'w')
@@ -1536,6 +1536,19 @@
self.failUnless(buf.getvalue().endswith(
"[ERROR] All your anonymity are belong to us\n"))
+ buf.truncate(0)
+ try:
+ 1/0
+ except:
+ inf = sys.exc_info()
+ log.error_exc(inf)
+ log.error_exc(inf, "And so on")
+ log.error_exc(inf, "And so %s", "on")
+
+ # print buf.getvalue()
+ # FFFF We should examine the value of the above, but inspection
+ # FFFF show that we're fine.
+
t = mix_mktemp("log")
t1 = t+"1"
@@ -1549,7 +1562,110 @@
log.close()
self.assertEquals(open(t).read().count("\n") , 1)
self.assertEquals(open(t1).read().count("\n"), 3)
-
+
+#----------------------------------------------------------------------
+# File paranoia
+from mixminion.Common import createPrivateDir, checkPrivateDir
+
+class FileParanoiaTests(unittest.TestCase):
+ def testPrivateDirs(self):
+ noia = mix_mktemp("noia")
+ try:
+ checkPrivateDir(_MM_TESTING_TEMPDIR)
+ except MixFatalError, e:
+ self.fail("Can't test directory paranoia, because something's\n"
+ +" wrong with %s: %s"%(_MM_TESTING_TEMPDIR,str(e)))
+
+ # Nonexistant directory.
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, noia)
+ # Bad permissions.
+ os.mkdir(noia)
+ os.chmod(noia, 0777)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, noia)
+ # Bad permissions on parent
+ subdir = os.path.join(noia, "subdir")
+ os.mkdir(subdir, 0700)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, subdir)
+ os.chmod(noia, 0755)
+ checkPrivateDir(subdir)
+ os.chmod(noia, 0700)
+ checkPrivateDir(subdir)
+ # Not writable by self
+ os.chmod(subdir, 0600)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, subdir)
+ # Not a directory
+ os.rmdir(subdir)
+ f = open(subdir,'w')
+ f.write("x")
+ f.close()
+ os.chmod(subdir, 0700)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, subdir)
+ os.unlink(subdir)
+ os.mkdir(subdir, 0700)
+
+ # Now we test a directory we don't own...
+ if os.getuid() == 0: # If we're root, we can play with chown!
+ # We don't own the directory
+ os.chown(subdir, 1, 1)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, subdir)
+ os.chown(subdir, 0, os.getgid())
+ # We don't own the parent
+ os.chown(noia, 1, 1)
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, subdir)
+ os.chown(noia, 0, os.getgid())
+ else:
+ # We're not root. We can't reliably find or make a directory
+ # that's non-root and non-us. Let's just make sure we don't
+ # own temp.
+ if os.path.exists("/tmp"):
+ self.failUnlessRaises(MixFatalError, checkPrivateDir, "/tmp")
+
+ # Helper fn: return mode,uid,isdir
+ def mud(f):
+ st = os.stat(f)
+ return st[stat.ST_MODE]&0777, st[stat.ST_UID], os.path.isdir(f)
+
+ # Okay. Now we try createPrivateDir a couple of times...
+ old_mask = None
+ try:
+ # Make sure umask is lenient, so we can tell whether c-p-d is
+ # strict.
+ old_mask = os.umask(022)
+ # 1. Create the world.
+ os.rmdir(subdir)
+ os.rmdir(noia)
+ createPrivateDir(subdir)
+ self.assertEquals((0700,os.getuid(),1), mud(subdir))
+ self.assertEquals((0700,os.getuid(),1), mud(noia))
+ # 2. Just create one dir.
+ os.rmdir(subdir)
+ os.chmod(noia, 0755)
+ createPrivateDir(subdir)
+ self.assertEquals((0700,os.getuid(),1), mud(subdir))
+ self.assertEquals((0755,os.getuid(),1), mud(noia))
+ # 3. Fail to create because of bad permissions
+ os.rmdir(subdir)
+ os.chmod(noia, 0777)
+ self.failUnlessRaises(MixFatalError, createPrivateDir, subdir)
+ # 4. Fail to create because of OSError
+ os.rmdir(subdir)
+ f = open(subdir, 'w')
+ f.write('W')
+ f.close()
+ self.failUnlessRaises(MixFatalError, createPrivateDir, subdir)
+ os.unlink(subdir)
+ # 5. Succeed: it's already there.
+ os.chmod(noia, 0700)
+ os.mkdir(subdir, 0700)
+ createPrivateDir(subdir)
+ # 6. Fail: it's already there, but has bad permissions
+ os.chmod(subdir, 0777)
+ self.failUnlessRaises(MixFatalError, createPrivateDir, subdir)
+ os.chmod(subdir, 0700)
+ finally:
+ if old_mask is not None:
+ os.umask(old_mask)
+
#----------------------------------------------------------------------
# SIGHANDLERS
# FFFF Write tests here
@@ -2260,24 +2376,31 @@
suite.addTest(tc(MinionlibCryptoTests))
suite.addTest(tc(CryptoTests))
- suite.addTest(tc(FormatTests))
+ suite.addTest(tc(PacketTests))
suite.addTest(tc(LogTests))
+ suite.addTest(tc(FileParanoiaTests))
suite.addTest(tc(ConfigFileTests))
- suite.addTest(tc(ServerInfoTests))
- suite.addTest(tc(ModuleManagerTests))
suite.addTest(tc(HashLogTests))
suite.addTest(tc(BuildMessageTests))
suite.addTest(tc(PacketHandlerTests))
suite.addTest(tc(QueueTests))
+
+ # These tests are slowest, so we do them last.
+ suite.addTest(tc(ModuleManagerTests))
+ suite.addTest(tc(ServerInfoTests))
suite.addTest(tc(MMTPTests))
return suite
def testAll():
+ # Suppress 'files-can't-be-securely-deleted message while testing'
+ getLog().setMinSeverity("FATAL")
+ mixminion.Common.secureDelete([],1)
+
# Disable TRACE and DEBUG log messages, unless somebody overrides from
# the environment.
getLog().setMinSeverity(os.environ.get('MM_TEST_LOGLEVEL', "WARN"))
- unittest.TextTestRunner().run(testSuite())
+ unittest.TextTestRunner(verbosity=1).run(testSuite())
if __name__ == '__main__':
init_crypto()