Merge pull request #3426 from rodrigc/manhole-py3

Update manhole to work with Python 3, and Twisted 16.0.0

Fixes #3160

master/buildbot/config.py

@@ -521,7 +521,7 @@ def copy_str_or_callable_param(name, alt_key=None):
         if 'manhole' in config_dict:
             # we don't check that this is a manhole instance, since that
             # requires importing buildbot.manhole for every user, and currently
-            # that will fail if pycrypto isn't installed
+            # that will fail if cryptography isn't installed
             self.manhole = config_dict['manhole']
 
         if 'revlink' in config_dict:

master/buildbot/manhole.py

@@ -16,12 +16,12 @@
 from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
+from future.utils import string_types
 
 import base64
 import binascii
 import os
 import types
-from builtins import str
 
 from twisted.application import strports
 from twisted.conch import manhole
@@ -36,14 +36,17 @@
 from buildbot import config
 from buildbot.util import ComparableMixin
 from buildbot.util import service
+from buildbot.util import unicode2bytes
 
 try:
     from twisted.conch import checkers as conchc, manhole_ssh
-    _hush_pyflakes = [manhole_ssh, conchc]
+    from twisted.conch.openssh_compat.factory import OpenSSHFactory
+    _hush_pyflakes = [manhole_ssh, conchc, OpenSSHFactory]
     del _hush_pyflakes
 except ImportError:
     manhole_ssh = None
     conchc = None
+    OpenSSHFactory = None
 
 
 # makeTelnetProtocol and _TelnetRealm are for the TelnetManhole
@@ -108,7 +111,7 @@ def __init__(self, authorized_keys_file):
                 authorized_keys_file)
 
         def checkKey(self, credentials):
-            with open(self.authorized_keys_file) as f:
+            with open(self.authorized_keys_file, "rb") as f:
                 for l in f.readlines():
                     l2 = l.split()
                     if len(l2) < 2:
@@ -129,7 +132,7 @@ class _BaseManhole(service.AsyncMultiService):
     buildbot developers. Connect to this by running an ssh client.
     """
 
-    def __init__(self, port, checker, using_ssh=True):
+    def __init__(self, port, checker, ssh_hostkey_dir=None):
         """
         @type port: string or int
         @param port: what port should the Manhole listen on? This is a
@@ -148,9 +151,9 @@ def __init__(self, port, checker, using_ssh=True):
             c = credc.FilePasswordDB(passwd_filename) # file of name:passwd
             c = conchc.UNIXPasswordDatabase # getpwnam() (probably /etc/passwd)
 
-        @type using_ssh: bool
-        @param using_ssh: If True, accept SSH connections. If False, accept
-                          regular unencrypted telnet connections.
+        @type ssh_hostkey_dir: str
+        @param ssh_hostkey_dir: directory which contains ssh host keys for
+                                this server
         """
 
         # unfortunately, these don't work unless we're running as root
@@ -178,13 +181,22 @@ def makeProtocol():
             p = insults.ServerProtocol(manhole.ColoredManhole, namespace)
             return p
 
-        self.using_ssh = using_ssh
-        if using_ssh:
+        self.ssh_hostkey_dir = ssh_hostkey_dir
+        if self.ssh_hostkey_dir:
+            self.using_ssh = True
+            if not self.ssh_hostkey_dir:
+                raise ValueError("Most specify a value for ssh_hostkey_dir")
             r = manhole_ssh.TerminalRealm()
             r.chainedProtocolFactory = makeProtocol
             p = portal.Portal(r, [self.checker])
             f = manhole_ssh.ConchFactory(p)
+            openSSHFactory = OpenSSHFactory()
+            openSSHFactory.dataRoot = self.ssh_hostkey_dir
+            openSSHFactory.dataModuliRoot = self.ssh_hostkey_dir
+            f.publicKeys = openSSHFactory.getPublicKeys()
+            f.privateKeys = openSSHFactory.getPrivateKeys()
         else:
+            self.using_ssh = False
             r = _TelnetRealm(makeNamespace)
             p = portal.Portal(r, [self.checker])
             f = protocol.ServerFactory()
@@ -226,9 +238,9 @@ def __init__(self, port, username, password):
         self.password = password
 
         c = checkers.InMemoryUsernamePasswordDatabaseDontUse()
-        c.addUser(username, password)
+        c.addUser(unicode2bytes(username), unicode2bytes(password))
 
-        _BaseManhole.__init__(self, port, c, using_ssh=False)
+        _BaseManhole.__init__(self, port, c)
 
 
 class PasswordManhole(_BaseManhole, ComparableMixin):
@@ -237,9 +249,9 @@ class PasswordManhole(_BaseManhole, ComparableMixin):
     username and password to authorize access.
     """
 
