Backport security fix #3415

docs/topics/telnetconsole.rst

@@ -16,6 +16,17 @@ The telnet console is a :ref:`built-in Scrapy extension
 disable it if you want. For more information about the extension itself see
 :ref:`topics-extensions-ref-telnetconsole`.
 
+.. warning::
+    It is not secure to use telnet console via public networks, as telnet
+    doesn't provide any transport-layer security. Having username/password
+    authentication doesn't change that.
+
+    Intended usage is connecting to a running Scrapy spider locally
+    (spider process and telnet client are on the same machine)
+    or over a secure connection (VPN, SSH tunnel).
+    Please avoid using telnet console over insecure connections,
+    or disable it completely using :setting:`TELNETCONSOLE_ENABLED` option.
+
 .. highlight:: none
 
 How to access the telnet console
@@ -26,8 +37,26 @@ The telnet console listens in the TCP port defined in the
 the console you need to type::
 
     telnet localhost 6023
+    Trying localhost...
+    Connected to localhost.
+    Escape character is '^]'.
+    Username:
+    Password:  
     >>>
 
+By default Username is ``scrapy`` and Password is autogenerated. The 
+autogenerated Password can be seen on scrapy logs like the example bellow::
+
+    2018-10-16 14:35:21 [scrapy.extensions.telnet] INFO: Telnet Password: 16f92501e8a59326
+
+Default Username and Password can be overriden by the settings 
+:setting:`TELNETCONSOLE_USERNAME` and :setting:`TELNETCONSOLE_PASSWORD`.
+
+.. warning::
+    Username and password provide only a limited protection, as telnet
+    is not using secure transport - by default traffic is not encrypted
+    even if username and password are set.
+
 You need the telnet program which comes installed by default in Windows, and
 most Linux distros.
 
@@ -160,3 +189,24 @@ Default: ``'127.0.0.1'``
 
 The interface the telnet console should listen on
 
+
+.. setting:: TELNETCONSOLE_USERNAME
+
+TELNETCONSOLE_USERNAME
+----------------------
+
+Default: ``'scrapy'``
+
+The username used for the telnet console
+
+
+.. setting:: TELNETCONSOLE_PASSWORD
+
+TELNETCONSOLE_PASSWORD
+----------------------
+
+Default: ``None``
+
+The password used for the telnet console, default behaviour is to have it
+autogenerated
+

scrapy/extensions/telnet.py

@@ -4,8 +4,10 @@ Scrapy Telnet Console extension
 See documentation in docs/topics/telnetconsole.rst
 """
 
+import os
 import pprint
 import logging
+import binascii
 
 from twisted.internet import protocol
 try:
@@ -20,6 +22,7 @@ from scrapy import signals
 from scrapy.utils.trackref import print_live_refs
 from scrapy.utils.engine import print_engine_status
 from scrapy.utils.reactor import listen_tcp
+from scrapy.utils.decorators import defers
 
 try:
     import guppy
@@ -45,6 +48,13 @@ class TelnetConsole(protocol.ServerFactory):
         self.noisy = False
         self.portrange = [int(x) for x in crawler.settings.getlist('TELNETCONSOLE_PORT')]
         self.host = crawler.settings['TELNETCONSOLE_HOST']
+        self.username = crawler.settings['TELNETCONSOLE_USERNAME']
+        self.password = crawler.settings['TELNETCONSOLE_PASSWORD']
+
+        if not self.password:
+            self.password = binascii.hexlify(os.urandom(8)).decode('utf8')
+            logger.info('Telnet Password: %s', self.password)
+
         self.crawler.signals.connect(self.start_listening, signals.engine_started)
         self.crawler.signals.connect(self.stop_listening, signals.engine_stopped)
 
@@ -63,9 +73,25 @@ class TelnetConsole(protocol.ServerFactory):
         self.port.stopListening()
 
     def protocol(self):
-        telnet_vars = self._get_telnet_vars()
+        class Portal:
-        return telnet.TelnetTransport(telnet.TelnetBootstrapProtocol,
+            """An implementation of IPortal"""
-            insults.ServerProtocol, manhole.Manhole, telnet_vars)
+            @defers
+            def login(self_, credentials, mind, *interfaces):
+                if not (credentials.username == self.username.encode('utf8') and
+                        credentials.checkPassword(self.password.encode('utf8'))):
+                    raise ValueError("Invalid credentials")
+
+                protocol = telnet.TelnetBootstrapProtocol(
+                    insults.ServerProtocol,
+                    manhole.Manhole,
+                    self._get_telnet_vars()
+                )
+                return (interfaces[0], protocol, lambda: None)
+
+        return telnet.TelnetTransport(
+            telnet.AuthenticatingTelnetProtocol,
+            Portal()
+        )
 
     def _get_telnet_vars(self):
         # Note: if you add entries here also update topics/telnetconsole.rst
@@ -81,8 +107,8 @@ class TelnetConsole(protocol.ServerFactory):
             'p': pprint.pprint,
             'prefs': print_live_refs,
             'hpy': hpy,
-            'help': "This is Scrapy telnet console. For more info see: " \
+            'help': "This is Scrapy telnet console. For more info see: "
-                "https://doc.scrapy.org/en/latest/topics/telnetconsole.html",
+                    "https://doc.scrapy.org/en/latest/topics/telnetconsole.html",
         }
         self.crawler.signals.send_catch_log(update_telnet_vars, telnet_vars=telnet_vars)
         return telnet_vars

scrapy/settings/default_settings.py

@@ -275,6 +275,8 @@ USER_AGENT = 'Scrapy/%s (+https://scrapy.org)' % import_module('scrapy').__versi
 TELNETCONSOLE_ENABLED = 1
 TELNETCONSOLE_PORT = [6023, 6073]
 TELNETCONSOLE_HOST = '127.0.0.1'
+TELNETCONSOLE_USERNAME = 'scrapy'
+TELNETCONSOLE_PASSWORD = None
 
 SPIDER_CONTRACTS = {}
 SPIDER_CONTRACTS_BASE = {

tests/test_extension_telnet.py

@@ -0,0 +1,60 @@
+try:
+    import unittest.mock as mock
+except ImportError:
+    import mock
+
+from twisted.trial import unittest
+from twisted.conch.telnet import ITelnetProtocol
+from twisted.cred import credentials
+from twisted.internet import defer
+
+from scrapy.extensions.telnet import TelnetConsole, logger
+from scrapy.utils.test import get_crawler
+
+
+class TelnetExtensionTest(unittest.TestCase):
+    def _get_console_and_portal(self, settings=None):
+        crawler = get_crawler(settings_dict=settings)
+        console = TelnetConsole(crawler)
+        username = console.username
+        password = console.password
+
+        # This function has some side effects we don't need for this test
+        console._get_telnet_vars = lambda: {}
+
+        console.start_listening()
+        protocol = console.protocol()
+        portal = protocol.protocolArgs[0]
+
+        return console, portal
+
+    @defer.inlineCallbacks
+    def test_bad_credentials(self):
+        console, portal = self._get_console_and_portal()
+        creds = credentials.UsernamePassword(b'username', b'password')
+        d = portal.login(creds, None, ITelnetProtocol)
+        yield self.assertFailure(d, ValueError)
+        console.stop_listening()
+
+    @defer.inlineCallbacks
+    def test_good_credentials(self):
+        console, portal = self._get_console_and_portal()
+        creds = credentials.UsernamePassword(
+            console.username.encode('utf8'),
+            console.password.encode('utf8')
+        )
+        d = portal.login(creds, None, ITelnetProtocol)
+        yield d
+        console.stop_listening()
+
+    @defer.inlineCallbacks
+    def test_custom_credentials(self):
+        settings = {
+            'TELNETCONSOLE_USERNAME': 'user',
+            'TELNETCONSOLE_PASSWORD': 'pass',
+        }
+        console, portal = self._get_console_and_portal(settings=settings)
+        creds = credentials.UsernamePassword(b'user', b'pass')
+        d = portal.login(creds, None, ITelnetProtocol)
+        yield d
+        console.stop_listening()