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

[Alejandro J. Cura]

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

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

[lp:~alecu/ubuntuone-client/proxy-tunnel-cookies]

1240. By Alejandro J. Cura on 2012-03-16

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

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[Manuel de la Peña (mandel)]

Looks good from spain.

review: Approve

=== modified file 'tests/proxy/__init__.py'
--- tests/proxy/__init__.py	2012-02-24 02:30:57 +0000
+++ tests/proxy/__init__.py	2012-03-16 13:44:20 +0000
@@ -24,6 +24,7 @@
 SIMPLERESOURCE = "simpleresource"
 DUMMY_KEY_FILENAME = "dummy.key"
 DUMMY_CERT_FILENAME = "dummy.cert"
+FAKE_COOKIE = "fa:ke:co:ok:ie"
 
 
 class SaveHTTPChannel(http.HTTPChannel):
@@ -137,6 +138,7 @@
 
     connected = True
     disconnecting = False
+    cookie = None
 
     def loseConnection(self):
         """Mark the connection as lost."""

=== modified file 'tests/proxy/test_tunnel_client.py'
--- tests/proxy/test_tunnel_client.py	2012-03-14 17:38:31 +0000
+++ tests/proxy/test_tunnel_client.py	2012-03-16 13:44:20 +0000
@@ -23,6 +23,7 @@
 
 from tests.proxy import (
     FakeTransport,
+    FAKE_COOKIE,
     MockWebServer,
     SAMPLE_CONTENT,
     SIMPLERESOURCE,
@@ -72,11 +73,12 @@
         yield super(TunnelClientProtocolTestCase, self).setUp()
         self.host, self.port = "9.9.9.9", 8765
         fake_addr = object()
+        self.cookie = FAKE_COOKIE
         self.other_proto = SavingProtocol()
         other_factory = protocol.ClientFactory()
         other_factory.buildProtocol = lambda _addr: self.other_proto
         tunnel_client_factory = tunnel_client.TunnelClientFactory(self.host,
-                                                      self.port, other_factory)
+                                         self.port, other_factory, self.cookie)
         tunnel_client_proto = tunnel_client_factory.buildProtocol(fake_addr)
         tunnel_client_proto.transport = FakeTransport()
         tunnel_client_proto.connectionMade()
@@ -91,6 +93,13 @@
         self.assertTrue(written.endswith(CRLF * 2),
                         "Ends with a double CRLF")
 
+    def test_sends_cookie_header(self):
+        """Sends the expected cookie header."""
+        expected = "%s: %s" % (tunnel_client.TUNNEL_COOKIE_HEADER, self.cookie)
+        written = self.tunnel_client_proto.transport.getvalue()
+        headers = written.split(CRLF)[1:]
+        self.assertIn(expected, headers)
+
     def test_handles_successful_connection(self):
         """A successful connection is handled."""
         self.tunnel_client_proto.dataReceived(FAKE_HEADER)
@@ -132,7 +141,8 @@
     def test_forwards_started(self):
         """The factory forwards the startedConnecting call."""
         fake_other_factory = FakeOtherFactory()
-        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
+        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
+                                                FAKE_COOKIE)
         fake_connector = object()
         tcf.startedConnecting(fake_connector)
         self.assertEqual(fake_other_factory.started_called, (fake_connector,))
@@ -141,7 +151,8 @@
         """The factory forwards the clientConnectionFailed call."""
         fake_reason = object()
         fake_other_factory = FakeOtherFactory()
-        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
+        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
+                                                FAKE_COOKIE)
         fake_connector = object()
         tcf.clientConnectionFailed(fake_connector, fake_reason)
         self.assertEqual(fake_other_factory.failed_called,
@@ -151,7 +162,8 @@
         """The factory forwards the clientConnectionLost call."""
         fake_reason = object()
         fake_other_factory = FakeOtherFactory()
-        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory)
+        tcf = tunnel_client.TunnelClientFactory(None, None, fake_other_factory,
+                                                FAKE_COOKIE)
         fake_connector = object()
         tcf.clientConnectionLost(fake_connector, fake_reason)
         self.assertEqual(fake_other_factory.lost_called,
@@ -172,13 +184,15 @@
         self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
         self.dest_ssl_url = (self.ws.get_ssl_iri().encode("utf-8") +
                              SIMPLERESOURCE)
-        self.tunnel_server = TunnelServer()
+        self.cookie = FAKE_COOKIE
+        self.tunnel_server = TunnelServer(self.cookie)
         self.addCleanup(self.tunnel_server.shutdown)
 
     @defer.inlineCallbacks
     def test_connects_right(self):
         """Uses the CONNECT method on the tunnel."""
-        tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port)
+        tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port,
+                                     self.cookie)
         factory = client.HTTPClientFactory(self.dest_url)
         scheme, host, port, path = client._parse(self.dest_url)
         tunnel_client.connectTCP(host, port, factory)
