Permalink
Browse files

various cleanups, fixed a regression with carbon-client.py, added mor…

…e secure unpickling support and USE_INSECURE_UNPICKLER option
  • Loading branch information...
1 parent 540d259 commit dd8c476c7756b44822a15f66df55c51ec8b1e4c8 @cdavis cdavis committed Oct 6, 2011
@@ -80,7 +80,6 @@
log.setDebugEnabled(True)
defer.setDebugging(True)
-#TODO(chrismd) make this more configurable via options
if options.routing == 'consistent-hashing':
router = ConsistentHashingRouter(options.replication)
elif options.routing == 'relay':
@@ -99,8 +98,6 @@
firstConnectAttempts = [client_manager.startClient(dest) for dest in destinations]
firstConnectsAttempted = defer.DeferredList(firstConnectAttempts)
-events.metricReceived.addHandler(client_manager.sendDatapoint)
-
class StdinMetricsReader(LineReceiver):
delimiter = '\n'
@@ -69,6 +69,11 @@ UDP_RECEIVER_PORT = 2003
PICKLE_RECEIVER_INTERFACE = 0.0.0.0
PICKLE_RECEIVER_PORT = 2004
+# Per security concerns outlined in Bug #817247 the pickle receiver
+# will use a more secure and slightly less efficient unpickler.
+# Set this to True to revert to the old-fashioned insecure unpickler.
+USE_INSECURE_UNPICKLER = False
+
CACHE_QUERY_INTERFACE = 0.0.0.0
CACHE_QUERY_PORT = 7002
@@ -1,14 +1,10 @@
-try:
- import cPickle as pickle
-except ImportError:
- import pickle
-
from twisted.application.service import Service
from twisted.internet import reactor
from twisted.internet.defer import Deferred, DeferredList
from twisted.internet.protocol import ReconnectingClientFactory
from twisted.protocols.basic import Int32StringReceiver
from carbon.conf import settings
+from carbon.util import pickle
from carbon import log, state, events, instrumentation
@@ -26,9 +22,9 @@ def connectionMade(self):
self.queuedUntilReady = 'destinations.%s.queuedUntilReady' % self.destinationName
self.sent = 'destinations.%s.sent' % self.destinationName
- self.sendQueued()
self.factory.connectionMade.callback(self)
self.factory.connectionMade = Deferred()
+ self.sendQueued()
def connectionLost(self, reason):
log.clients("%s::connectionLost %s" % (self, reason.getErrorMessage()))
@@ -242,8 +238,5 @@ def sendDatapoint(self, metric, datapoint):
for destination in self.router.getDestinations(metric):
self.client_factories[destination].sendDatapoint(metric, datapoint)
- def whenClientQueueEmpty(self, destination):
- return self.client_factories[destination].queueEmpty
-
def __str__(self):
return "<%s[%x]>" % (self.__class__.__name__, id(self))
@@ -58,6 +58,7 @@
REPLICATION_FACTOR=1,
DESTINATIONS=[],
USE_FLOW_CONTROL=True,
+ USE_INSECURE_UNPICKLER=False,
)
@@ -3,11 +3,8 @@
from twisted.internet.error import ConnectionDone
from twisted.protocols.basic import LineOnlyReceiver, Int32StringReceiver
from carbon import log, events, state, management
-
-try:
- import cPickle as pickle
-except ImportError:
- import pickle
+from carbon.conf import settings
+from carbon.util import pickle, get_unpickler
class MetricReceiver:
@@ -82,9 +79,13 @@ def datagramReceived(self, data, (host, port)):
class MetricPickleReceiver(MetricReceiver, Int32StringReceiver):
MAX_LENGTH = 2 ** 20
+ def connectionMade(self):
+ MetricReceiver.connectionMade(self)
+ self.unpickler = get_unpickler(insecure=settings.USE_INSECURE_UNPICKLER)
+
def stringReceived(self, data):
try:
- datapoints = pickle.loads(data)
+ datapoints = self.unpickler.loads(data)
except:
log.listener('invalid pickle received from %s, ignoring' % self.peerName)
return
@@ -103,6 +104,7 @@ def connectionMade(self):
peer = self.transport.getPeer()
self.peerAddr = "%s:%d" % (peer.host, peer.port)
log.query("%s connected" % self.peerAddr)
+ self.unpickler = get_unpickler(insecure=settings.USE_INSECURE_UNPICKLER)
def connectionLost(self, reason):
if reason.check(ConnectionDone):
@@ -111,7 +113,7 @@ def connectionLost(self, reason):
log.query("%s connection lost: %s" % (self.peerAddr, reason.value))
def stringReceived(self, rawRequest):
- request = pickle.loads(rawRequest)
+ request = self.unpickler.loads(rawRequest)
if request['type'] == 'cache-query':
metric = request['metric']
datapoints = MetricCache.get(metric, [])
@@ -17,13 +17,9 @@
from os.path import join, exists
from carbon.conf import OrderedConfigParser, settings
+from carbon.util import pickle
from carbon import log
-try:
- import cPickle as pickle
-except ImportError:
- import pickle
-
STORAGE_SCHEMAS_CONFIG = join(settings.CONF_DIR, 'storage-schemas.conf')
STORAGE_AGGREGATION_CONFIG = join(settings.CONF_DIR, 'storage-aggregation.conf')
View
@@ -1,7 +1,18 @@
+import sys
import os
import pwd
from os.path import abspath, basename, dirname, join
+try:
+ from cStringIO import StringIO
+except ImportError:
+ from StringIO import StringIO
+try:
+ import cPickle as pickle
+ USING_CPICKLE = True
+except:
+ import pickle
+ USING_CPICKLE = False
from twisted.python.util import initgroups
from twisted.scripts.twistd import runApp
@@ -97,3 +108,59 @@ def parseDestinations(destination_strings):
destinations.append( (server, int(port), instance) )
return destinations
+
+
+
+# This whole song & dance is due to pickle being insecure
+# yet performance critical for carbon. We leave the insecure
+# mode (which is faster) as an option (USE_INSECURE_UNPICKLER).
+# The SafeUnpickler classes were largely derived from
+# http://nadiana.com/python-pickle-insecure
+if USING_CPICKLE:
+ class SafeUnpickler(object):
+ PICKLE_SAFE = {
+ 'copy_reg' : set(['_reconstructor']),
+ '__builtin__' : set(['object']),
+ }
+
+ @classmethod
+ def find_class(cls, module, name):
+ if not module in cls.PICKLE_SAFE:
+ raise pickle.UnpicklingError('Attempting to unpickle unsafe module %s' % module)
+ __import__(module)
+ mod = sys.modules[module]
+ if not name in cls.PICKLE_SAFE[module]:
+ raise pickle.UnpicklingError('Attempting to unpickle unsafe class %s' % name)
+ return getattr(mod, name)
+
+ @classmethod
+ def loads(cls, pickle_string):
+ pickle_obj = pickle.Unpickler(StringIO(pickle_string))
+ pickle_obj.find_global = cls.find_class
+ return pickle_obj.load()
+
+else:
+ class SafeUnpickler(pickle.Unpickler):
+ PICKLE_SAFE = {
+ 'copy_reg' : set(['_reconstructor']),
+ '__builtin__' : set(['object']),
+ }
+ def find_class(self, module, name):
+ if not module in self.PICKLE_SAFE:
+ raise pickle.UnpicklingError('Attempting to unpickle unsafe module %s' % module)
+ __import__(module)
+ mod = sys.modules[module]
+ if not name in self.PICKLE_SAFE[module]:
+ raise pickle.UnpicklingError('Attempting to unpickle unsafe class %s' % name)
+ return getattr(mod, name)
+
+ @classmethod
+ def loads(cls, pickle_string):
+ return cls(StringIO(pickle_string)).load()
+
+
+def get_unpickler(insecure=False):
+ if insecure:
+ return pickle
+ else:
+ return SafeUnpickler
@@ -113,8 +113,8 @@ def writeCachedDataPoints():
dbDir = dirname(dbFilePath)
os.system("mkdir -p -m 755 '%s'" % dbDir)
- log.creates("creating database file %s" % dbFilePath)
- log.creates(" %s, %s, %s" % (archiveConfig, xFilesFactor, aggregationMethod))
+ log.creates("creating database file %s (archive=%s xff=%s agg=%s)" %
+ (dbFilePath, archiveConfig, xFilesFactor, aggregationMethod))
whisper.create(dbFilePath, archiveConfig, xFilesFactor, aggregationMethod)
os.chmod(dbFilePath, 0755)
instrumentation.increment('creates')
View
@@ -33,7 +33,7 @@
setup(
name='graphite-web',
- version='0.9.9-pre5',
+ version='0.9.9',
url='https://launchpad.net/graphite',
author='Chris Davis',
author_email='chrismd@gmail.com',
@@ -16,11 +16,11 @@
import sys, os
from os.path import join, dirname, abspath
+WEBAPP_VERSION = '0.9.9'
DEBUG = False
JAVASCRIPT_DEBUG = False
# Filesystem layout (all directores should end in a /)
-WEBAPP_VERSION='0.9.9-pre5'
WEB_DIR = dirname( abspath(__file__) ) + '/'
WEBAPP_DIR = dirname( dirname(WEB_DIR) ) + '/'
GRAPHITE_ROOT = dirname( dirname(WEBAPP_DIR) ) + '/'

0 comments on commit dd8c476

Please sign in to comment.