← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~wrigri/cloud-init:ssh-hostkey-publish into cloud-init:master

 

Rick Wright has proposed merging ~wrigri/cloud-init:ssh-hostkey-publish into cloud-init:master.

Commit message:
Add support for publishing host keys to GCE guest attributes.

This adds an empty publish_host_keys() method to the default datasource that is called by cc_ssh.py. This feature can be controlled by the 'ssh_publish_hostkeys' config option. It is enabled by default but can be disabled by setting 'enabled' to false. Also, a blacklist of key types is supported.

In addition, this change implements ssh_publish_hostkeys() for the GCE datasource, attempting to write the hostkeys to the instance's guest attributes. Using these hostkeys for ssh connections is currently supported by the alpha version of Google's 'gcloud' command-line tool.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~wrigri/cloud-init/+git/cloud-init/+merge/370348

On Google Compute Engine, this feature will be enabled by setting the 'enable-guest-attributes' metadata key to 'true' for the project/instance that you would like to use this feature for. When connecting to the instance for the first time using 'gcloud compute ssh' the hostkeys will be read from the guest attributes for the instance and written to the user's local known_hosts file for Google Compute Engine instances.
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~wrigri/cloud-init:ssh-hostkey-publish into cloud-init:master.
diff --git a/cloudinit/config/cc_ssh.py b/cloudinit/config/cc_ssh.py
index f8f7cb3..6d8e3b1 100755
--- a/cloudinit/config/cc_ssh.py
+++ b/cloudinit/config/cc_ssh.py
@@ -91,6 +91,9 @@ public keys.
     ssh_authorized_keys:
         - ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAGEA3FSyQwBI6Z+nCSjUU ...
         - ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA3I7VUf2l5gSn5uavROsc5HRDpZ ...
