openstack/swift
Code Issues Proposed changes

Merge "Fix some request-smuggling vectors on py3"

Browse Source
This commit is contained in:
Zuul 2019-10-02 23:09:48 +00:00 committed by Gerrit Code Review
commit 6114965ab9
4 changed files with 98 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
swift/common/wsgi.py
View File

@ -498,6 +498,11 @@ class SwiftHttpProtocol(wsgi.HttpProtocol):
break
header, value = line.split(':', 1)
value = value.strip(' \t\n\r')
# NB: Eventlet looks at the headers obj to figure out
# whether the client said the connection should close;
# see https://github.com/eventlet/eventlet/blob/v0.25.0/
# eventlet/wsgi.py#L504
self.headers.add_header(header, value)
headers_raw.append((header, value))
wsgi_key = 'HTTP_' + header.replace('-', '_').encode(
'latin1').upper().decode('latin1')
@ -506,6 +511,20 @@ class SwiftHttpProtocol(wsgi.HttpProtocol):
wsgi_key = wsgi_key[5:]
environ[wsgi_key] = value
environ['headers_raw'] = tuple(headers_raw)
# Since we parsed some more headers, check to see if they
# change how our wsgi.input should behave
te = environ.get('HTTP_TRANSFER_ENCODING', '').lower()
if te.rsplit(',', 1)[-1].strip() == 'chunked':
environ['wsgi.input'].chunked_input = True
else:
length = environ.get('CONTENT_LENGTH')
if length:
length = int(length)
environ['wsgi.input'].content_length = length
if environ.get('HTTP_EXPECT', '').lower() == '100-continue':
environ['wsgi.input'].wfile = self.wfile
environ['wsgi.input'].wfile_line = \
b'HTTP/1.1 100 Continue\r\n'
return environ

7
swift/proxy/server.py
View File

@ -457,6 +457,13 @@ class Application(object):
if 'x-storage-token' in req.headers and \
'x-auth-token' not in req.headers:
req.headers['x-auth-token'] = req.headers['x-storage-token']
te = req.headers.get('transfer-encoding', '').lower()
if te.rsplit(',', 1)[-1].strip() == 'chunked' and \
'content-length' in req.headers:
# RFC says if both are present, transfer-encoding wins.
# Definitely *don't* forward on the header the backend
# ought to ignore; that offers request-smuggling vectors.
del req.headers['content-length']
return req
def handle_request(self, req):

1
test/unit/proxy/controllers/test_obj.py
View File

@ -1104,6 +1104,7 @@ class TestReplicatedObjController(CommonObjectControllerMixin,
body = unchunk_body(body)
self.assertEqual('100-continue', headers['Expect'])
self.assertEqual('chunked', headers['Transfer-Encoding'])
self.assertNotIn('Content-Length', headers)
else:
self.assertNotIn('Transfer-Encoding', headers)
if body or not test_body:

74
test/unit/proxy/test_server.py
View File

@ -3171,14 +3171,68 @@ class TestReplicatedObjectController(
prolis = _test_sockets[0]
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile('rwb')
fd.write(b'PUT /v1/a/c/o.chunked HTTP/1.1\r\n'
with mock.patch('swift.obj.diskfile.fallocate') as mock_fallocate:
fd.write(b'PUT /v1/a/c/o.chunked HTTP/1.1\r\n'
b'Host: localhost\r\n'
b'Connection: keep-alive\r\n'
b'X-Storage-Token: t\r\n'
b'Content-Type: application/octet-stream\r\n'
b'Content-Length: 33\r\n'
b'Transfer-Encoding: chunked\r\n\r\n'
b'2\r\n'
b'oh\r\n'
b'4\r\n'
b' say\r\n'
b'4\r\n'
b' can\r\n'
b'4\r\n'
b' you\r\n'
b'4\r\n'
b' see\r\n'
b'3\r\n'
b' by\r\n'
b'4\r\n'
b' the\r\n'
b'8\r\n'
b' dawns\'\n\r\n'
b'0\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = b'HTTP/1.1 201'
self.assertEqual(headers[:len(exp)], exp)
self.assertFalse(mock_fallocate.mock_calls)
fd.write(b'GET /v1/a/c/o.chunked HTTP/1.1\r\n'
b'Host: localhost\r\n'
b'Connection: close\r\n'
b'X-Storage-Token: t\r\n'
b'\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = b'HTTP/1.1 200'
self.assertEqual(headers[:len(exp)], exp)
self.assertIn(b'Content-Length: 33', headers.split(b'\r\n'))
self.assertEqual(b"oh say can you see by the dawns'\n", fd.read(33))
@unpatch_policies
def test_PUT_message_length_using_both_with_crazy_meta(self):
prolis = _test_sockets[0]
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile('rwb')
fd.write(b'PUT /v1/a/c/o.chunked HTTP/1.1\r\n'
b'Host: localhost\r\n'
b'X-Storage-Token: t\r\n'
b'Content-Type: application/octet-stream\r\n'
b'Content-Length: 33\r\n'
b'Transfer-Encoding: chunked\r\n\r\n'
b'2\r\n'
b'X-Object-Meta-\xf0\x9f\x8c\xb4: \xf0\x9f\x91\x8d\r\n'
b'Expect: 100-continue\r\n'
b'Transfer-Encoding: chunked\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = b'HTTP/1.1 100 Continue'
self.assertEqual(headers[:len(exp)], exp)
# Since we got our 100 Continue, now we can send the body
fd.write(b'2\r\n'
b'oh\r\n'
b'4\r\n'
b' say\r\n'
@ -3200,6 +3254,20 @@ class TestReplicatedObjectController(
exp = b'HTTP/1.1 201'
self.assertEqual(headers[:len(exp)], exp)
fd.write(b'GET /v1/a/c/o.chunked HTTP/1.1\r\n'
b'Host: localhost\r\n'
b'Connection: close\r\n'
b'X-Storage-Token: t\r\n'
b'\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = b'HTTP/1.1 200'
self.assertEqual(headers[:len(exp)], exp)
self.assertIn(b'Content-Length: 33', headers.split(b'\r\n'))
self.assertIn(b'X-Object-Meta-\xf0\x9f\x8c\xb4: \xf0\x9f\x91\x8d',
headers.split(b'\r\n'))
self.assertEqual(b"oh say can you see by the dawns'\n", fd.read(33))
@unpatch_policies
def test_PUT_bad_message_length(self):
prolis = _test_sockets[0]