r46985 - Merge manhole-hardcodedkey-8229: Remove a hardcoded SSH key from manhole

hawkowl-TA+aISz0psMTMxyoc4vAAJOcrHinNvQL0E9HWUfgJXw@public.gmane.org Mon, 14 Mar 2016 01:29:17 -0600 (MDT)
Newsgroups gmane.comp.python.twisted.commits
Message-ID <[email protected]>
Author: hawkowl
Date: Mon Mar 14 01:29:00 2016
New Revision: 46985

Added:
   trunk/twisted/conch/test/test_manhole_tap.py
   trunk/twisted/conch/topfiles/8229.feature
   trunk/twisted/python/_appdirs.py
   trunk/twisted/python/test/test_appdirs.py
Modified:
   trunk/tox.ini
   trunk/twisted/conch/manhole_ssh.py
   trunk/twisted/conch/manhole_tap.py
   trunk/twisted/conch/ssh/keys.py
   trunk/twisted/conch/test/test_keys.py
   trunk/twisted/conch/test/test_recvline.py
   trunk/twisted/python/dist.py
   trunk/twisted/python/dist3.py
   trunk/twisted/python/test/test_dist.py

Log:
Merge manhole-hardcodedkey-8229: Remove a hardcoded SSH key from manhole

Author: hawkowl
Reviewer: glyph
Fixes: #8229

Modified: trunk/tox.ini
==============================================================================
--- trunk/tox.ini	(original)
+++ trunk/tox.ini	Mon Mar 14 01:29:00 2016
@@ -22,11 +22,13 @@
      {tests,coverage}: pyserial
      {tests,coverage}: python-subunit
      {tests,coverage}: pycrypto
-     py27-{tests,coverage}-posix: pysqlite
-     windows: pypiwin32
+     {tests,coverage}: appdirs
 
+     py27-{tests,coverage}-posix: pysqlite
      py27-{tests,coverage}: soappy
 
+     windows: pypiwin32
+
      coverage: coverage
 
      ; Code quality checkers

Modified: trunk/twisted/conch/manhole_ssh.py
==============================================================================
--- trunk/twisted/conch/manhole_ssh.py	(original)
+++ trunk/twisted/conch/manhole_ssh.py	Mon Mar 14 01:29:00 2016
@@ -11,11 +11,12 @@
 from zope.interface import implementer
 
 from twisted.conch import avatar, interfaces as iconch, error as econch
-from twisted.conch.ssh import factory, keys, session
+from twisted.conch.ssh import factory, session
 from twisted.python import components
 
 from twisted.conch.insults import insults
 
+
 class _Glue:
     """A feeble class for making one attribute look like another.
 
@@ -28,6 +29,8 @@
     def __getattr__(self, name):
         raise AttributeError(self.name, "has no attribute", name)
 
+
+
 class TerminalSessionTransport:
     def __init__(self, proto, chainedProtocol, avatar, width, height):
         self.proto = proto
@@ -79,12 +82,16 @@
     def closed(self):
         pass
 
+
+
 class TerminalUser(avatar.ConchUser, components.Adapter):
     def __init__(self, original, avatarId):
         components.Adapter.__init__(self, original)
         avatar.ConchUser.__init__(self)
         self.channelLookup['session'] = session.SSHSession
 
+
+
 class TerminalRealm:
     userFactory = TerminalUser
     sessionFactory = TerminalSession
@@ -117,30 +124,11 @@
                         lambda: None)
         raise NotImplementedError()
 
-class ConchFactory(factory.SSHFactory):
-    publicKey = 'ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAGEArzJx8OYOnJmzf4tfBEvLi8DVPrJ3/c9k2I/Az64fxjHf9imyRJbixtQhlH9lfNjUIx+4LmrJH5QNRsFporcHDKOTwTTYLh5KmRpslkYHRivcJSkbh/C+BR3utDS555mV'
 
-    publicKeys = {
-        'ssh-rsa' : keys.Key.fromString(publicKey)
-    }
-    del publicKey
-
-    privateKey = """-----BEGIN RSA PRIVATE KEY-----
-MIIByAIBAAJhAK8ycfDmDpyZs3+LXwRLy4vA1T6yd/3PZNiPwM+uH8Yx3/YpskSW
-4sbUIZR/ZXzY1CMfuC5qyR+UDUbBaaK3Bwyjk8E02C4eSpkabJZGB0Yr3CUpG4fw
-vgUd7rQ0ueeZlQIBIwJgbh+1VZfr7WftK5lu7MHtqE1S1vPWZQYE3+VUn8yJADyb
-Z4fsZaCrzW9lkIqXkE3GIY+ojdhZhkO1gbG0118sIgphwSWKRxK0mvh6ERxKqIt1
-xJEJO74EykXZV4oNJ8sjAjEA3J9r2ZghVhGN6V8DnQrTk24Td0E8hU8AcP0FVP+8
-PQm/g/aXf2QQkQT+omdHVEJrAjEAy0pL0EBH6EVS98evDCBtQw22OZT52qXlAwZ2
-gyTriKFVoqjeEjt3SZKKqXHSApP/AjBLpF99zcJJZRq2abgYlf9lv1chkrWqDHUu
-DZttmYJeEfiFBBavVYIF1dOlZT0G8jMCMBc7sOSZodFnAiryP+Qg9otSBjJ3bQML
-pSTqy7c3a2AScC/YyOwkDaICHnnD3XyjMwIxALRzl0tQEKMXs6hH8ToUdlLROCrP
-EhQ0wahUTCk1gKA4uPD6TMTChavbh4K63OvbKg==
------END RSA PRIVATE KEY-----"""
-    privateKeys = {
-        'ssh-rsa' : keys.Key.fromString(privateKey)
-    }
-    del privateKey
+
+class ConchFactory(factory.SSHFactory):
+    publicKeys = {}
+    privateKeys = {}
 
     def __init__(self, portal):
         self.portal = portal

Modified: trunk/twisted/conch/manhole_tap.py
==============================================================================
--- trunk/twisted/conch/manhole_tap.py	(original)
+++ trunk/twisted/conch/manhole_tap.py	Mon Mar 14 01:29:00 2016
@@ -12,10 +12,13 @@
 from twisted.internet import protocol
 from twisted.application import service, strports
 from twisted.cred import portal, checkers
-from twisted.python import usage
+from twisted.python import usage, filepath
 
-from twisted.conch.insults import insults
 from twisted.conch import manhole, manhole_ssh, telnet
+from twisted.conch.insults import insults
+from twisted.conch.ssh import keys
+
+
 
 class makeTelnetProtocol:
     def __init__(self, portal):
@@ -26,10 +29,12 @@
         args = (self.portal,)
         return telnet.TelnetTransport(auth, *args)
 
+
+
 class chainedProtocolFactory:
     def __init__(self, namespace):
         self.namespace = namespace
-    
+
     def __call__(self):
         return insults.ServerProtocol(manhole.ColoredManhole, self.namespace)
 
@@ -45,26 +50,45 @@
     def requestAvatar(self, avatarId, *interfaces):
         if telnet.ITelnetProtocol in interfaces:
             return (telnet.ITelnetProtocol,
-                    self.protocolFactory(*self.protocolArgs, **self.protocolKwArgs),
+                    self.protocolFactory(*self.protocolArgs,
+                                         **self.protocolKwArgs),
                     lambda: None)
         raise NotImplementedError()
 
+
+
 class Options(usage.Options):
     optParameters = [
-        ["telnetPort", "t", None, "strports description of the address on which to listen for telnet connections"],
-        ["sshPort", "s", None, "strports description of the address on which to listen for ssh connections"],
-        ["passwd", "p", "/etc/passwd", "name of a passwd(5)-format username/password file"]]
+        ["telnetPort", "t", None,
+         ("strports description of the address on which to listen for telnet "
+          "connections")],
+        ["sshPort", "s", None,
+         ("strports description of the address on which to listen for ssh "
+          "connections")],
+        ["passwd", "p", "/etc/passwd",
+         "name of a passwd(5)-format username/password file"],
+        ["sshKeyDir", None, "<USER DATA DIR>",
+         "Directory where the autogenerated SSH key is kept."],
+        ["sshKeyName", None, "server.key",
+         "Filename of the autogenerated SSH key."],
+        ["sshKeySize", None, 4096,
+         "Size of the automatically generated SSH key."],
+    ]
 
     def __init__(self):
         usage.Options.__init__(self)
         self['namespace'] = None
-    
+
     def postOptions(self):
         if self['telnetPort'] is None and self['sshPort'] is None:
-            raise usage.UsageError("At least one of --telnetPort and --sshPort must be specified")
+            raise usage.UsageError(
+                "At least one of --telnetPort and --sshPort must be specified")
+
+
 
 def makeService(options):
-    """Create a manhole server service.
+    """
+    Create a manhole server service.
 
     @type options: C{dict}
     @param options: A mapping describing the configuration of
@@ -84,10 +108,15 @@
 
         "passwd": Name of a passwd(5)-format username/password file.
 
+        "sshKeyDir": The folder that the SSH server key will be kept in.
+
+        "sshKeyName": The filename of the key.
+
+        "sshKeySize": The size of the key, in bits. Default is 4096.
+
     @rtype: L{twisted.application.service.IService}
     @return: A manhole service.
     """
-
     svc = service.MultiService()
 
     namespace = options['namespace']
@@ -116,8 +145,21 @@
 
         sshPortal = portal.Portal(sshRealm, [checker])
         sshFactory = manhole_ssh.ConchFactory(sshPortal)
-        sshService = strports.service(options['sshPort'],
-                                      sshFactory)
+
+        if options['sshKeyDir'] != "<USER DATA DIR>":
+            keyDir = options['sshKeyDir']
+        else:
+            from twisted.python._appdirs import getDataDirectory
+            keyDir = getDataDirectory()
+
+        keyLocation = filepath.FilePath(keyDir).child(options['sshKeyName'])
+
+        sshKey = keys._getPersistentRSAKey(keyLocation,
+                                           int(options['sshKeySize']))
+        sshFactory.publicKeys["ssh-rsa"] = sshKey
+        sshFactory.privateKeys["ssh-rsa"] = sshKey
+
+        sshService = strports.service(options['sshPort'], sshFactory)
         sshService.setServiceParent(svc)
 
     return svc

Modified: trunk/twisted/conch/ssh/keys.py
==============================================================================
--- trunk/twisted/conch/ssh/keys.py	(original)
+++ trunk/twisted/conch/ssh/keys.py	Mon Mar 14 01:29:00 2016
@@ -11,11 +11,12 @@
 import base64
 import itertools
 import warnings
+
 from hashlib import md5
 
 from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.backends import default_backend
-from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives import hashes, serialization
 from cryptography.hazmat.primitives.asymmetric import dsa, rsa, padding
 try:
     from cryptography.hazmat.primitives.asymmetric.utils import (
@@ -1229,6 +1230,55 @@
         raise BadKeyError("invalid key object", obj)
 
 
+
+def _getPersistentRSAKey(location, keySize=4096):
+    """
+    This function returns a persistent L{Key}.
+
+    The key is loaded from a PEM file in C{location}. If it does not exist, a
+    key with the key size of C{keySize} is generated and saved.
+
+    @param location: Where the key is stored.
+    @type location: L{twisted.python.filepath.FilePath)
+
+    @param keySize: The size of the key, if it needs to be generated.
+    @type keySize: L{int}
+
+    @returns: A persistent key.
+    @rtype: L{Key}
+    """
+    location.parent().makedirs(ignoreExistingDirectory=True)
+
+    # If it doesn't exist, we want to generate a new key and save it
+    if not location.exists():
+        privateKey = rsa.generate_private_key(
+            public_exponent=65537,
+            key_size=keySize,
+            backend=default_backend()
+        )
+
+        pem = privateKey.private_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PrivateFormat.TraditionalOpenSSL,
+            encryption_algorithm=serialization.NoEncryption()
+        )
+
+        location.setContent(pem)
+
+    # By this point (save any hilarious race conditions) we should have a
+    # working PEM file. Load it!
+    # (Future archaelogical readers: I chose not to short circuit above,
+    # because then there's two exit paths to this code!)
+    with location.open("rb") as keyFile:
+        privateKey = serialization.load_pem_private_key(
+            keyFile.read(),
+            password=None,
+            backend=default_backend()
+        )
+        return Key(privateKey)
+
+
+
 if _PY3:
     # The objectType function is deprecated and not being ported to Python 3.
     del objectType

Modified: trunk/twisted/conch/test/test_keys.py
==============================================================================
--- trunk/twisted/conch/test/test_keys.py	(original)
+++ trunk/twisted/conch/test/test_keys.py	Mon Mar 14 01:29:00 2016
@@ -29,12 +29,15 @@
 if cryptography and pyasn1:
     from twisted.conch.ssh import keys, common, sexpy
 
-import os, base64
+import base64
+import os
+
 from twisted.conch.test import keydata
 from twisted.python import randbytes
 from twisted.trial import unittest
 from twisted.python.compat import long, _PY3
 from twisted.python.versions import Version
+from twisted.python.filepath import FilePath
 
 
 
@@ -1159,3 +1162,46 @@
             'e': keydata.RSAData['e'],
             },
             key.data())
+
+
+
+class PersistentRSAKeyTests(unittest.TestCase):
+    """
+    Tests for L{keys._getPersistentRSAKey}.
+    """
+
+    if cryptography is None:
+        skip = skipCryptography
+
+
+    def test_providedArguments(self):
+        """
+        L{keys._getPersistentRSAKey} will put the key in
+        C{directory}/C{filename}, with the key length of C{keySize}.
+        """
+        tempDir = FilePath(self.mktemp())
+        keyFile = tempDir.child("mykey.pem")
+
+        key = keys._getPersistentRSAKey(keyFile, keySize=512)
+        self.assertEqual(key.size(), 512)
+        self.assertTrue(keyFile.exists())
+
+
+    def test_noRegeneration(self):
+        """
+        L{keys._getPersistentRSAKey} will not regenerate the key if the key
+        already exists.
+        """
+        tempDir = FilePath(self.mktemp())
+        keyFile = tempDir.child("mykey.pem")
+
+        key = keys._getPersistentRSAKey(keyFile, keySize=512)
+        self.assertEqual(key.size(), 512)
+        self.assertTrue(keyFile.exists())
+        keyContent = keyFile.getContent()
+
+        # Set the key size to 1024 bits. Since it exists already, it will find
+        # the 512 bit key, and not generate a 1024 bit key.
+        key = keys._getPersistentRSAKey(keyFile, keySize=1024)
+        self.assertEqual(key.size(), 512)
+        self.assertEqual(keyFile.getContent(), keyContent)

Modified: trunk/twisted/conch/test/test_recvline.py
==============================================================================
--- trunk/twisted/conch/test/test_recvline.py	(original)
+++ trunk/twisted/conch/test/test_recvline.py	Mon Mar 14 01:29:00 2016
@@ -7,17 +7,19 @@
 functionality.
 """
 
-import sys, os
+import os
+import sys
 
 from twisted.conch.insults import insults
 from twisted.conch import recvline
 
-from twisted.python import reflect, components
+from twisted.python import reflect, components, filepath
 from twisted.internet import defer, error
 from twisted.trial import unittest
 from twisted.cred import portal
 from twisted.test.proto_helpers import StringTransport
 
+
 class ArrowsTests(unittest.TestCase):
     def setUp(self):
         self.underlyingTransport = StringTransport()
@@ -298,7 +300,8 @@
 from twisted.cred import checkers
 
 try:
-    from twisted.conch.ssh import userauth, transport, channel, connection, session
+    from twisted.conch.ssh import (userauth, transport, channel, connection,
+                                   session, keys)
     from twisted.conch.manhole_ssh import TerminalUser, TerminalSession, TerminalRealm, TerminalSessionTransport, ConchFactory
 except ImportError:
     ssh = False
@@ -467,6 +470,12 @@
             rlm,
             [checkers.InMemoryUsernamePasswordDatabaseDontUse(**{u: p})])
         sshFactory = ConchFactory(ptl)
+
+        sshKey = keys._getPersistentRSAKey(filepath.FilePath(self.mktemp()),
+                                           keySize=512)
+        sshFactory.publicKeys["ssh-rsa"] = sshKey
+        sshFactory.privateKeys["ssh-rsa"] = sshKey
+
         sshFactory.serverProtocol = self.serverProtocol
         sshFactory.startFactory()
 

Modified: trunk/twisted/python/dist.py
==============================================================================
--- trunk/twisted/python/dist.py	(original)
+++ trunk/twisted/python/dist.py	Mon Mar 14 01:29:00 2016
@@ -75,6 +75,7 @@
     conch=['gmpy',
            'pyasn1',
            'cryptography >= 0.9.1',
+           'appdirs >= 1.4.0',
            ],
     soap=['soappy'],
     serial=['pyserial'],

Modified: trunk/twisted/python/dist3.py
==============================================================================
--- trunk/twisted/python/dist3.py	(original)
+++ trunk/twisted/python/dist3.py	Mon Mar 14 01:29:00 2016
@@ -160,6 +160,7 @@
     "twisted.protocols.test.__init__",
     "twisted.protocols.tls",
     "twisted.python.__init__",
+    "twisted.python._appdirs",
     "twisted.python._tzhelper",
     "twisted.python._url",
     "twisted.python.compat",
@@ -320,6 +321,7 @@
     "twisted.positioning.test.test_sentence",
     "twisted.protocols.test.test_basic",
     "twisted.protocols.test.test_tls",
+    "twisted.python.test.test_appdirs",
     "twisted.python.test.test_components",
     "twisted.python.test.test_constants",
     "twisted.python.test.test_deprecate",

Modified: trunk/twisted/python/test/test_dist.py
==============================================================================
--- trunk/twisted/python/test/test_dist.py	(original)
+++ trunk/twisted/python/test/test_dist.py	Mon Mar 14 01:29:00 2016
@@ -131,6 +131,7 @@
         self.assertIn('gmpy', deps)
         self.assertIn('pyasn1', deps)
         self.assertIn('cryptography >= 0.9.1', deps)
+        self.assertIn('appdirs >= 1.4.0', deps)
 
 
     def test_extrasRequiresSoapDeps(self):
@@ -170,6 +171,7 @@
         self.assertIn('cryptography >= 0.9.1', deps)
         self.assertIn('soappy', deps)
         self.assertIn('pyserial', deps)
+        self.assertIn('appdirs >= 1.4.0', deps)
 
 
     def test_extrasRequiresOsxPlatformDeps(self):