[openstack-swift/f19: 1/2] Fix handling of DELETE obj reqs with old timestamp
Pete Zaitcev
zaitcev at fedoraproject.org
Wed Aug 7 18:17:04 UTC 2013
commit ac36771c0c7a6a83d61fcf4cb3f09c7bdbb7f4be
Author: Pete Zaitcev <zaitcev at kotori.zaitcev.us>
Date: Wed Aug 7 11:58:22 2013 -0600
Fix handling of DELETE obj reqs with old timestamp
CVE-2013-4155
Fedora tracking bug 994665
See also top level bz#991626
openstack-swift.spec | 8 +-
swift-1.8.0-lp1196932.patch | 413 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 420 insertions(+), 1 deletions(-)
---
diff --git a/openstack-swift.spec b/openstack-swift.spec
index 0dee9d0..9ceb641 100644
--- a/openstack-swift.spec
+++ b/openstack-swift.spec
@@ -4,7 +4,7 @@
Name: openstack-swift
Version: 1.8.0
-Release: 2%{?dist}
+Release: 3%{?dist}
Summary: OpenStack Object Storage (Swift)
Group: Development/Languages
@@ -61,6 +61,8 @@ Requires: pyxattr
Requires: python-setuptools
Requires: python-netifaces
+Patch1: swift-1.8.0-lp1196932.patch
+
BuildRequires: systemd
Requires(post): systemd
Requires(preun): systemd
@@ -156,6 +158,7 @@ This package contains documentation files for %{name}.
%prep
%setup -q -n swift-%{version}
+%patch1 -p1
# Remove bundled egg-info
rm -rf swift.egg-info
@@ -443,6 +446,9 @@ exit 0
%doc LICENSE doc/build/html
%changelog
+* Wed Aug 07 2013 Pete Zaitcev <zaitcev at redhat.com> 1.8.0-3
+- CVE-2013-4155 "Fix handling of DELETE obj reqs with old timestamp"
+
* Fri Apr 05 2013 Derek Higgins <derekh at redhat.com> - 1.8.0-2
- change path to middleware in proxy conf file
- add dependency for python-keystoneclient for proxy
diff --git a/swift-1.8.0-lp1196932.patch b/swift-1.8.0-lp1196932.patch
new file mode 100644
index 0000000..cec1035
--- /dev/null
+++ b/swift-1.8.0-lp1196932.patch
@@ -0,0 +1,413 @@
+commit 1f4ec235cdfd8c868f2d6458532f9dc32c00b8ca
+Author: Peter Portante <peter.portante at redhat.com>
+Date: Fri Jul 26 15:03:34 2013 -0400
+
+ Fix handling of DELETE obj reqs with old timestamp
+
+ The DELETE object REST API was creating tombstone files with old
+ timestamps, potentially filling up the disk, as well as sending
+ container updates.
+
+ Here we now make DELETEs with a request timestamp return a 409 (HTTP
+ Conflict) if a data file exists with a newer timestamp, only creating
+ tombstones if they have a newer timestamp.
+
+ The key fix is to actually read the timestamp metadata from an
+ existing tombstone file (thanks to Pete Zaitcev for catching this),
+ and then only create tombstone files with newer timestamps.
+
+ We also prevent PUT and POST operations using old timestamps as well.
+
+ Change-Id: I631957029d17c6578bca5779367df5144ba01fc9
+ Signed-off-by: Peter Portante <peter.portante at redhat.com>
+
+diff --git a/swift/obj/server.py b/swift/obj/server.py
+index fc23ea2..f416162 100644
+--- a/swift/obj/server.py
++++ b/swift/obj/server.py
+@@ -46,7 +46,7 @@ from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPCreated, \
+ HTTPInternalServerError, HTTPNoContent, HTTPNotFound, HTTPNotModified, \
+ HTTPPreconditionFailed, HTTPRequestTimeout, HTTPUnprocessableEntity, \
+ HTTPClientDisconnect, HTTPMethodNotAllowed, Request, Response, UTC, \
+- HTTPInsufficientStorage, multi_range_iterator
++ HTTPInsufficientStorage, multi_range_iterator, HTTPConflict
+
+
+ DATADIR = 'objects'
+@@ -121,7 +121,6 @@ class DiskFile(object):
+ self.tmppath = None
+ self.logger = logger
+ self.metadata = {}
+- self.meta_file = None
+ self.data_file = None
+ self.fp = None
+ self.iter_etag = None
+@@ -133,15 +132,18 @@ class DiskFile(object):
+ if not os.path.exists(self.datadir):
+ return
+ files = sorted(os.listdir(self.datadir), reverse=True)
+- for file in files:
+- if file.endswith('.ts'):
+- self.data_file = self.meta_file = None
+- self.metadata = {'deleted': True}
+- return
+- if file.endswith('.meta') and not self.meta_file:
+- self.meta_file = os.path.join(self.datadir, file)
+- if file.endswith('.data') and not self.data_file:
+- self.data_file = os.path.join(self.datadir, file)
++ meta_file = None
++ for afile in files:
++ if afile.endswith('.ts'):
++ self.data_file = None
++ with open(os.path.join(self.datadir, afile)) as mfp:
++ self.metadata = read_metadata(mfp)
++ self.metadata['deleted'] = True
++ break
++ if afile.endswith('.meta') and not meta_file:
++ meta_file = os.path.join(self.datadir, afile)
++ if afile.endswith('.data') and not self.data_file:
++ self.data_file = os.path.join(self.datadir, afile)
+ break
+ if not self.data_file:
+ return
+@@ -149,8 +151,8 @@ class DiskFile(object):
+ self.metadata = read_metadata(self.fp)
+ if not keep_data_fp:
+ self.close(verify_file=False)
+- if self.meta_file:
+- with open(self.meta_file) as mfp:
++ if meta_file:
++ with open(meta_file) as mfp:
+ for key in self.metadata.keys():
+ if key.lower() not in DISALLOWED_HEADERS:
+ del self.metadata[key]
+@@ -594,6 +596,9 @@ class ObjectController(object):
+ except (DiskFileError, DiskFileNotExist):
+ file.quarantine()
+ return HTTPNotFound(request=request)
++ orig_timestamp = file.metadata.get('X-Timestamp', '0')
++ if orig_timestamp >= request.headers['x-timestamp']:
++ return HTTPConflict(request=request)
+ metadata = {'X-Timestamp': request.headers['x-timestamp']}
+ metadata.update(val for val in request.headers.iteritems()
+ if val[0].lower().startswith('x-object-meta-'))
+@@ -639,6 +644,8 @@ class ObjectController(object):
+ file = DiskFile(self.devices, device, partition, account, container,
+ obj, self.logger, disk_chunk_size=self.disk_chunk_size)
+ orig_timestamp = file.metadata.get('X-Timestamp')
++ if orig_timestamp and orig_timestamp >= request.headers['x-timestamp']:
++ return HTTPConflict(request=request)
+ upload_expiration = time.time() + self.max_upload_time
+ etag = md5()
+ upload_size = 0
+@@ -863,23 +870,26 @@ class ObjectController(object):
+ return HTTPPreconditionFailed(
+ request=request,
+ body='X-If-Delete-At and X-Delete-At do not match')
+- orig_timestamp = file.metadata.get('X-Timestamp')
+- if file.is_deleted() or file.is_expired():
+- response_class = HTTPNotFound
+- metadata = {
+- 'X-Timestamp': request.headers['X-Timestamp'], 'deleted': True,
+- }
+ old_delete_at = int(file.metadata.get('X-Delete-At') or 0)
+ if old_delete_at:
+ self.delete_at_update('DELETE', old_delete_at, account,
+ container, obj, request.headers, device)
+- file.put_metadata(metadata, tombstone=True)
+- file.unlinkold(metadata['X-Timestamp'])
+- if not orig_timestamp or \
+- orig_timestamp < request.headers['x-timestamp']:
++ orig_timestamp = file.metadata.get('X-Timestamp', 0)
++ req_timestamp = request.headers['X-Timestamp']
++ if file.is_deleted() or file.is_expired():
++ response_class = HTTPNotFound
++ else:
++ if orig_timestamp < req_timestamp:
++ response_class = HTTPNoContent
++ else:
++ response_class = HTTPConflict
++ if orig_timestamp < req_timestamp:
++ file.put_metadata({'X-Timestamp': req_timestamp},
++ tombstone=True)
++ file.unlinkold(req_timestamp)
+ self.container_update(
+ 'DELETE', account, container, obj, request.headers,
+- {'x-timestamp': metadata['X-Timestamp'],
++ {'x-timestamp': req_timestamp,
+ 'x-trans-id': request.headers.get('x-trans-id', '-')},
+ device)
+ resp = response_class(request=request)
+diff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py
+index 8ee266b..b354b97 100755
+--- a/test/unit/obj/test_server.py
++++ b/test/unit/obj/test_server.py
+@@ -509,6 +509,41 @@ class TestObjectController(unittest.TestCase):
+ "X-Object-Meta-3" in resp.headers)
+ self.assertEquals(resp.headers['Content-Type'], 'application/x-test')
+
++ def test_POST_old_timestamp(self):
++ ts = time()
++ timestamp = normalize_timestamp(ts)
++ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
++ headers={'X-Timestamp': timestamp,
++ 'Content-Type': 'application/x-test',
++ 'X-Object-Meta-1': 'One',
++ 'X-Object-Meta-Two': 'Two'})
++ req.body = 'VERIFY'
++ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 201)
++
++ # Same timestamp should result in 409
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'POST'},
++ headers={'X-Timestamp': timestamp,
++ 'X-Object-Meta-3': 'Three',
++ 'X-Object-Meta-4': 'Four',
++ 'Content-Encoding': 'gzip',
++ 'Content-Type': 'application/x-test'})
++ resp = self.object_controller.POST(req)
++ self.assertEquals(resp.status_int, 409)
++
++ # Earlier timestamp should result in 409
++ timestamp = normalize_timestamp(ts - 1)
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'POST'},
++ headers={'X-Timestamp': timestamp,
++ 'X-Object-Meta-5': 'Five',
++ 'X-Object-Meta-6': 'Six',
++ 'Content-Encoding': 'gzip',
++ 'Content-Type': 'application/x-test'})
++ resp = self.object_controller.POST(req)
++ self.assertEquals(resp.status_int, 409)
++
+ def test_POST_not_exist(self):
+ timestamp = normalize_timestamp(time())
+ req = Request.blank('/sda1/p/a/c/fail',
+@@ -555,11 +590,15 @@ class TestObjectController(unittest.TestCase):
+
+ old_http_connect = object_server.http_connect
+ try:
+- timestamp = normalize_timestamp(time())
++ ts = time()
++ timestamp = normalize_timestamp(ts)
+ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD':
+ 'POST'}, headers={'X-Timestamp': timestamp, 'Content-Type':
+ 'text/plain', 'Content-Length': '0'})
+ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 201)
++
++ timestamp = normalize_timestamp(ts + 1)
+ req = Request.blank('/sda1/p/a/c/o',
+ environ={'REQUEST_METHOD': 'POST'},
+ headers={'X-Timestamp': timestamp,
+@@ -571,6 +610,8 @@ class TestObjectController(unittest.TestCase):
+ object_server.http_connect = mock_http_connect(202)
+ resp = self.object_controller.POST(req)
+ self.assertEquals(resp.status_int, 202)
++
++ timestamp = normalize_timestamp(ts + 2)
+ req = Request.blank('/sda1/p/a/c/o',
+ environ={'REQUEST_METHOD': 'POST'},
+ headers={'X-Timestamp': timestamp,
+@@ -582,6 +623,8 @@ class TestObjectController(unittest.TestCase):
+ object_server.http_connect = mock_http_connect(202, with_exc=True)
+ resp = self.object_controller.POST(req)
+ self.assertEquals(resp.status_int, 202)
++
++ timestamp = normalize_timestamp(ts + 3)
+ req = Request.blank('/sda1/p/a/c/o',
+ environ={'REQUEST_METHOD': 'POST'},
+ headers={'X-Timestamp': timestamp,
+@@ -718,6 +761,32 @@ class TestObjectController(unittest.TestCase):
+ 'name': '/a/c/o',
+ 'Content-Encoding': 'gzip'})
+
++ def test_PUT_old_timestamp(self):
++ ts = time()
++ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
++ headers={'X-Timestamp': normalize_timestamp(ts),
++ 'Content-Length': '6',
++ 'Content-Type': 'application/octet-stream'})
++ req.body = 'VERIFY'
++ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 201)
++
++ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
++ headers={'X-Timestamp': normalize_timestamp(ts),
++ 'Content-Type': 'text/plain',
++ 'Content-Encoding': 'gzip'})
++ req.body = 'VERIFY TWO'
++ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 409)
++
++ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
++ headers={'X-Timestamp': normalize_timestamp(ts - 1),
++ 'Content-Type': 'text/plain',
++ 'Content-Encoding': 'gzip'})
++ req.body = 'VERIFY THREE'
++ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 409)
++
+ def test_PUT_no_etag(self):
+ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
+ headers={'X-Timestamp': normalize_timestamp(time()),
+@@ -1306,12 +1375,32 @@ class TestObjectController(unittest.TestCase):
+ self.assertEquals(resp.status_int, 400)
+ # self.assertRaises(KeyError, self.object_controller.DELETE, req)
+
++ # The following should have created a tombstone file
+ timestamp = normalize_timestamp(time())
+ req = Request.blank('/sda1/p/a/c/o',
+ environ={'REQUEST_METHOD': 'DELETE'},
+ headers={'X-Timestamp': timestamp})
+ resp = self.object_controller.DELETE(req)
+ self.assertEquals(resp.status_int, 404)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assert_(os.path.isfile(objfile))
++
++ # The following should *not* have created a tombstone file.
++ timestamp = normalize_timestamp(float(timestamp) - 1)
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'DELETE'},
++ headers={'X-Timestamp': timestamp})
++ resp = self.object_controller.DELETE(req)
++ self.assertEquals(resp.status_int, 404)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assertFalse(os.path.exists(objfile))
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
+
+ sleep(.00001)
+ timestamp = normalize_timestamp(time())
+@@ -1325,17 +1414,19 @@ class TestObjectController(unittest.TestCase):
+ resp = self.object_controller.PUT(req)
+ self.assertEquals(resp.status_int, 201)
+
++ # The following should *not* have created a tombstone file.
+ timestamp = normalize_timestamp(float(timestamp) - 1)
+ req = Request.blank('/sda1/p/a/c/o',
+ environ={'REQUEST_METHOD': 'DELETE'},
+ headers={'X-Timestamp': timestamp})
+ resp = self.object_controller.DELETE(req)
+- self.assertEquals(resp.status_int, 204)
++ self.assertEquals(resp.status_int, 409)
+ objfile = os.path.join(self.testdir, 'sda1',
+ storage_directory(object_server.DATADIR, 'p',
+ hash_path('a', 'c', 'o')),
+ timestamp + '.ts')
+- self.assert_(os.path.isfile(objfile))
++ self.assertFalse(os.path.exists(objfile))
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
+
+ sleep(.00001)
+ timestamp = normalize_timestamp(time())
+@@ -1350,6 +1441,103 @@ class TestObjectController(unittest.TestCase):
+ timestamp + '.ts')
+ self.assert_(os.path.isfile(objfile))
+
++ def test_DELETE_container_updates(self):
++ # Test swift.object_server.ObjectController.DELETE and container
++ # updates, making sure container update is called in the correct
++ # state.
++ timestamp = normalize_timestamp(time())
++ req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
++ headers={
++ 'X-Timestamp': timestamp,
++ 'Content-Type': 'application/octet-stream',
++ 'Content-Length': '4',
++ })
++ req.body = 'test'
++ resp = self.object_controller.PUT(req)
++ self.assertEquals(resp.status_int, 201)
++
++ calls_made = [0]
++
++ def our_container_update(*args, **kwargs):
++ calls_made[0] += 1
++
++ orig_cu = self.object_controller.container_update
++ self.object_controller.container_update = our_container_update
++ try:
++ # The following request should return 409 (HTTP Conflict). A
++ # tombstone file should not have been created with this timestamp.
++ timestamp = normalize_timestamp(float(timestamp) - 1)
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'DELETE'},
++ headers={'X-Timestamp': timestamp})
++ resp = self.object_controller.DELETE(req)
++ self.assertEquals(resp.status_int, 409)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assertFalse(os.path.isfile(objfile))
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
++ self.assertEquals(0, calls_made[0])
++
++ # The following request should return 204, and the object should
++ # be truly deleted (container update is performed) because this
++ # timestamp is newer. A tombstone file should have been created
++ # with this timestamp.
++ sleep(.00001)
++ timestamp = normalize_timestamp(time())
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'DELETE'},
++ headers={'X-Timestamp': timestamp})
++ resp = self.object_controller.DELETE(req)
++ self.assertEquals(resp.status_int, 204)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assert_(os.path.isfile(objfile))
++ self.assertEquals(1, calls_made[0])
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
++
++ # The following request should return a 404, as the object should
++ # already have been deleted, but it should have also performed a
++ # container update because the timestamp is newer, and a tombstone
++ # file should also exist with this timestamp.
++ sleep(.00001)
++ timestamp = normalize_timestamp(time())
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'DELETE'},
++ headers={'X-Timestamp': timestamp})
++ resp = self.object_controller.DELETE(req)
++ self.assertEquals(resp.status_int, 404)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assert_(os.path.isfile(objfile))
++ self.assertEquals(2, calls_made[0])
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
++
++ # The following request should return a 404, as the object should
++ # already have been deleted, and it should not have performed a
++ # container update because the timestamp is older, or created a
++ # tombstone file with this timestamp.
++ timestamp = normalize_timestamp(float(timestamp) - 1)
++ req = Request.blank('/sda1/p/a/c/o',
++ environ={'REQUEST_METHOD': 'DELETE'},
++ headers={'X-Timestamp': timestamp})
++ resp = self.object_controller.DELETE(req)
++ self.assertEquals(resp.status_int, 404)
++ objfile = os.path.join(self.testdir, 'sda1',
++ storage_directory(object_server.DATADIR, 'p',
++ hash_path('a', 'c', 'o')),
++ timestamp + '.ts')
++ self.assertFalse(os.path.isfile(objfile))
++ self.assertEquals(2, calls_made[0])
++ self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
++ finally:
++ self.object_controller.container_update = orig_cu
++
+ def test_call(self):
+ """ Test swift.object_server.ObjectController.__call__ """
+ inbuf = StringIO()
More information about the scm-commits
mailing list