-    compare_attrs = ("port", "username", "password")
+    compare_attrs = ("port", "username", "password", "ssh_hostkey_dir")
 
-    def __init__(self, port, username, password):
+    def __init__(self, port, username, password, ssh_hostkey_dir):
         """
         @type port: string or int
         @param port: what port should the Manhole listen on? This is a
@@ -250,17 +262,21 @@ def __init__(self, port, username, password):
         @param username:
         @param password: username= and password= form a pair of strings to
                          use when authenticating the remote user.
+        @type ssh_hostkey_dir: str
+        @param ssh_hostkey_dir: directory which contains ssh host keys for
+                                this server
         """
 
         if not manhole_ssh:
-            config.error("pycrypto required for ssh mahole.")
+            config.error("cryptography required for ssh mahole.")
         self.username = username
         self.password = password
+        self.ssh_hostkey_dir = ssh_hostkey_dir
 
         c = checkers.InMemoryUsernamePasswordDatabaseDontUse()
-        c.addUser(username, password)
+        c.addUser(unicode2bytes(username), unicode2bytes(password))
 
-        _BaseManhole.__init__(self, port, c)
+        _BaseManhole.__init__(self, port, c, ssh_hostkey_dir)
 
 
 class AuthorizedKeysManhole(_BaseManhole, ComparableMixin):
@@ -270,9 +286,9 @@ class AuthorizedKeysManhole(_BaseManhole, ComparableMixin):
     keys in our authorized_keys file. It is created with the name of a file
     that contains the public keys that we will accept."""
 
-    compare_attrs = ("port", "keyfile")
+    compare_attrs = ("port", "keyfile", "ssh_hostkey_dir")
 
-    def __init__(self, port, keyfile):
+    def __init__(self, port, keyfile, ssh_hostkey_dir):
         """
         @type port: string or int
         @param port: what port should the Manhole listen on? This is a
@@ -284,16 +300,19 @@ def __init__(self, port, keyfile):
                         basedir) that contains SSH public keys of authorized
                         users, one per line. This is the exact same format
                         as used by sshd in ~/.ssh/authorized_keys .
+        @type ssh_hostkey_dir: str
+        @param ssh_hostkey_dir: directory which contains ssh host keys for
+                                this server
         """
 
         if not manhole_ssh:
-            config.error("pycrypto required for ssh mahole.")
+            config.error("cryptography required for ssh mahole.")
 
         # TODO: expanduser this, and make it relative to the buildmaster's
         # basedir
         self.keyfile = keyfile
         c = AuthorizedKeysChecker(keyfile)
-        _BaseManhole.__init__(self, port, c)
+        _BaseManhole.__init__(self, port, c, ssh_hostkey_dir)
 
 
 class ArbitraryCheckerManhole(_BaseManhole, ComparableMixin):
@@ -316,7 +335,7 @@ def __init__(self, port, checker):
         """
 
         if not manhole_ssh:
-            config.error("pycrypto required for ssh mahole.")
+            config.error("cryptography required for ssh mahole.")
 
         _BaseManhole.__init__(self, port, checker)
 
@@ -330,19 +349,18 @@ def show(x):
     maxlen = max([0] + [len(n) for n in names])
     for k in names:
         v = getattr(x, k)
-        t = type(v)
-        if t == types.MethodType:
+        if isinstance(v, types.MethodType):
             continue
         if k[:2] == '__' and k[-2:] == '__':
             continue
-        if t is str:
+        if isinstance(v, string_types):
             if len(v) > 80 - maxlen - 5:
                 v = repr(v[:80 - maxlen - 5]) + "..."
-        elif t in (int, type(None)):
+        elif isinstance(v, (int, type(None))):
             v = str(v)
-        elif v in (list, tuple, dict):
+        elif isinstance(v, (list, tuple, dict)):
             v = "%s (%d elements)" % (v, len(v))
         else:
-            v = str(t)
+            v = str(type(v))
         print("%*s : %s" % (maxlen, k, v))
     return x

master/buildbot/newsfragments/manhole-py3.bugfix

@@ -0,0 +1,3 @@
+Fix Manhole support to work with Python 3 and Twisted 16.0.0+ (:issue:`3160`).
+:py:class:`~buildbot.manhole.AuthorizedKeysManhole` and :py:class:`~buildbot.manhole.PasswordManhole`
+now require a directory containing SSH host keys to be specified.