+    ssh_publish_hostkeys:
+        enabled: <true/false>
+        blacklist: <list of key types>
 """
 
 import glob
@@ -176,6 +179,21 @@ def handle(_name, cfg, cloud, log, _args):
                         util.logexc(log, "Failed generating key type %s to "
                                     "file %s", keytype, keyfile)
 
+    host_key_blacklist = ["ssh-dss"]
+    publish_hostkeys = True
+    if "ssh_publish_hostkeys" in cfg:
+        host_key_blacklist = util.get_cfg_option_list(
+            cfg["ssh_publish_hostkeys"], "blacklist", ["ssh-dss"])
+        publish_hostkeys = util.get_cfg_option_bool(
+            cfg["ssh_publish_hostkeys"], "enabled", True)
+
+    if publish_hostkeys:
+        try:
+            cloud.datasource.publish_host_keys(blacklist=host_key_blacklist)
+        except Exception as e:
+            util.logexc(log, "Publishing host keys failed!")
+            util.logexc(log, e)
+
     try:
         (users, _groups) = ug_util.normalize_users_groups(cfg, cloud.distro)
         (user, _user_config) = ug_util.extract_default(users)
diff --git a/cloudinit/config/tests/test_ssh.py b/cloudinit/config/tests/test_ssh.py
index c8a4271..856c117 100644
--- a/cloudinit/config/tests/test_ssh.py
+++ b/cloudinit/config/tests/test_ssh.py
@@ -149,3 +149,50 @@ class TestHandleSsh(CiTestCase):
         self.assertEqual([mock.call(set(keys), user),
                           mock.call(set(keys), "root", options="")],
                          m_setup_keys.call_args_list)
+
+    @mock.patch(MODPATH + "glob.glob")
+    @mock.patch(MODPATH + "ug_util.normalize_users_groups")
+    @mock.patch(MODPATH + "os.path.exists")
+    def test_handle_publish_hostkeys(self, m_path_exists, m_nug,
+                                     m_glob, m_setup_keys):
+        """Test handle with various configs for ssh_publish_hostkeys."""
+        keys = ["key1"]
+        user = "clouduser"
+        m_glob.return_value = []  # Return no matching keys to prevent removal
+        # Mock os.path.exits to True to short-circuit the key writing logic
+        m_path_exists.return_value = True
+        m_nug.return_value = ({user: {"default": user}}, {})
+        cloud = self.tmp_cloud(
+            distro='ubuntu', metadata={'public-keys': keys})
+        cloud.datasource.publish_host_keys = mock.Mock()
+
+        cfg = {}
+        cc_ssh.handle("name", cfg, cloud, None, None)
+        cloud.datasource.publish_host_keys.assert_called_once_with(
+            blacklist=['ssh-dss'])
+        cloud.datasource.publish_host_keys.reset_mock()
+
+        cfg = {'ssh_publish_hostkeys': {'enabled': True}}
+        cc_ssh.handle("name", cfg, cloud, None, None)
+        cloud.datasource.publish_host_keys.assert_called_once_with(
+            blacklist=['ssh-dss'])
+        cloud.datasource.publish_host_keys.reset_mock()
+
+        cfg = {'ssh_publish_hostkeys': {'enabled': False}}
+        cc_ssh.handle("name", cfg, cloud, None, None)
+        cloud.datasource.publish_host_keys.assert_not_called()
+        cloud.datasource.publish_host_keys.reset_mock()
+
+        cfg = {'ssh_publish_hostkeys': {'enabled': True,
+                                        'blacklist': ['ssh-dss', 'ssh-rsa']}}
+        cc_ssh.handle("name", cfg, cloud, None, None)
+        cloud.datasource.publish_host_keys.assert_called_once_with(
+            blacklist=['ssh-dss', 'ssh-rsa'])
+        cloud.datasource.publish_host_keys.reset_mock()
+
+        cfg = {'ssh_publish_hostkeys': {'enabled': True,
+                                        'blacklist': []}}
+        cc_ssh.handle("name", cfg, cloud, None, None)
+        cloud.datasource.publish_host_keys.assert_called_once_with(
+            blacklist=[])
+        cloud.datasource.publish_host_keys.reset_mock()
diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
index d816262..a2e3bf4 100644
--- a/cloudinit/sources/DataSourceGCE.py
+++ b/cloudinit/sources/DataSourceGCE.py
@@ -3,7 +3,9 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 import datetime
+import glob
 import json
+import os.path
 
 from base64 import b64decode
 
@@ -18,10 +20,13 @@ LOG = logging.getLogger(__name__)
 MD_V1_URL = 'http://metadata.google.internal/computeMetadata/v1/'
 BUILTIN_DS_CONFIG = {'metadata_url': MD_V1_URL}
 REQUIRED_FIELDS = ('instance-id', 'availability-zone', 'local-hostname')
+GUEST_ATTRIBUTES_URL = ('http://metadata.google.internal/computeMetadata/'
+                        'v1beta1/instance/guest-attributes')
+HOSTKEY_NAMESPACE = 'hostkeys'
+HEADERS = {'Metadata-Flavor': 'Google'}
 
 
 class GoogleMetadataFetcher(object):
-    headers = {'Metadata-Flavor': 'Google'}
 
     def __init__(self, metadata_address):
         self.metadata_address = metadata_address
@@ -32,7 +37,7 @@ class GoogleMetadataFetcher(object):
             url = self.metadata_address + path
             if is_recursive:
                 url += '/?recursive=True'
-            resp = url_helper.readurl(url=url, headers=self.headers)
+            resp = url_helper.readurl(url=url, headers=HEADERS)
         except url_helper.UrlError as exc:
             msg = "url %s raised exception %s"
             LOG.debug(msg, path, exc)
@@ -90,6 +95,15 @@ class DataSourceGCE(sources.DataSource):
         public_keys_data = self.metadata['public-keys-data']
         return _parse_public_keys(public_keys_data, self.default_user)
 
+    def publish_host_keys(self, blacklist=None):
+        if blacklist is None:
+            blacklist = []
+        host_keys = _get_public_host_keys()
+        for key in host_keys:
+            if key[0] not in blacklist:
+                _write_host_key_to_guest_attributes(*key)
+        return
+
     def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False):
         # GCE has long FDQN's and has asked for short hostnames.
         return self.metadata['local-hostname'].split('.')[0]
@@ -103,6 +117,30 @@ class DataSourceGCE(sources.DataSource):
         return self.availability_zone.rsplit('-', 1)[0]
 
 
+def _get_public_host_keys(key_dir='/etc/ssh'):
+    key_list = []
+    key_path = os.path.join(key_dir, 'ssh_host_*key.pub')
+    file_list = glob.glob(key_path)
+    for file_name in file_list:
+        with open(file_name) as f:
+            key_data = f.readlines()[0].split()
+        if key_data and len(key_data) > 1:
+            key_list.append(key_data[:2])
+    return key_list
+
+
+def _write_host_key_to_guest_attributes(key_type, key_value):
+    url = '%s/%s/%s' % (GUEST_ATTRIBUTES_URL, HOSTKEY_NAMESPACE, key_type)
+    key_value = key_value.encode('utf-8')
+    resp = url_helper.readurl(url=url, data=key_value, headers=HEADERS,
+                              request_method='PUT', check_status=False)
+    if resp.ok():
+        util.multi_log('Wrote %s host key to guest attributes.\n' % key_type)
+    else:
+        util.multi_log('Unable to write %s host key to guest attributes.\n'
+                       % key_type)
+
+
 def _has_expired(public_key):
     # Check whether an SSH key is expired. Public key input is a single SSH
     # public key in the GCE specific key format documented here:
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index e6966b3..d6dde57 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -474,6 +474,9 @@ class DataSource(object):
     def get_public_ssh_keys(self):
         return normalize_pubkey_data(self.metadata.get('public-keys'))
 
+    def publish_host_keys(self, blacklist=None):
+        return
+
     def _remap_device(self, short_name):
         # LP: #611137
         # the metadata service may believe that devices are named 'sda'
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
index 0af0d9e..44ee61d 100644
--- a/cloudinit/url_helper.py
+++ b/cloudinit/url_helper.py
@@ -199,18 +199,19 @@ def _get_ssl_args(url, ssl_details):
 def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
             headers=None, headers_cb=None, ssl_details=None,
             check_status=True, allow_redirects=True, exception_cb=None,
-            session=None, infinite=False, log_req_resp=True):
+            session=None, infinite=False, log_req_resp=True,
+            request_method=None):
     url = _cleanurl(url)
     req_args = {
         'url': url,
     }
     req_args.update(_get_ssl_args(url, ssl_details))
     req_args['allow_redirects'] = allow_redirects
-    req_args['method'] = 'GET'
+    if not request_method:
+        request_method = 'POST' if data else 'GET'
+    req_args['method'] = request_method
     if timeout is not None:
         req_args['timeout'] = max(float(timeout), 0)
-    if data:
-        req_args['method'] = 'POST'
     # It doesn't seem like config
     # was added in older library versions (or newer ones either), thus we
     # need to manually do the retries if it wasn't...
diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py
index 41176c6..3c2acd2 100644
--- a/tests/unittests/test_datasource/test_gce.py
+++ b/tests/unittests/test_datasource/test_gce.py
@@ -8,6 +8,7 @@ import datetime
 import httpretty
 import json
 import mock
+import os.path
 import re
 
 from base64 import b64encode, b64decode
@@ -55,6 +56,8 @@ GCE_USER_DATA_TEXT = {
 HEADERS = {'Metadata-Flavor': 'Google'}
 MD_URL_RE = re.compile(
     r'http://metadata.google.internal/computeMetadata/v1/.*')
+GUEST_ATTRIBUTES_URL = ('http://metadata.google.internal/computeMetadata/'
+                        'v1beta1/instance/guest-attributes/hostkeys/')
 
 
 def _set_mock_metadata(gce_meta=None):
@@ -341,4 +344,54 @@ class TestDataSourceGCE(test_helpers.HttprettyTestCase):
             public_key_data, default_user='default')
         self.assertEqual(sorted(found), sorted(expected))
 
+    @mock.patch("cloudinit.sources.DataSourceGCE._get_public_host_keys")
+    @mock.patch("cloudinit.util.multi_log")
+    @mock.patch("cloudinit.url_helper.readurl")
+    def test_publish_host_keys(self, m_readurl, m_multi_log, m_get_host_keys):
+        blacklist = ['ssh-dss']
+        m_get_host_keys.return_value = [('ssh-rsa', 'asdfasdf'),
+                                        ('ssh-dss', 'hjklhjkl'),
+                                        ('ssh-ed25519', 'qwerqwer')]
+        readurl_expected_calls = [
+            mock.call(check_status=False, data=b'asdfasdf', headers=HEADERS,
+                      request_method='PUT',
+                      url='%s%s' % (GUEST_ATTRIBUTES_URL, 'ssh-rsa')),
+            mock.call(check_status=False, data=b'qwerqwer', headers=HEADERS,
+                      request_method='PUT',
+                      url='%s%s' % (GUEST_ATTRIBUTES_URL, 'ssh-ed25519')),
+        ]
+        readurl_expected_not_called = [
+            mock.call(check_status=False, data=b'hjklhjkl', headers=HEADERS,
+                      request_method='PUT',
+                      url='%s%s' % (GUEST_ATTRIBUTES_URL, 'ssh-dss')),
+        ]
+        self.ds.publish_host_keys(blacklist)
+        m_readurl.assert_has_calls(readurl_expected_calls, any_order=True)
+        with self.assertRaises(AssertionError):
+            m_readurl.assert_has_calls(readurl_expected_not_called,
+                                       any_order=True)
+        multilog_expected_calls = [
+            mock.call('Wrote ssh-rsa host key to guest attributes.\n'),
+            mock.call('Wrote ssh-ed25519 host key to guest attributes.\n'),
+        ]
+        m_multi_log.assert_has_calls(multilog_expected_calls, any_order=True)
+
+    def test_get_public_host_keys(self):
+        keys = {'ssh_host_dsa_key.pub': ['ssh-dss', 'AAAAB3NzaC1kc3MAAACB'],
+                'ssh_host_ecdsa_key.pub': ['ecdsa-sha2-nistp256', 'AAAAE2VjZ'],
+                'ssh_host_ed25519_key.pub': ['ssh-ed25519', 'AAAAC3NzaC1lZDI'],
+                'ssh_host_rsa_key.pub': ['ssh-rsa', 'AAAAB3NzaC1yc2EAAA'],
+                }
+        hostkey_tmpdir = self.tmp_dir()
+        for filename, key_data in keys.items():
+            filepath = os.path.join(hostkey_tmpdir, filename)
+            with open(filepath, 'w') as f:
+                f.write(' '.join(key_data))
+
+        host_keys = DataSourceGCE._get_public_host_keys(key_dir=hostkey_tmpdir)
+
+        for k in keys.values():
+            self.assertIn(k, host_keys)
+
+
 # vi: ts=4 expandtab

Follow ups