Merge lp:~alecu/ubuntuone-client/proxy-tunnel-cookies into lp:ubuntuone-client

Proposed by Alejandro J. Cura on 2012-03-16
Status: Merged
Approved by: Alejandro J. Cura on 2012-03-18
Approved revision: 1240
Merged at revision: 1212
Proposed branch: lp:~alecu/ubuntuone-client/proxy-tunnel-cookies
Merge into: lp:ubuntuone-client
Prerequisite: lp:~alecu/ubuntuone-client/proxy-tunnel-auth
Diff against target: 480 lines (+121/-36)
7 files modified
tests/proxy/__init__.py (+2/-0)
tests/proxy/test_tunnel_client.py (+22/-8)
tests/proxy/test_tunnel_server.py (+39/-9)
tests/syncdaemon/test_tunnel_runner.py (+6/-2)
ubuntuone/proxy/common.py (+2/-0)
ubuntuone/proxy/tunnel_client.py (+26/-11)
ubuntuone/proxy/tunnel_server.py (+24/-6)
To merge this branch: bzr merge lp:~alecu/ubuntuone-client/proxy-tunnel-cookies
Reviewer Review Type Date Requested Status
Manuel de la Peña (community) Approve on 2012-03-18
Diego Sarmentero (community) 2012-03-16 Approve on 2012-03-16
Review via email: mp+97791@code.launchpad.net

Commit message