master/docs/examples/hello.cfg

@@ -63,7 +63,7 @@ c['titleURL'] = "http://www.hello.example.com/"
 c['buildbotURL'] = "http://localhost:8080"
 
 c['slavePortnum'] = 8007
-c['manhole'] = util.PasswordManhole(9900, "username", "password")
+c['manhole'] = util.PasswordManhole(9900, "username", "password", ssh_hostkey_dir="/data/ssh_host_keys/")
 
 c['www'] = {
     'port': 8080

master/docs/manual/cfg-global.rst

@@ -458,16 +458,18 @@ Two of them use a username+password combination to grant access, one of them use
 
 .. note::
 
-   Using any Manhole requires that ``pycrypto`` and ``pyasn1`` be installed.
+   Using any Manhole requires that ``cryptography`` and ``pyasn1`` be installed.
    These are not part of the normal Buildbot dependencies.
 
 `manhole.AuthorizedKeysManhole`
     You construct this with the name of a file that contains one SSH public key per line, just like :file:`~/.ssh/authorized_keys`.
     If you provide a non-absolute filename, it will be interpreted relative to the buildmaster's base directory.
+    You must also specify a directory which contains an SSH host key for the Manhole server.
 
 `manhole.PasswordManhole`
     This one accepts SSH connections but asks for a username and password when authenticating.
     It accepts only one such pair.
+    You must also specify a directory which contains an SSH host key for the Manhole server.
 
 `manhole.TelnetManhole`
     This accepts regular unencrypted telnet connections, and asks for a username/password pair before providing access.
@@ -477,8 +479,8 @@ Two of them use a username+password combination to grant access, one of them use
 
     # some examples:
     from buildbot.plugins import util
-    c['manhole'] = util.AuthorizedKeysManhole(1234, "authorized_keys")
-    c['manhole'] = util.PasswordManhole(1234, "alice", "mysecretpassword")
+    c['manhole'] = util.AuthorizedKeysManhole(1234, "authorized_keys", ssh_hostkey_dir="/data/ssh_host_keys/")
+    c['manhole'] = util.PasswordManhole(1234, "alice", "mysecretpassword", ssh_hostkey_dir="/data/ssh_host_keys/")
     c['manhole'] = util.TelnetManhole(1234, "bob", "snoop_my_password_please")
 
 The :class:`Manhole` instance can be configured to listen on a specific port.
@@ -487,7 +489,7 @@ You may wish to have this listening port bind to the loopback interface (sometim
 ::
 
     from buildbot.plugins import util
-    c['manhole'] = util.PasswordManhole("tcp:9999:interface=127.0.0.1","admin","passwd")
+    c['manhole'] = util.PasswordManhole("tcp:9999:interface=127.0.0.1","admin","passwd", ssh_hostkey_dir="/data/ssh_host_keys/")
 
 To have the :class:`Manhole` listen on all interfaces, use ``"tcp:9999"`` or simply 9999.
 This port specification uses ``twisted.application.strports``, so you can make it listen on SSL or even UNIX-domain sockets if you want.

master/docs/tutorial/tour.rst

@@ -249,7 +249,7 @@ Debugging with Manhole
 ----------------------
 
 You can do some debugging by using manhole, an interactive Python shell.
-It exposes full access to the buildmaster's account (including the ability to modify and delete files), so it should not be enabled with a weak or easily guessable password. 
+It exposes full access to the buildmaster's account (including the ability to modify and delete files), so it should not be enabled with a weak or easily guessable password.
 
 To use this you will need to install an additional package or two to your virtualenv:
 
@@ -260,6 +260,13 @@ To use this you will need to install an additional package or two to your virtua
   pip install -U pip
   pip install cryptography pyasn1
 
+You will also need to generate an SSH host key for the Manhole server.
+
+.. code-block:: bash
+
+  mkdir -p /data/ssh_host_keys
+  ckeygen -t rsa -f /data/ssh_host_keys/ssh_host_rsa_key
+
 In your master.cfg find::
 
   c = BuildmasterConfig = {}
@@ -268,7 +275,7 @@ Insert the following to enable debugging mode with manhole::
 
   ####### DEBUGGING
   from buildbot import manhole
-  c['manhole'] = manhole.PasswordManhole("tcp:1234:interface=127.0.0.1","admin","passwd")
+  c['manhole'] = manhole.PasswordManhole("tcp:1234:interface=127.0.0.1","admin","passwd", ssh_hostkey_dir="/data/ssh_host_keys/")
 
 After restarting the master, you can ssh into the master and get an interactive Python shell: