Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Do not use pickle for serialization in memcache, but JSON

We don't want to use pickle as it can execute arbitrary code. JSON is
safer. However, note that it supports serialization for only some
specific subset of object types; this should be enough for what we need,
though.

To avoid issues on upgrades (unability to read pickled values, and cache
poisoning for old servers not understanding JSON), we add a
memcache_serialization_support configuration option, with the following
values:

 0 = older, insecure pickle serialization
 1 = json serialization but pickles can still be read (still insecure)
 2 = json serialization only (secure and the default)

To avoid an instant full cache flush, existing installations should
upgrade with 0, then set to 1 and reload, then after some time (24
hours) set to 2 and reload. Support for 0 and 1 will be removed in
future versions.

Part of bug 1006414.

Change-Id: Id7d6d547b103b4f23ebf5be98b88f09ec6027ce4
  • Loading branch information...
commit e1ff51c04554d51616d2845f92ab726cb0e5831a 1 parent ef3f8bb
Vincent Untz authored June 21, 2012
15  doc/manpages/proxy-server.conf.5
@@ -213,6 +213,21 @@ Logging address. The default is /dev/log.
213 213
 Enables the ability to log request headers. The default is False.
214 214
 .IP \fBmemcache_servers\fR
215 215
 If not set in the configuration file, the value for memcache_servers will be read from /etc/swift/memcache.conf (see memcache.conf-sample) or lacking that file, it will default to the value below. You can specify multiple servers separated with commas, as in: 10.1.2.3:11211,10.1.2.4:11211. This can be a list separated by commas. The default is 127.0.0.1:11211.
  216
+.IP \fBmemcache_serialization_support\fR
  217
+This sets how memcache values are serialized and deserialized:
  218
+.RE
  219
+
  220
+.PD 0
  221
+.RS 10
  222
+.IP "0 = older, insecure pickle serialization"
  223
+.IP "1 = json serialization but pickles can still be read (still insecure)"
  224
+.IP "2 = json serialization only (secure and the default)"
  225
+.RE
  226
+
  227
+.RS 10
  228
+To avoid an instant full cache flush, existing installations should upgrade with 0, then set to 1 and reload, then after some time (24 hours) set to 2 and reload. In the future, the ability to use pickle serialization will be removed.
  229
+
  230
+If not set in the configuration file, the value for memcache_serialization_support will be read from /etc/swift/memcache.conf if it exists (see memcache.conf-sample). Otherwise, the default value as indicated above will be used.
216 231
 .RE
217 232
 
218 233
 
10  etc/memcache.conf-sample
@@ -3,3 +3,13 @@
3 3
 # several other conf files under [filter:cache] for example. You can specify
4 4
 # multiple servers separated with commas, as in: 10.1.2.3:11211,10.1.2.4:11211
5 5
 # memcache_servers = 127.0.0.1:11211
  6
+#
  7
+# Sets how memcache values are serialized and deserialized:
  8
+# 0 = older, insecure pickle serialization
  9
+# 1 = json serialization but pickles can still be read (still insecure)
  10
+# 2 = json serialization only (secure and the default)
  11
+# To avoid an instant full cache flush, existing installations should
  12
+# upgrade with 0, then set to 1 and reload, then after some time (24 hours)
  13
+# set to 2 and reload.
  14
+# In the future, the ability to use pickle serialization will be removed.
  15
+# memcache_serialization_support = 2
12  etc/proxy-server.conf-sample
@@ -167,6 +167,18 @@ use = egg:swift#memcache
167 167
 # default to the value below. You can specify multiple servers separated with
168 168
 # commas, as in: 10.1.2.3:11211,10.1.2.4:11211
169 169
 # memcache_servers = 127.0.0.1:11211
  170
+#
  171
+# Sets how memcache values are serialized and deserialized:
  172
+# 0 = older, insecure pickle serialization
  173
+# 1 = json serialization but pickles can still be read (still insecure)
  174
+# 2 = json serialization only (secure and the default)
  175
+# If not set here, the value for memcache_serialization_support will be read
  176
+# from /etc/swift/memcache.conf (see memcache.conf-sample).
  177
+# To avoid an instant full cache flush, existing installations should
  178
+# upgrade with 0, then set to 1 and reload, then after some time (24 hours)
  179
+# set to 2 and reload.
  180
+# In the future, the ability to use pickle serialization will be removed.
  181
+# memcache_serialization_support = 2
170 182
 
171 183
 [filter:ratelimit]
172 184
 use = egg:swift#ratelimit
48  swift/common/memcached.py
@@ -27,11 +27,17 @@
27 27
 from bisect import bisect
28 28
 from hashlib import md5
29 29
 
  30
+try:
  31
+    import simplejson as json
  32
+except ImportError:
  33
+    import json
  34
+
30 35
 DEFAULT_MEMCACHED_PORT = 11211
31 36
 
32 37
 CONN_TIMEOUT = 0.3
33 38
 IO_TIMEOUT = 2.0
34 39
 PICKLE_FLAG = 1
  40
+JSON_FLAG = 2
35 41
 NODE_WEIGHT = 50
36 42
 PICKLE_PROTOCOL = 2
37 43
 TRY_COUNT = 3
@@ -57,7 +63,8 @@ class MemcacheRing(object):
57 63
     """
58 64
 
59 65
     def __init__(self, servers, connect_timeout=CONN_TIMEOUT,
60  
-                 io_timeout=IO_TIMEOUT, tries=TRY_COUNT):
  66
+                 io_timeout=IO_TIMEOUT, tries=TRY_COUNT,
  67
+                 allow_pickle=False, allow_unpickle=False):
61 68
         self._ring = {}
62 69
         self._errors = dict(((serv, []) for serv in servers))
63 70
         self._error_limited = dict(((serv, 0) for serv in servers))
@@ -69,6 +76,8 @@ def __init__(self, servers, connect_timeout=CONN_TIMEOUT,
69 76
         self._client_cache = dict(((server, []) for server in servers))
70 77
         self._connect_timeout = connect_timeout
71 78
         self._io_timeout = io_timeout
  79
+        self._allow_pickle = allow_pickle
  80
+        self._allow_unpickle = allow_unpickle or allow_pickle
72 81
 
73 82
     def _exception_occurred(self, server, e, action='talking'):
74 83
         if isinstance(e, socket.timeout):
@@ -130,16 +139,21 @@ def set(self, key, value, serialize=True, timeout=0):
130 139
 
131 140
         :param key: key
132 141
         :param value: value
133  
-        :param serialize: if True, value is pickled before sending to memcache
  142
+        :param serialize: if True, value is serialized with JSON before sending
  143
+                          to memcache, or with pickle if configured to use
  144
+                          pickle instead of JSON (to avoid cache poisoning)
134 145
         :param timeout: ttl in memcache
135 146
         """
136 147
         key = md5hash(key)
137 148
         if timeout > 0:
138 149
             timeout += time.time()
139 150
         flags = 0
140  
-        if serialize:
  151
+        if serialize and self._allow_pickle:
141 152
             value = pickle.dumps(value, PICKLE_PROTOCOL)
142 153
             flags |= PICKLE_FLAG
  154
+        elif serialize:
  155
+            value = json.dumps(value)
  156
+            flags |= JSON_FLAG
143 157
         for (server, fp, sock) in self._get_conns(key):
144 158
             try:
145 159
                 sock.sendall('set %s %d %d %s noreply\r\n%s\r\n' % \
@@ -151,8 +165,9 @@ def set(self, key, value, serialize=True, timeout=0):
151 165
 
152 166
     def get(self, key):
153 167
         """
154  
-        Gets the object specified by key.  It will also unpickle the object
155  
-        before returning if it is pickled in memcache.
  168
+        Gets the object specified by key.  It will also unserialize the object
  169
+        before returning if it is serialized in memcache with JSON, or if it
  170
+        is pickled and unpickling is allowed.
156 171
 
157 172
         :param key: key
158 173
         :returns: value of the key in memcache
@@ -168,7 +183,12 @@ def get(self, key):
168 183
                         size = int(line[3])
169 184
                         value = fp.read(size)
170 185
                         if int(line[2]) & PICKLE_FLAG:
171  
-                            value = pickle.loads(value)
  186
+                            if self._allow_unpickle:
  187
+                                value = pickle.loads(value)
  188
+                            else:
  189
+                                value = None
  190
+                        elif int(line[2]) & JSON_FLAG:
  191
+                            value = json.loads(value)
172 192
                         fp.readline()
173 193
                     line = fp.readline().strip().split()
174 194
                 self._return_conn(server, fp, sock)
@@ -258,7 +278,9 @@ def set_multi(self, mapping, server_key, serialize=True, timeout=0):
258 278
         :param mapping: dictonary of keys and values to be set in memcache
259 279
         :param servery_key: key to use in determining which server in the ring
260 280
                             is used
261  
-        :param serialize: if True, value is pickled before sending to memcache
  281
+        :param serialize: if True, value is serialized with JSON before sending
  282
+                          to memcache, or with pickle if configured to use
  283
+                          pickle instead of JSON (to avoid cache poisoning)
262 284
         :param timeout: ttl for memcache
263 285
         """
264 286
         server_key = md5hash(server_key)
@@ -268,9 +290,12 @@ def set_multi(self, mapping, server_key, serialize=True, timeout=0):
268 290
         for key, value in mapping.iteritems():
269 291
             key = md5hash(key)
270 292
             flags = 0
271  
-            if serialize:
  293
+            if serialize and self._allow_pickle:
272 294
                 value = pickle.dumps(value, PICKLE_PROTOCOL)
273 295
                 flags |= PICKLE_FLAG
  296
+            elif serialize:
  297
+                value = json.dumps(value)
  298
+                flags |= JSON_FLAG
274 299
             msg += ('set %s %d %d %s noreply\r\n%s\r\n' %
275 300
                     (key, flags, timeout, len(value), value))
276 301
         for (server, fp, sock) in self._get_conns(server_key):
@@ -302,7 +327,12 @@ def get_multi(self, keys, server_key):
302 327
                         size = int(line[3])
303 328
                         value = fp.read(size)
304 329
                         if int(line[2]) & PICKLE_FLAG:
305  
-                            value = pickle.loads(value)
  330
+                            if self._allow_unpickle:
  331
+                                value = pickle.loads(value)
  332
+                            else:
  333
+                                value = None
  334
+                        elif int(line[2]) & JSON_FLAG:
  335
+                            value = json.loads(value)
306 336
                         responses[line[1]] = value
307 337
                         fp.readline()
308 338
                     line = fp.readline().strip().split()
30  swift/common/middleware/memcache.py
@@ -27,20 +27,36 @@ class MemcacheMiddleware(object):
27 27
     def __init__(self, app, conf):
28 28
         self.app = app
29 29
         self.memcache_servers = conf.get('memcache_servers')
30  
-        if not self.memcache_servers:
  30
+        serialization_format = conf.get('memcache_serialization_support')
  31
+
  32
+        if not self.memcache_servers or serialization_format is None:
31 33
             path = os.path.join(conf.get('swift_dir', '/etc/swift'),
32 34
                                 'memcache.conf')
33 35
             memcache_conf = ConfigParser()
34 36
             if memcache_conf.read(path):
35  
-                try:
36  
-                    self.memcache_servers = \
37  
-                        memcache_conf.get('memcache', 'memcache_servers')
38  
-                except (NoSectionError, NoOptionError):
39  
-                    pass
  37
+                if not self.memcache_servers:
  38
+                    try:
  39
+                        self.memcache_servers = \
  40
+                            memcache_conf.get('memcache', 'memcache_servers')
  41
+                    except (NoSectionError, NoOptionError):
  42
+                        pass
  43
+                if serialization_format is None:
  44
+                    try:
  45
+                        serialization_format = \
  46
+                            memcache_conf.get('memcache',
  47
+                                              'memcache_serialization_support')
  48
+                    except (NoSectionError, NoOptionError):
  49
+                        pass
  50
+
40 51
         if not self.memcache_servers:
41 52
             self.memcache_servers = '127.0.0.1:11211'
  53
+        if serialization_format is None:
  54
+            serialization_format = 2
  55
+
42 56
         self.memcache = MemcacheRing(
43  
-            [s.strip() for s in self.memcache_servers.split(',') if s.strip()])
  57
+            [s.strip() for s in self.memcache_servers.split(',') if s.strip()],
  58
+            allow_pickle=(serialization_format == 0),
  59
+            allow_unpickle=(serialization_format <= 1))
44 60
 
45 61
     def __call__(self, env, start_response):
46 62
         env['swift.cache'] = self.memcache
5  test/unit/common/middleware/test_memcache.py
@@ -47,6 +47,8 @@ def get(self, section, option):
47 47
         if section == 'memcache':
48 48
             if option == 'memcache_servers':
49 49
                 return '1.2.3.4:5'
  50
+            elif option == 'memcache_serialization_support':
  51
+                return '2'
50 52
             else:
51 53
                 raise NoOptionError(option)
52 54
         else:
@@ -86,7 +88,8 @@ def test_conf_set_no_read(self):
86 88
         exc = None
87 89
         try:
88 90
             app = memcache.MemcacheMiddleware(
89  
-                    FakeApp(), {'memcache_servers': '1.2.3.4:5'})
  91
+                    FakeApp(), {'memcache_servers': '1.2.3.4:5',
  92
+                                'memcache_serialization_support': '2'})
90 93
         except Exception, err:
91 94
             exc = err
92 95
         finally:
22  test/unit/common/test_memcached.py
... ...
@@ -1,3 +1,4 @@
  1
+ # -*- coding: utf8 -*-
1 2
 # Copyright (c) 2010-2012 OpenStack, LLC.
2 3
 #
3 4
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -161,6 +162,9 @@ def test_set_get(self):
161 162
         self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
162 163
         memcache_client.set('some_key', [4, 5, 6])
163 164
         self.assertEquals(memcache_client.get('some_key'), [4, 5, 6])
  165
+        memcache_client.set('some_key', ['simple str', 'utf8 str éà'])
  166
+        # As per http://wiki.openstack.org/encoding, we should expect to have unicode
  167
+        self.assertEquals(memcache_client.get('some_key'), ['simple str', u'utf8 str éà'])
164 168
         self.assert_(float(mock.cache.values()[0][1]) == 0)
165 169
         esttimeout = time.time() + 10
166 170
         memcache_client.set('some_key', [1, 2, 3], timeout=10)
@@ -239,6 +243,24 @@ def test_multi(self):
239 243
         self.assertEquals(memcache_client.get_multi(('some_key2', 'some_key1',
240 244
             'not_exists'), 'multi_key'), [[4, 5, 6], [1, 2, 3], None])
241 245
 
  246
+    def test_serialization(self):
  247
+        memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'],
  248
+                                                 allow_pickle=True)
  249
+        mock = MockMemcached()
  250
+        memcache_client._client_cache['1.2.3.4:11211'] = [(mock, mock)] * 2
  251
+        memcache_client.set('some_key', [1, 2, 3])
  252
+        self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
  253
+        memcache_client._allow_pickle = False
  254
+        memcache_client._allow_unpickle = True
  255
+        self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
  256
+        memcache_client._allow_unpickle = False
  257
+        self.assertEquals(memcache_client.get('some_key'), None)
  258
+        memcache_client.set('some_key', [1, 2, 3])
  259
+        self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
  260
+        memcache_client._allow_unpickle = True
  261
+        self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
  262
+        memcache_client._allow_pickle = True
  263
+        self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
242 264
 
243 265
 if __name__ == '__main__':
244 266
     unittest.main()

Git Notes

review

Verified+2: Jenkins
Code-Review+1: Christoph Thiel <cthiel@suse.com>
Code-Review+2: gholt <z-launchpad@brim.net>
Approved+1: Samuel Merritt <sam@swiftstack.com>
Code-Review+2: Samuel Merritt <sam@swiftstack.com>
Code-Review+1: Darrell Bishop <darrell@swiftstack.com>
Submitted-by: Jenkins
Submitted-at: Tue, 28 Aug 2012 21:39:57 +0000
Reviewed-on: https://review.openstack.org/9105
Project: openstack/swift
Branch: refs/heads/master

0 notes on commit e1ff51c

Please sign in to comment.
Something went wrong with that request. Please try again.