- Only allow connections that provide the right cookie thru the tunnel (LP: #929207).

Description of the change

- Only allow connections that provide the right cookie thru the tunnel (LP: #929207).

To post a comment you must log in.
1240. By Alejandro J. Cura on 2012-03-16

Merged proxy-tunnel-auth into proxy-tunnel-cookies.

Diego Sarmentero (diegosarmentero) wrote :

+1

review: Approve
Manuel de la Peña (mandel) wrote :

Looks good from spain.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/proxy/__init__.py'
2--- tests/proxy/__init__.py 2012-02-24 02:30:57 +0000
3+++ tests/proxy/__init__.py 2012-03-16 13:44:20 +0000
4@@ -24,6 +24,7 @@
5 SIMPLERESOURCE = "simpleresource"
6 DUMMY_KEY_FILENAME = "dummy.key"
7 DUMMY_CERT_FILENAME = "dummy.cert"
8+FAKE_COOKIE = "fa:ke:co:ok:ie"
9
10
11 class SaveHTTPChannel(http.HTTPChannel):
12@@ -137,6 +138,7 @@
13
14 connected = True
15 disconnecting = False
16+ cookie = None
17
18 def loseConnection(self):
19 """Mark the connection as lost."""
20
21=== modified file 'tests/proxy/test_tunnel_client.py'
22--- tests/proxy/test_tunnel_client.py 2012-03-14 17:38:31 +0000
23+++ tests/proxy/test_tunnel_client.py 2012-03-16 13:44:20 +0000
24@@ -23,6 +23,7 @@
25
26 from tests.proxy import (
27 FakeTransport,
28+ FAKE_COOKIE,
29 MockWebServer,
30 SAMPLE_CONTENT,
31 SIMPLERESOURCE,
32@@ -72,11 +73,12 @@
33 yield super(TunnelClientProtocolTestCase, self).setUp()
34 self.host, self.port = "9.9.9.9", 8765
35 fake_addr = object()
36+ self.cookie = FAKE_COOKIE
37 self.other_proto = SavingProtocol()
38 other_factory = protocol.ClientFactory()
39 other_factory.buildProtocol = lambda _addr: self.other_proto
40 tunnel_client_factory = tunnel_client.TunnelClientFactory(self.host,
41- self.port, other_factory)
42+ self.port, other_factory, self.cookie)
43 tunnel_client_proto = tunnel_client_factory.buildProtocol(fake_addr)
44 tunnel_client_proto.transport = FakeTransport()
45 tunnel_client_proto.connectionMade()
46@@ -91,6 +93,13 @@
47 self.assertTrue(written.endswith(CRLF * 2),
48 "Ends with a double CRLF")
49
50+ def test_sends_cookie_header(self):
51+ """Sends the expected cookie header."""
52+ expected = "%s: %s" % (tunnel_client.TUNNEL_COOKIE_HEADER, self.cookie)
53+ written = self.tunnel_client_proto.transport.getvalue()
54+ headers = written.split(CRLF)[1:]
55+ self.assertIn(expected, headers)
56+
57 def test_handles_successful_connection(self):
58 """A successful connection is handled."""
59 self.tunnel_client_proto.dataReceived(FAKE_HEADER)
60@@ -132,7 +141,8 @@
61 def test_forwards_started(self):
62 """The factory forwards the startedConnecting call."""
63 fake_other_factory = FakeOtherFactory()
64- tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
65+ tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
66+ FAKE_COOKIE)
67 fake_connector = object()
68 tcf.startedConnecting(fake_connector)
69 self.assertEqual(fake_other_factory.started_called, (fake_connector,))
70@@ -141,7 +151,8 @@
71 """The factory forwards the clientConnectionFailed call."""
72 fake_reason = object()
73 fake_other_factory = FakeOtherFactory()
74- tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
75+ tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
76+ FAKE_COOKIE)
77 fake_connector = object()
78 tcf.clientConnectionFailed(fake_connector, fake_reason)
79 self.assertEqual(fake_other_factory.failed_called,
80@@ -151,7 +162,8 @@
81 """The factory forwards the clientConnectionLost call."""
82 fake_reason = object()
83 fake_other_factory = FakeOtherFactory()
84- tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
85+ tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
86+ FAKE_COOKIE)
87 fake_connector = object()
88 tcf.clientConnectionLost(fake_connector, fake_reason)
89 self.assertEqual(fake_other_factory.lost_called,
90@@ -172,13 +184,15 @@
91 self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
92 self.dest_ssl_url = (self.ws.get_ssl_iri().encode("utf-8") +
93 SIMPLERESOURCE)
94- self.tunnel_server = TunnelServer()
95+ self.cookie = FAKE_COOKIE
96+ self.tunnel_server = TunnelServer(self.cookie)
97 self.addCleanup(self.tunnel_server.shutdown)
98
99 @defer.inlineCallbacks
100 def test_connects_right(self):
101 """Uses the CONNECT method on the tunnel."""
102- tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port)
103+ tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port,
104+ self.cookie)
105 factory = client.HTTPClientFactory(self.dest_url)
106 scheme, host, port, path = client._parse(self.dest_url)
107 tunnel_client.connectTCP(host, port, factory)
108@@ -188,8 +202,8 @@
109 @defer.inlineCallbacks
110 def test_starts_tls_connection(self):
111 """TLS is started after connecting; control passed to the client."""
112- tunnel_client = TunnelClient("0.0.0.0",
113- self.tunnel_server.port)
114+ tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port,
115+ self.cookie)
116 factory = client.HTTPClientFactory(self.dest_ssl_url)
117 scheme, host, port, path = client._parse(self.dest_ssl_url)
118 context_factory = ssl.ClientContextFactory()
119
120=== modified file 'tests/proxy/test_tunnel_server.py'
121--- tests/proxy/test_tunnel_server.py 2012-03-16 13:44:20 +0000
122+++ tests/proxy/test_tunnel_server.py 2012-03-16 13:44:20 +0000
123@@ -26,6 +26,7 @@
124
125 from tests.proxy import (
126 FakeTransport,
127+ FAKE_COOKIE,
128 MockWebServer,
129 SAMPLE_CONTENT,
130 SIMPLERESOURCE,
131@@ -38,6 +39,7 @@
132 "CONNECT %s HTTP/1.0" + CRLF +
133 "Header1: value1" + CRLF +
134 "Header2: value2" + CRLF +
135+ tunnel_server.TUNNEL_COOKIE_HEADER + ": %s" + CRLF +
136 CRLF +
137 "GET %s HTTP/1.0" + CRLF + CRLF
138 )
139@@ -130,7 +132,8 @@
140 self.ws = MockWebServer()
141 self.addCleanup(self.ws.stop)
142 self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
143- self.tunnel_server = tunnel_server.TunnelServer()
144+ self.cookie = FAKE_COOKIE
145+ self.tunnel_server = tunnel_server.TunnelServer(self.cookie)
146 self.addCleanup(self.tunnel_server.shutdown)
147
148 def test_init(self):
149@@ -148,7 +151,8 @@
150 def test_complete_connection(self):
151 """Test from the tunnel server down."""
152 url = urlparse(self.dest_url)
153- fake_session = FAKE_SESSION_TEMPLATE % (url.netloc, url.path)
154+ fake_session = FAKE_SESSION_TEMPLATE % (
155+ url.netloc, self.cookie, url.path)
156 client = FakeClientFactory(fake_session)
157 reactor.connectTCP("0.0.0.0", self.tunnel_server.port, client)
158 response = yield client.response
159@@ -196,11 +200,14 @@
160 self.addCleanup(self.ws.stop)
161 self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
162 self.transport = FakeTransport()
163+ self.transport.cookie = FAKE_COOKIE
164 self.fake_client = FakeClient()
165 self.proto = tunnel_server.ServerTunnelProtocol(
166 lambda _: self.fake_client)
167 self.fake_client.protocol = self.proto
168 self.proto.transport = self.transport
169+ self.cookie_line = "%s: %s" % (tunnel_server.TUNNEL_COOKIE_HEADER,
170+ FAKE_COOKIE)
171
172 def test_broken_request(self):
173 """Broken request."""
174@@ -223,7 +230,8 @@
175 def test_connection_is_established(self):
176 """The response code is sent."""
177 expected = "HTTP/1.0 200 Proxy connection established" + CRLF
178- self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF * 2)
179+ self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF +
180+ self.cookie_line + CRLF * 2)
181 self.assertTrue(self.transport.getvalue().startswith(expected),
182 "First line must be the response status")
183
184@@ -232,7 +240,8 @@
185 error = tunnel_server.ConnectionError()
186 self.patch(self.fake_client, "connection_result", defer.fail(error))
187 expected = "HTTP/1.0 500 Connection error" + CRLF
188- self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF * 2)
189+ self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF +
190+ self.cookie_line + CRLF * 2)
191 self.assertTrue(self.transport.getvalue().startswith(expected),
192 "The connection should fail at this point.")
193
194@@ -247,10 +256,25 @@
195 "Header2: value2" + CRLF + CRLF)
196 self.assertEqual(self.proto.received_headers, expected)
197
198+ def test_cookie_header_present(self):
199+ """The cookie header must be present."""
200+ self.proto.received_headers = [
201+ (tunnel_server.TUNNEL_COOKIE_HEADER, FAKE_COOKIE),
202+ ]
203+ self.proto.verify_cookie()
204+
205+ def test_cookie_header_absent(self):
206+ """The tunnel should refuse connections without the cookie."""
207+ self.proto.received_headers = []
208+ exception = self.assertRaises(tunnel_server.ConnectionError,
209+ self.proto.verify_cookie)
210+ self.assertEqual(exception.code, 418)
211+
212 def test_successful_connect(self):
213 """A successful connect thru the tunnel."""
214 url = urlparse(self.dest_url)
215- data = FAKE_SESSION_TEMPLATE % (url.netloc, url.path)
216+ data = FAKE_SESSION_TEMPLATE % (url.netloc, self.transport.cookie,
217+ url.path)
218 self.proto.dataReceived(data)
219 lines = self.transport.getvalue().split(CRLF)
220 self.assertEqual(lines[-1], SAMPLE_CONTENT)
221@@ -264,6 +288,7 @@
222 def test_keyring_credentials_are_retried(self):
223 """Wrong credentials are retried with values from keyring."""
224 self.fake_client.check_credentials = True
225+ self.patch(self.proto, "verify_cookie", lambda: None)
226 self.patch(self.proto, "error_response",
227 lambda code, desc: self.fail(desc))
228 self.proto.proxy_domain = "xxx"
229@@ -610,12 +635,17 @@
230 tunnel_server.main([])
231 self.assertEqual(len(self.called), 1)
232
233- def test_on_proxies_enabled_prints_port(self):
234+ def test_on_proxies_enabled_prints_port_and_cookie(self):
235 """With proxies enabled print port to stdout and start the mainloop."""
236+ self.patch(tunnel_server.uuid, "uuid4", lambda: FAKE_COOKIE)
237 self.proxies_enabled = True
238- tunnel_server.main(["example.com", "443"])
239- self.assertIn(tunnel_server.TUNNEL_PORT_LABEL + ":",
240- self.fake_stdout.getvalue())
241+ port = 443
242+ tunnel_server.main(["example.com", str(port)])
243+ stdout = self.fake_stdout.getvalue()
244+
245+ self.assertIn(tunnel_server.TUNNEL_PORT_LABEL + ": ", stdout)
246+ cookie_line = tunnel_server.TUNNEL_COOKIE_LABEL + ": " + FAKE_COOKIE
247+ self.assertIn(cookie_line, stdout)
248
249 def test_on_proxies_disabled_exit(self):
250 """With proxies disabled, print a message and exit gracefully."""
251
252=== modified file 'tests/syncdaemon/test_tunnel_runner.py'
253--- tests/syncdaemon/test_tunnel_runner.py 2012-03-12 17:42:57 +0000
254+++ tests/syncdaemon/test_tunnel_runner.py 2012-03-16 13:44:20 +0000
255@@ -18,6 +18,7 @@
256 from twisted.internet import defer, error, reactor, task
257 from twisted.trial.unittest import TestCase
258
259+from tests.proxy import FAKE_COOKIE
260 from ubuntuone.proxy import tunnel_client
261 from ubuntuone.syncdaemon import tunnel_runner
262
263@@ -106,9 +107,12 @@
264 self.assertEqual(client, clock)
265
266 @defer.inlineCallbacks
267- def test_tunnel_process_prints_port_number(self):
268+ def test_tunnel_process_prints_port_number_and_cookie(self):
269 """The tunnel process prints the port number."""
270- received = "%s: %d\n" % (tunnel_client.TUNNEL_PORT_LABEL, FAKE_PORT)
271+ received = "%s: %d\n%s: %s\n" % (
272+ tunnel_client.TUNNEL_PORT_LABEL, FAKE_PORT,
273+ tunnel_client.TUNNEL_COOKIE_LABEL, FAKE_COOKIE)
274 self.process_protocol.outReceived(received)
275 client = yield self.tr.get_client()
276 self.assertEqual(client.tunnel_port, FAKE_PORT)
277+ self.assertEqual(client.cookie, FAKE_COOKIE)
278
279=== modified file 'ubuntuone/proxy/common.py'
280--- ubuntuone/proxy/common.py 2012-03-16 13:44:20 +0000
281+++ ubuntuone/proxy/common.py 2012-03-16 13:44:20 +0000
282@@ -19,6 +19,8 @@
283
284 CRLF = "\r\n"
285 TUNNEL_PORT_LABEL = "Tunnel port"
286+TUNNEL_COOKIE_LABEL = "Tunnel cookie"
287+TUNNEL_COOKIE_HEADER = "Proxy-Tunnel-Cookie"
288
289
290 class BaseTunnelProtocol(basic.LineReceiver):
291
292=== modified file 'ubuntuone/proxy/tunnel_client.py'
293--- ubuntuone/proxy/tunnel_client.py 2012-03-14 17:38:31 +0000
294+++ ubuntuone/proxy/tunnel_client.py 2012-03-16 13:44:20 +0000
295@@ -19,7 +19,13 @@
296
297 from twisted.internet import protocol, reactor
298
299-from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF, TUNNEL_PORT_LABEL
300+from ubuntuone.proxy.common import (
301+ BaseTunnelProtocol,
302+ CRLF,
303+ TUNNEL_COOKIE_LABEL,
304+ TUNNEL_COOKIE_HEADER,
305+ TUNNEL_PORT_LABEL,
306+)
307
308 METHOD_LINE = "CONNECT %s:%d HTTP/1.0" + CRLF
309 LOCALHOST = "127.0.0.1"
310@@ -34,7 +40,8 @@
311 method_line = METHOD_LINE % (self.factory.tunnel_host,
312 self.factory.tunnel_port)
313 headers = {
314- "User-Agent": "Ubuntu One tunnel client"
315+ "User-Agent": "Ubuntu One tunnel client",
316+ TUNNEL_COOKIE_HEADER: self.factory.cookie,
317 }
318 self.transport.write(method_line +
319 self.format_headers(headers) +
320@@ -70,13 +77,14 @@
321
322 protocol = TunnelClientProtocol
323
324- def __init__(self, tunnel_host, tunnel_port, other_factory,
325+ def __init__(self, tunnel_host, tunnel_port, other_factory, cookie,
326 context_factory=None):
327 """Initialize this factory."""
328 self.tunnel_host = tunnel_host
329 self.tunnel_port = tunnel_port
330 self.other_factory = other_factory
331 self.context_factory = context_factory
332+ self.cookie = cookie
333
334 def startedConnecting(self, connector):
335 """Forward this call to the other factory."""
336@@ -94,16 +102,17 @@
337 class TunnelClient(object):
338 """A client for the proxy tunnel."""
339
340- def __init__(self, tunnel_host, tunnel_port):
341+ def __init__(self, tunnel_host, tunnel_port, cookie):
342 """Initialize this client."""
343 self.tunnel_host = tunnel_host
344 self.tunnel_port = tunnel_port
345+ self.cookie = cookie
346
347 def connectTCP(self, host, port, factory, *args, **kwargs):
348 """A connectTCP going thru the tunnel."""
349 logger.info("Connecting (TCP) to %r:%r via tunnel at %r:%r",
350 host, port, self.tunnel_host, self.tunnel_port)
351- tunnel_factory = TunnelClientFactory(host, port, factory)
352+ tunnel_factory = TunnelClientFactory(host, port, factory, self.cookie)
353 return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
354 tunnel_factory, *args, **kwargs)
355
356@@ -112,7 +121,7 @@
357 """A connectSSL going thru the tunnel."""
358 logger.info("Connecting (SSL) to %r:%r via tunnel at %r:%r",
359 host, port, self.tunnel_host, self.tunnel_port)
360- tunnel_factory = TunnelClientFactory(host, port, factory,
361+ tunnel_factory = TunnelClientFactory(host, port, factory, self.cookie,
362 contextFactory)
363 return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
364 tunnel_factory, *args, **kwargs)
365@@ -127,6 +136,8 @@
366 """Initialize this protocol."""
367 self.client_d = client_d
368 self.timer = None
369+ self.port = None
370+ self.cookie = None
371
372 def connectionMade(self):
373 """The process has started, start a timer."""
374@@ -159,8 +170,12 @@
375 for line in data.split("\n"):
376 if line.startswith(TUNNEL_PORT_LABEL):
377 _header, port = line.split(":", 1)
378- port = int(port.strip())
379- logger.info("Tunnel process listening on port %r.", port)
380- client = TunnelClient(LOCALHOST, port)
381- self.client_d.callback(client)
382- break
383+ self.port = int(port.strip())
384+ if line.startswith(TUNNEL_COOKIE_LABEL):
385+ _header, cookie = line.split(":", 1)
386+ self.cookie = cookie.strip()
387+
388+ if self.port and self.cookie:
389+ logger.info("Tunnel process listening on port %r.", self.port)
390+ client = TunnelClient(LOCALHOST, self.port, self.cookie)
391+ self.client_d.callback(client)
392
393=== modified file 'ubuntuone/proxy/tunnel_server.py'
394--- ubuntuone/proxy/tunnel_server.py 2012-03-16 13:44:20 +0000
395+++ ubuntuone/proxy/tunnel_server.py 2012-03-16 13:44:20 +0000
396@@ -30,6 +30,7 @@
397 """
398
399 import sys
400+import uuid
401
402 from PyQt4.QtCore import QCoreApplication, QTimer
403 from PyQt4.QtNetwork import (
404@@ -46,7 +47,13 @@
405
406 from ubuntu_sso.keyring import Keyring
407 from ubuntu_sso.utils.webclient import gsettings
408-from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF, TUNNEL_PORT_LABEL
409+from ubuntuone.proxy.common import (
410+ BaseTunnelProtocol,
411+ CRLF,
412+ TUNNEL_COOKIE_HEADER,
413+ TUNNEL_COOKIE_LABEL,
414+ TUNNEL_PORT_LABEL,
415+)
416 from ubuntuone.proxy.logger import logger
417
418 DEFAULT_CODE = 500
419@@ -219,10 +226,17 @@
420 except ValueError:
421 self.error_response(400, "Bad request")
422
423+ def verify_cookie(self):
424+ """Fail if the cookie is wrong or missing."""
425+ cookie_received = dict(self.received_headers).get(TUNNEL_COOKIE_HEADER)
426+ if cookie_received != self.transport.cookie:
427+ raise ConnectionError(418, "Please see RFC 2324")
428+
429 @defer.inlineCallbacks
430 def headers_done(self):
431 """An empty line was received, start connecting and switch mode."""
432 try:
433+ self.verify_cookie()
434 try:
435 logger.info("Connecting once")
436 self.client = self.client_class(self)
437@@ -267,8 +281,9 @@
438
439 implements(interfaces.ITransport)
440
441- def __init__(self, local_socket):
442+ def __init__(self, local_socket, cookie):
443 """Initialize this Tunnel instance."""
444+ self.cookie = cookie
445 self.disconnecting = False
446 self.local_socket = local_socket
447 self.protocol = ServerTunnelProtocol(RemoteSocket)
448@@ -300,9 +315,10 @@
449 class TunnelServer(object):
450 """A server for tunnel instances."""
451
452- def __init__(self):
453+ def __init__(self, cookie):
454 """Initialize this tunnel instance."""
455 self.tunnels = []
456+ self.cookie = cookie
457 self.server = QTcpServer(QCoreApplication.instance())
458 self.server.newConnection.connect(self.new_connection)
459 self.server.listen(QHostAddress.LocalHost, 0)
460@@ -312,7 +328,7 @@
461 """On a new connection create a new tunnel instance."""
462 logger.info("New connection made")
463 local_socket = self.server.nextPendingConnection()
464- tunnel = Tunnel(local_socket)
465+ tunnel = Tunnel(local_socket, self.cookie)
466 self.tunnels.append(tunnel)
467
468 def shutdown(self):
469@@ -352,7 +368,9 @@
470 from dbus.mainloop.qt import DBusQtMainLoop
471 DBusQtMainLoop(set_as_default=True)
472 app = QCoreApplication(argv)
473- tunnel_server = TunnelServer()
474- sys.stdout.write("%s: %d\n" % (TUNNEL_PORT_LABEL, tunnel_server.port))
475+ cookie = str(uuid.uuid4())
476+ tunnel_server = TunnelServer(cookie)
477+ sys.stdout.write("%s: %d\n" % (TUNNEL_PORT_LABEL, tunnel_server.port) +
478+ "%s: %s\n" % (TUNNEL_COOKIE_LABEL, cookie))
479 sys.stdout.flush()
480 app.exec_()

Subscribers

People subscribed via source and target branches