@@ -188,8 +202,8 @@
     @defer.inlineCallbacks
     def test_starts_tls_connection(self):
         """TLS is started after connecting; control passed to the client."""
-        tunnel_client = TunnelClient("0.0.0.0",
-                                             self.tunnel_server.port)
+        tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port,
+                                     self.cookie)
         factory = client.HTTPClientFactory(self.dest_ssl_url)
         scheme, host, port, path = client._parse(self.dest_ssl_url)
         context_factory = ssl.ClientContextFactory()

=== modified file 'tests/proxy/test_tunnel_server.py'
--- tests/proxy/test_tunnel_server.py	2012-03-16 13:44:20 +0000
+++ tests/proxy/test_tunnel_server.py	2012-03-16 13:44:20 +0000
@@ -26,6 +26,7 @@
 
 from tests.proxy import (
     FakeTransport,
+    FAKE_COOKIE,
     MockWebServer,
     SAMPLE_CONTENT,
     SIMPLERESOURCE,
@@ -38,6 +39,7 @@
     "CONNECT %s HTTP/1.0" + CRLF +
     "Header1: value1" + CRLF +
     "Header2: value2" + CRLF +
+    tunnel_server.TUNNEL_COOKIE_HEADER + ": %s" + CRLF +
     CRLF +
     "GET %s HTTP/1.0" + CRLF + CRLF
 )
@@ -130,7 +132,8 @@
         self.ws = MockWebServer()
         self.addCleanup(self.ws.stop)
         self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
-        self.tunnel_server = tunnel_server.TunnelServer()
+        self.cookie = FAKE_COOKIE
+        self.tunnel_server = tunnel_server.TunnelServer(self.cookie)
         self.addCleanup(self.tunnel_server.shutdown)
 
     def test_init(self):
@@ -148,7 +151,8 @@
     def test_complete_connection(self):
         """Test from the tunnel server down."""
         url = urlparse(self.dest_url)
-        fake_session = FAKE_SESSION_TEMPLATE % (url.netloc, url.path)
+        fake_session = FAKE_SESSION_TEMPLATE % (
+                                    url.netloc, self.cookie, url.path)
         client = FakeClientFactory(fake_session)
         reactor.connectTCP("0.0.0.0", self.tunnel_server.port, client)
         response = yield client.response
@@ -196,11 +200,14 @@
         self.addCleanup(self.ws.stop)
         self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
         self.transport = FakeTransport()
+        self.transport.cookie = FAKE_COOKIE
         self.fake_client = FakeClient()
         self.proto = tunnel_server.ServerTunnelProtocol(
                                                 lambda _: self.fake_client)
         self.fake_client.protocol = self.proto
         self.proto.transport = self.transport
+        self.cookie_line = "%s: %s" % (tunnel_server.TUNNEL_COOKIE_HEADER,
+                                       FAKE_COOKIE)
 
     def test_broken_request(self):
         """Broken request."""
@@ -223,7 +230,8 @@
     def test_connection_is_established(self):
         """The response code is sent."""
         expected = "HTTP/1.0 200 Proxy connection established" + CRLF
-        self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF * 2)
+        self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF +
+                                self.cookie_line + CRLF * 2)
         self.assertTrue(self.transport.getvalue().startswith(expected),
                         "First line must be the response status")
 
@@ -232,7 +240,8 @@
         error = tunnel_server.ConnectionError()
         self.patch(self.fake_client, "connection_result", defer.fail(error))
         expected = "HTTP/1.0 500 Connection error" + CRLF
-        self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF * 2)
+        self.proto.dataReceived("CONNECT 127.0.0.1:9999 HTTP/1.0" + CRLF +
+                                self.cookie_line + CRLF * 2)
         self.assertTrue(self.transport.getvalue().startswith(expected),
                         "The connection should fail at this point.")
 
@@ -247,10 +256,25 @@
                                 "Header2: value2" + CRLF + CRLF)
         self.assertEqual(self.proto.received_headers, expected)
 
+    def test_cookie_header_present(self):
+        """The cookie header must be present."""
+        self.proto.received_headers = [
+            (tunnel_server.TUNNEL_COOKIE_HEADER, FAKE_COOKIE),
+        ]
+        self.proto.verify_cookie()
+
+    def test_cookie_header_absent(self):
+        """The tunnel should refuse connections without the cookie."""
+        self.proto.received_headers = []
+        exception = self.assertRaises(tunnel_server.ConnectionError,
+                                      self.proto.verify_cookie)
+        self.assertEqual(exception.code, 418)
+
     def test_successful_connect(self):
         """A successful connect thru the tunnel."""
         url = urlparse(self.dest_url)
-        data = FAKE_SESSION_TEMPLATE % (url.netloc, url.path)
+        data = FAKE_SESSION_TEMPLATE % (url.netloc, self.transport.cookie,
+                                        url.path)
         self.proto.dataReceived(data)
         lines = self.transport.getvalue().split(CRLF)
         self.assertEqual(lines[-1], SAMPLE_CONTENT)
@@ -264,6 +288,7 @@
     def test_keyring_credentials_are_retried(self):
         """Wrong credentials are retried with values from keyring."""
         self.fake_client.check_credentials = True
+        self.patch(self.proto, "verify_cookie", lambda: None)
         self.patch(self.proto, "error_response",
                    lambda code, desc: self.fail(desc))
         self.proto.proxy_domain = "xxx"
@@ -610,12 +635,17 @@
         tunnel_server.main([])
         self.assertEqual(len(self.called), 1)
 
-    def test_on_proxies_enabled_prints_port(self):
+    def test_on_proxies_enabled_prints_port_and_cookie(self):
         """With proxies enabled print port to stdout and start the mainloop."""
+        self.patch(tunnel_server.uuid, "uuid4", lambda: FAKE_COOKIE)
         self.proxies_enabled = True
-        tunnel_server.main(["example.com", "443"])
-        self.assertIn(tunnel_server.TUNNEL_PORT_LABEL + ":",
-                      self.fake_stdout.getvalue())
+        port = 443
+        tunnel_server.main(["example.com", str(port)])
+        stdout = self.fake_stdout.getvalue()
+
+        self.assertIn(tunnel_server.TUNNEL_PORT_LABEL + ": ", stdout)
+        cookie_line = tunnel_server.TUNNEL_COOKIE_LABEL + ": " + FAKE_COOKIE
+        self.assertIn(cookie_line, stdout)
 
     def test_on_proxies_disabled_exit(self):
         """With proxies disabled, print a message and exit gracefully."""

=== modified file 'tests/syncdaemon/test_tunnel_runner.py'
--- tests/syncdaemon/test_tunnel_runner.py	2012-03-12 17:42:57 +0000
+++ tests/syncdaemon/test_tunnel_runner.py	2012-03-16 13:44:20 +0000
@@ -18,6 +18,7 @@
 from twisted.internet import defer, error, reactor, task
 from twisted.trial.unittest import TestCase
 
+from tests.proxy import FAKE_COOKIE
 from ubuntuone.proxy import tunnel_client
 from ubuntuone.syncdaemon import tunnel_runner
 
@@ -106,9 +107,12 @@
         self.assertEqual(client, clock)
 
     @defer.inlineCallbacks
-    def test_tunnel_process_prints_port_number(self):
+    def test_tunnel_process_prints_port_number_and_cookie(self):
         """The tunnel process prints the port number."""
-        received = "%s: %d\n" % (tunnel_client.TUNNEL_PORT_LABEL, FAKE_PORT)
+        received = "%s: %d\n%s: %s\n" % (
+                                tunnel_client.TUNNEL_PORT_LABEL, FAKE_PORT,
+                                tunnel_client.TUNNEL_COOKIE_LABEL, FAKE_COOKIE)
         self.process_protocol.outReceived(received)
         client = yield self.tr.get_client()
         self.assertEqual(client.tunnel_port, FAKE_PORT)
+        self.assertEqual(client.cookie, FAKE_COOKIE)

=== modified file 'ubuntuone/proxy/common.py'
--- ubuntuone/proxy/common.py	2012-03-16 13:44:20 +0000
+++ ubuntuone/proxy/common.py	2012-03-16 13:44:20 +0000
@@ -19,6 +19,8 @@
 
 CRLF = "\r\n"
 TUNNEL_PORT_LABEL = "Tunnel port"
+TUNNEL_COOKIE_LABEL = "Tunnel cookie"
+TUNNEL_COOKIE_HEADER = "Proxy-Tunnel-Cookie"
 
 
 class BaseTunnelProtocol(basic.LineReceiver):

=== modified file 'ubuntuone/proxy/tunnel_client.py'
--- ubuntuone/proxy/tunnel_client.py	2012-03-14 17:38:31 +0000
+++ ubuntuone/proxy/tunnel_client.py	2012-03-16 13:44:20 +0000
@@ -19,7 +19,13 @@
 
 from twisted.internet import protocol, reactor
 
-from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF, TUNNEL_PORT_LABEL
+from ubuntuone.proxy.common import (
+    BaseTunnelProtocol,
+    CRLF,
+    TUNNEL_COOKIE_LABEL,
+    TUNNEL_COOKIE_HEADER,
+    TUNNEL_PORT_LABEL,
+)
 
 METHOD_LINE = "CONNECT %s:%d HTTP/1.0" + CRLF
 LOCALHOST = "127.0.0.1"
@@ -34,7 +40,8 @@
         method_line = METHOD_LINE % (self.factory.tunnel_host,
                                      self.factory.tunnel_port)
         headers = {
-            "User-Agent": "Ubuntu One tunnel client"
+            "User-Agent": "Ubuntu One tunnel client",
+            TUNNEL_COOKIE_HEADER: self.factory.cookie,
         }
         self.transport.write(method_line +
                              self.format_headers(headers) +
@@ -70,13 +77,14 @@
 
     protocol = TunnelClientProtocol
 
-    def __init__(self, tunnel_host, tunnel_port, other_factory,
+    def __init__(self, tunnel_host, tunnel_port, other_factory, cookie,
                  context_factory=None):
         """Initialize this factory."""
         self.tunnel_host = tunnel_host
         self.tunnel_port = tunnel_port
         self.other_factory = other_factory
         self.context_factory = context_factory
+        self.cookie = cookie
 
     def startedConnecting(self, connector):
         """Forward this call to the other factory."""
@@ -94,16 +102,17 @@
 class TunnelClient(object):
     """A client for the proxy tunnel."""
 
-    def __init__(self, tunnel_host, tunnel_port):
+    def __init__(self, tunnel_host, tunnel_port, cookie):
         """Initialize this client."""
         self.tunnel_host = tunnel_host
         self.tunnel_port = tunnel_port
+        self.cookie = cookie
 
     def connectTCP(self, host, port, factory, *args, **kwargs):
         """A connectTCP going thru the tunnel."""
         logger.info("Connecting (TCP) to %r:%r via tunnel at %r:%r",
                     host, port, self.tunnel_host, self.tunnel_port)
-        tunnel_factory = TunnelClientFactory(host, port, factory)
+        tunnel_factory = TunnelClientFactory(host, port, factory, self.cookie)
         return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
                                   tunnel_factory, *args, **kwargs)
 
@@ -112,7 +121,7 @@
         """A connectSSL going thru the tunnel."""
         logger.info("Connecting (SSL) to %r:%r via tunnel at %r:%r",
                     host, port, self.tunnel_host, self.tunnel_port)
-        tunnel_factory = TunnelClientFactory(host, port, factory,
+        tunnel_factory = TunnelClientFactory(host, port, factory, self.cookie,
                                       contextFactory)
         return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
                                   tunnel_factory, *args, **kwargs)
@@ -127,6 +136,8 @@
         """Initialize this protocol."""
         self.client_d = client_d
         self.timer = None
+        self.port = None
+        self.cookie = None
 
     def connectionMade(self):
         """The process has started, start a timer."""
@@ -159,8 +170,12 @@
         for line in data.split("\n"):
             if line.startswith(TUNNEL_PORT_LABEL):
                 _header, port = line.split(":", 1)
-                port = int(port.strip())
-                logger.info("Tunnel process listening on port %r.", port)
-                client = TunnelClient(LOCALHOST, port)
-                self.client_d.callback(client)
-                break
+                self.port = int(port.strip())
+            if line.startswith(TUNNEL_COOKIE_LABEL):
+                _header, cookie = line.split(":", 1)
+                self.cookie = cookie.strip()
+
+        if self.port and self.cookie:
+            logger.info("Tunnel process listening on port %r.", self.port)
+            client = TunnelClient(LOCALHOST, self.port, self.cookie)
+            self.client_d.callback(client)

=== modified file 'ubuntuone/proxy/tunnel_server.py'
--- ubuntuone/proxy/tunnel_server.py	2012-03-16 13:44:20 +0000
+++ ubuntuone/proxy/tunnel_server.py	2012-03-16 13:44:20 +0000
@@ -30,6 +30,7 @@
 """
 
 import sys
+import uuid
 
 from PyQt4.QtCore import QCoreApplication, QTimer
 from PyQt4.QtNetwork import (
@@ -46,7 +47,13 @@
 
 from ubuntu_sso.keyring import Keyring
 from ubuntu_sso.utils.webclient import gsettings
-from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF, TUNNEL_PORT_LABEL
+from ubuntuone.proxy.common import (
+    BaseTunnelProtocol,
+    CRLF,
+    TUNNEL_COOKIE_HEADER,
+    TUNNEL_COOKIE_LABEL,
+    TUNNEL_PORT_LABEL,
+)
 from ubuntuone.proxy.logger import logger
 
 DEFAULT_CODE = 500
@@ -219,10 +226,17 @@
         except ValueError:
             self.error_response(400, "Bad request")
 
+    def verify_cookie(self):
+        """Fail if the cookie is wrong or missing."""
+        cookie_received = dict(self.received_headers).get(TUNNEL_COOKIE_HEADER)
+        if cookie_received != self.transport.cookie:
+            raise ConnectionError(418, "Please see RFC 2324")
+
     @defer.inlineCallbacks
     def headers_done(self):
         """An empty line was received, start connecting and switch mode."""
         try:
+            self.verify_cookie()
             try:
                 logger.info("Connecting once")
                 self.client = self.client_class(self)
@@ -267,8 +281,9 @@
 
     implements(interfaces.ITransport)
 
-    def __init__(self, local_socket):
+    def __init__(self, local_socket, cookie):
         """Initialize this Tunnel instance."""
+        self.cookie = cookie
         self.disconnecting = False
         self.local_socket = local_socket
         self.protocol = ServerTunnelProtocol(RemoteSocket)
@@ -300,9 +315,10 @@
 class TunnelServer(object):
     """A server for tunnel instances."""
 
-    def __init__(self):
+    def __init__(self, cookie):
         """Initialize this tunnel instance."""
         self.tunnels = []
+        self.cookie = cookie
         self.server = QTcpServer(QCoreApplication.instance())
         self.server.newConnection.connect(self.new_connection)
         self.server.listen(QHostAddress.LocalHost, 0)
@@ -312,7 +328,7 @@
         """On a new connection create a new tunnel instance."""
         logger.info("New connection made")
         local_socket = self.server.nextPendingConnection()
-        tunnel = Tunnel(local_socket)
+        tunnel = Tunnel(local_socket, self.cookie)
         self.tunnels.append(tunnel)
 
     def shutdown(self):
@@ -352,7 +368,9 @@
         from dbus.mainloop.qt import DBusQtMainLoop
         DBusQtMainLoop(set_as_default=True)
         app = QCoreApplication(argv)
-        tunnel_server = TunnelServer()
-        sys.stdout.write("%s: %d\n" % (TUNNEL_PORT_LABEL, tunnel_server.port))
+        cookie = str(uuid.uuid4())
+        tunnel_server = TunnelServer(cookie)
+        sys.stdout.write("%s: %d\n" % (TUNNEL_PORT_LABEL, tunnel_server.port) +
+                         "%s: %s\n" % (TUNNEL_COOKIE_LABEL, cookie))
         sys.stdout.flush()
         app.exec_()