Change-Id: I04c858f82acaab52ebfe0b44082cbb9f09844540
Signed-off-by: Tapas Kundu <tkundu@vmware.com>
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/9959
Tested-by: gerrit-photon <photon-checkins@vmware.com>
Reviewed-by: Srinidhi Rao <srinidhir@vmware.com>
1 | 1 |
new file mode 100644 |
... | ... |
@@ -0,0 +1,260 @@ |
0 |
+From 4a7d22e490bb8ff836892cc99a1f54b85ccb0281 Mon Sep 17 00:00:00 2001 |
|
1 |
+From: Mark Williams <mrw@enotuniq.org> |
|
2 |
+Date: Sun, 16 Feb 2020 19:00:10 -0800 |
|
3 |
+Subject: [PATCH] Fix several request smuggling attacks. |
|
4 |
+ |
|
5 |
+1. Requests with multiple Content-Length headers were allowed (thanks |
|
6 |
+to Jake Miller from Bishop Fox and ZeddYu Lu) and now fail with a 400; |
|
7 |
+ |
|
8 |
+2. Requests with a Content-Length header and a Transfer-Encoding |
|
9 |
+header honored the first header (thanks to Jake Miller from Bishop |
|
10 |
+Fox) and now fail with a 400; |
|
11 |
+ |
|
12 |
+3. Requests whose Transfer-Encoding header had a value other than |
|
13 |
+"chunked" and "identity" (thanks to ZeddYu Lu) were allowed and now fail |
|
14 |
+with a 400. |
|
15 |
+--- |
|
16 |
+ src/twisted/web/http.py | 64 +++++++--- |
|
17 |
+ src/twisted/web/newsfragments/9770.bugfix | 1 + |
|
18 |
+ src/twisted/web/test/test_http.py | 137 ++++++++++++++++++++++ |
|
19 |
+ 3 files changed, 187 insertions(+), 15 deletions(-) |
|
20 |
+ create mode 100644 src/twisted/web/newsfragments/9770.bugfix |
|
21 |
+ |
|
22 |
+diff --git a/src/twisted/web/http.py b/src/twisted/web/http.py |
|
23 |
+index f0fb05b4d69..06d830fe30f 100644 |
|
24 |
+--- a/src/twisted/web/http.py |
|
25 |
+@@ -2171,6 +2171,51 @@ def _finishRequestBody(self, data): |
|
26 |
+ self.allContentReceived() |
|
27 |
+ self._dataBuffer.append(data) |
|
28 |
+ |
|
29 |
++ def _maybeChooseTransferDecoder(self, header, data): |
|
30 |
++ """ |
|
31 |
++ If the provided header is C{content-length} or |
|
32 |
++ C{transfer-encoding}, choose the appropriate decoder if any. |
|
33 |
++ |
|
34 |
++ Returns L{True} if the request can proceed and L{False} if not. |
|
35 |
++ """ |
|
36 |
++ |
|
37 |
++ def fail(): |
|
38 |
++ self._respondToBadRequestAndDisconnect() |
|
39 |
++ self.length = None |
|
40 |
++ |
|
41 |
++ # Can this header determine the length? |
|
42 |
++ if header == b'content-length': |
|
43 |
++ try: |
|
44 |
++ length = int(data) |
|
45 |
++ except ValueError: |
|
46 |
++ fail() |
|
47 |
++ return False |
|
48 |
++ newTransferDecoder = _IdentityTransferDecoder( |
|
49 |
++ length, self.requests[-1].handleContentChunk, self._finishRequestBody) |
|
50 |
++ elif header == b'transfer-encoding': |
|
51 |
++ # XXX Rather poorly tested code block, apparently only exercised by |
|
52 |
++ # test_chunkedEncoding |
|
53 |
++ if data.lower() == b'chunked': |
|
54 |
++ length = None |
|
55 |
++ newTransferDecoder = _ChunkedTransferDecoder( |
|
56 |
++ self.requests[-1].handleContentChunk, self._finishRequestBody) |
|
57 |
++ elif data.lower() == b'identity': |
|
58 |
++ return True |
|
59 |
++ else: |
|
60 |
++ fail() |
|
61 |
++ return False |
|
62 |
++ else: |
|
63 |
++ # It's not a length related header, so exit |
|
64 |
++ return True |
|
65 |
++ |
|
66 |
++ if self._transferDecoder is not None: |
|
67 |
++ fail() |
|
68 |
++ return False |
|
69 |
++ else: |
|
70 |
++ self.length = length |
|
71 |
++ self._transferDecoder = newTransferDecoder |
|
72 |
++ return True |
|
73 |
++ |
|
74 |
+ |
|
75 |
+ def headerReceived(self, line): |
|
76 |
+ """ |
|
77 |
+@@ -2196,21 +2241,10 @@ def headerReceived(self, line): |
|
78 |
+ |
|
79 |
+ header = header.lower() |
|
80 |
+ data = data.strip() |
|
81 |
+- if header == b'content-length': |
|
82 |
+- try: |
|
83 |
+- self.length = int(data) |
|
84 |
+- except ValueError: |
|
85 |
+- self._respondToBadRequestAndDisconnect() |
|
86 |
+- self.length = None |
|
87 |
+- return False |
|
88 |
+- self._transferDecoder = _IdentityTransferDecoder( |
|
89 |
+- self.length, self.requests[-1].handleContentChunk, self._finishRequestBody) |
|
90 |
+- elif header == b'transfer-encoding' and data.lower() == b'chunked': |
|
91 |
+- # XXX Rather poorly tested code block, apparently only exercised by |
|
92 |
+- # test_chunkedEncoding |
|
93 |
+- self.length = None |
|
94 |
+- self._transferDecoder = _ChunkedTransferDecoder( |
|
95 |
+- self.requests[-1].handleContentChunk, self._finishRequestBody) |
|
96 |
++ |
|
97 |
++ if not self._maybeChooseTransferDecoder(header, data): |
|
98 |
++ return False |
|
99 |
++ |
|
100 |
+ reqHeaders = self.requests[-1].requestHeaders |
|
101 |
+ values = reqHeaders.getRawHeaders(header) |
|
102 |
+ if values is not None: |
|
103 |
+diff --git a/src/twisted/web/newsfragments/9770.bugfix b/src/twisted/web/newsfragments/9770.bugfix |
|
104 |
+new file mode 100644 |
|
105 |
+index 00000000000..4f1be97de8a |
|
106 |
+--- /dev/null |
|
107 |
+@@ -0,0 +1 @@ |
|
108 |
++Fix several request smuggling attacks: requests with multiple Content-Length headers were allowed (thanks to Jake Miller from Bishop Fox and ZeddYu Lu) and now fail with a 400; requests with a Content-Length header and a Transfer-Encoding header honored the first header (thanks to Jake Miller from Bishop Fox) and now fail with a 400; requests whose Transfer-Encoding header had a value other than "chunked" and "identity" (thanks to ZeddYu Lu) were allowed and now fail a 400. |
|
109 |
+\ No newline at end of file |
|
110 |
+diff --git a/src/twisted/web/test/test_http.py b/src/twisted/web/test/test_http.py |
|
111 |
+index 0a0db09b750..578cb500cda 100644 |
|
112 |
+--- a/src/twisted/web/test/test_http.py |
|
113 |
+@@ -2252,6 +2252,143 @@ def process(self): |
|
114 |
+ self.flushLoggedErrors(AttributeError) |
|
115 |
+ |
|
116 |
+ |
|
117 |
++ def assertDisconnectingBadRequest(self, request): |
|
118 |
++ """ |
|
119 |
++ Assert that the given request bytes fail with a 400 bad |
|
120 |
++ request without calling L{Request.process}. |
|
121 |
++ |
|
122 |
++ @param request: A raw HTTP request |
|
123 |
++ @type request: L{bytes} |
|
124 |
++ """ |
|
125 |
++ class FailedRequest(http.Request): |
|
126 |
++ processed = False |
|
127 |
++ def process(self): |
|
128 |
++ FailedRequest.processed = True |
|
129 |
++ |
|
130 |
++ channel = self.runRequest(request, FailedRequest, success=False) |
|
131 |
++ self.assertFalse(FailedRequest.processed, "Request.process called") |
|
132 |
++ self.assertEqual( |
|
133 |
++ channel.transport.value(), |
|
134 |
++ b"HTTP/1.1 400 Bad Request\r\n\r\n") |
|
135 |
++ self.assertTrue(channel.transport.disconnecting) |
|
136 |
++ |
|
137 |
++ |
|
138 |
++ def test_duplicateContentLengths(self): |
|
139 |
++ """ |
|
140 |
++ A request which includes multiple C{content-length} headers |
|
141 |
++ fails with a 400 response without calling L{Request.process}. |
|
142 |
++ """ |
|
143 |
++ self.assertRequestRejected([ |
|
144 |
++ b'GET /a HTTP/1.1', |
|
145 |
++ b'Content-Length: 56', |
|
146 |
++ b'Content-Length: 0', |
|
147 |
++ b'Host: host.invalid', |
|
148 |
++ b'', |
|
149 |
++ b'', |
|
150 |
++ ]) |
|
151 |
++ |
|
152 |
++ |
|
153 |
++ def test_duplicateContentLengthsWithPipelinedRequests(self): |
|
154 |
++ """ |
|
155 |
++ Two pipelined requests, the first of which includes multiple |
|
156 |
++ C{content-length} headers, trigger a 400 response without |
|
157 |
++ calling L{Request.process}. |
|
158 |
++ """ |
|
159 |
++ self.assertRequestRejected([ |
|
160 |
++ b'GET /a HTTP/1.1', |
|
161 |
++ b'Content-Length: 56', |
|
162 |
++ b'Content-Length: 0', |
|
163 |
++ b'Host: host.invalid', |
|
164 |
++ b'', |
|
165 |
++ b'', |
|
166 |
++ b'GET /a HTTP/1.1', |
|
167 |
++ b'Host: host.invalid', |
|
168 |
++ b'', |
|
169 |
++ b'', |
|
170 |
++ ]) |
|
171 |
++ |
|
172 |
++ |
|
173 |
++ def test_contentLengthAndTransferEncoding(self): |
|
174 |
++ """ |
|
175 |
++ A request that includes both C{content-length} and |
|
176 |
++ C{transfer-encoding} headers fails with a 400 response without |
|
177 |
++ calling L{Request.process}. |
|
178 |
++ """ |
|
179 |
++ self.assertRequestRejected([ |
|
180 |
++ b'GET /a HTTP/1.1', |
|
181 |
++ b'Transfer-Encoding: chunked', |
|
182 |
++ b'Content-Length: 0', |
|
183 |
++ b'Host: host.invalid', |
|
184 |
++ b'', |
|
185 |
++ b'', |
|
186 |
++ ]) |
|
187 |
++ |
|
188 |
++ |
|
189 |
++ def test_contentLengthAndTransferEncodingWithPipelinedRequests(self): |
|
190 |
++ """ |
|
191 |
++ Two pipelined requests, the first of which includes both |
|
192 |
++ C{content-length} and C{transfer-encoding} headers, triggers a |
|
193 |
++ 400 response without calling L{Request.process}. |
|
194 |
++ """ |
|
195 |
++ self.assertRequestRejected([ |
|
196 |
++ b'GET /a HTTP/1.1', |
|
197 |
++ b'Transfer-Encoding: chunked', |
|
198 |
++ b'Content-Length: 0', |
|
199 |
++ b'Host: host.invalid', |
|
200 |
++ b'', |
|
201 |
++ b'', |
|
202 |
++ b'GET /a HTTP/1.1', |
|
203 |
++ b'Host: host.invalid', |
|
204 |
++ b'', |
|
205 |
++ b'', |
|
206 |
++ ]) |
|
207 |
++ |
|
208 |
++ |
|
209 |
++ def test_unknownTransferEncoding(self): |
|
210 |
++ """ |
|
211 |
++ A request whose C{transfer-encoding} header includes a value |
|
212 |
++ other than C{chunked} or C{identity} fails with a 400 response |
|
213 |
++ without calling L{Request.process}. |
|
214 |
++ """ |
|
215 |
++ self.assertRequestRejected([ |
|
216 |
++ b'GET /a HTTP/1.1', |
|
217 |
++ b'Transfer-Encoding: unknown', |
|
218 |
++ b'Host: host.invalid', |
|
219 |
++ b'', |
|
220 |
++ b'', |
|
221 |
++ ]) |
|
222 |
++ |
|
223 |
++ |
|
224 |
++ def test_transferEncodingIdentity(self): |
|
225 |
++ """ |
|
226 |
++ A request with a valid C{content-length} and a |
|
227 |
++ C{transfer-encoding} whose value is C{identity} succeeds. |
|
228 |
++ """ |
|
229 |
++ body = [] |
|
230 |
++ |
|
231 |
++ class SuccessfulRequest(http.Request): |
|
232 |
++ processed = False |
|
233 |
++ def process(self): |
|
234 |
++ body.append(self.content.read()) |
|
235 |
++ self.setHeader(b'content-length', b'0') |
|
236 |
++ self.finish() |
|
237 |
++ |
|
238 |
++ request = b'''\ |
|
239 |
++GET / HTTP/1.1 |
|
240 |
++Host: host.invalid |
|
241 |
++Content-Length: 2 |
|
242 |
++Transfer-Encoding: identity |
|
243 |
++ |
|
244 |
++ok |
|
245 |
++''' |
|
246 |
++ channel = self.runRequest(request, SuccessfulRequest, False) |
|
247 |
++ self.assertEqual(body, [b'ok']) |
|
248 |
++ self.assertEqual( |
|
249 |
++ channel.transport.value(), |
|
250 |
++ b'HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n', |
|
251 |
++ ) |
|
252 |
++ |
|
253 |
++ |
|
254 |
+ |
|
255 |
+ class QueryArgumentsTests(unittest.TestCase): |
|
256 |
+ def testParseqs(self): |
... | ... |
@@ -4,7 +4,7 @@ |
4 | 4 |
Summary: An asynchronous networking framework written in Python |
5 | 5 |
Name: python-Twisted |
6 | 6 |
Version: 19.10.0 |
7 |
-Release: 3%{?dist} |
|
7 |
+Release: 4%{?dist} |
|
8 | 8 |
License: MIT |
9 | 9 |
Group: Development/Languages/Python |
10 | 10 |
Vendor: VMware, Inc. |
... | ... |
@@ -12,9 +12,9 @@ Distribution: Photon |
12 | 12 |
Url: https://twistedmatrix.com |
13 | 13 |
Source0: https://pypi.python.org/packages/source/T/Twisted/Twisted-%{version}.tar.bz2 |
14 | 14 |
%define sha1 Twisted=38a7f1b9c63ba0d2db553e2d210af2fd01b3ed21 |
15 |
-Patch0: extra_dependency.patch |
|
16 |
-Patch1: no_packet.patch |
|
17 |
- |
|
15 |
+Patch0: extra_dependency.patch |
|
16 |
+Patch1: no_packet.patch |
|
17 |
+Patch2: CVE-2020-10108_10109.patch |
|
18 | 18 |
BuildRequires: python2 |
19 | 19 |
BuildRequires: python2-libs |
20 | 20 |
BuildRequires: python2-devel |
... | ... |
@@ -77,6 +77,7 @@ Python 3 version. |
77 | 77 |
%setup -q -n Twisted-%{version} |
78 | 78 |
%patch0 -p1 |
79 | 79 |
%patch1 -p1 |
80 |
+%patch2 -p1 |
|
80 | 81 |
rm -rf ../p3dir |
81 | 82 |
cp -a . ../p3dir |
82 | 83 |
|
... | ... |
@@ -135,6 +136,8 @@ popd |
135 | 135 |
%{_bindir}/cftp3 |
136 | 136 |
|
137 | 137 |
%changelog |
138 |
+* Sat Jun 27 2020 Tapas Kundu <tkundu@vmware.com> 19.10.0-4 |
|
139 |
+- Address CVE-2020-10108 and CVE-2020-10109 |
|
138 | 140 |
* Mon Jun 01 2020 Tapas Kundu <tkundu@vmware.com> 19.10.0-3 |
139 | 141 |
- Requires service_identity |
140 | 142 |
* Wed Mar 04 2020 Tapas Kundu <tkundu@vmware.com> 19.10.0-2 |