cloud-init-dev team mailing list archive

[Evan Zacks <zackse@xxxxxxxxx>]

Evan Zacks has proposed merging ~zackse/cloud-init:ssh_util-authorizedkeysfile-allow-multiple-values into cloud-init:master.

Commit message:
ssh_util: support AuthorizedKeysFile directive with multiple values

As of OpenSSH 5.9, the AuthorizedKeysFile directive in sshd_config
supports multiple values.

This change changes extract_authorized_keys to walk the list of entries,
selecting the first value with a matching file on disk.

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

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

Proposing for initial review/comments -- no tests included yet.

Given an EC2 instance with an sshd_configuration with the
following directive:

  AuthorizedKeysFile /etc/ssh_keys/%u .ssh/authorized_keys

cloud-init would create the following structure for the centos user:
    
  /etc/ssh_keys/
  /etc/ssh_keys/centos .ssh
  /etc/ssh_keys/centos .ssh/authorized_keys

because it treats the AuthorizedKeysFile entry as a single value
vs. a whitespace-separated list of values.

As written the code is a bit "deep" in the existing routine...

It also changes the behavior of the case where no matching file
is found -- previously the code would use the value from the config
file, whether a file existed or not. Now there is a default case of
~/.ssh/authorized_keys (the sshd_config default).

If this approach (diff best viewed with `--ignore-whitespace`) were
deemed acceptable, I could use some help in test placement. Should
we add a new class in tests/unittests/test_sshutil.py that mocks out
util.SeLinuxGuard, parse_ssh_config_map, etc., or should the change
be reworked to modify parse_ssh_config_map, for example? 
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~zackse/cloud-init:ssh_util-authorizedkeysfile-allow-multiple-values into cloud-init:master.

diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
index 3f99b58..09fdb1a 100644
--- a/cloudinit/ssh_util.py
+++ b/cloudinit/ssh_util.py
@@ -221,15 +221,21 @@ def extract_authorized_keys(username):
             # The following tokens are defined: %% is replaced by a literal
             # '%', %h is replaced by the home directory of the user being
             # authenticated and %u is replaced by the username of that user.
+            # Note: multiple values are allowed, separated by whitespace.
+            # In this case, we attempt to use the first value with a matching
+            # file on disk, falling back to the default ~/.ssh/authorized_keys.
             ssh_cfg = parse_ssh_config_map(DEF_SSHD_CFG)
-            auth_key_fn = ssh_cfg.get("authorizedkeysfile", '').strip()
-            if not auth_key_fn:
+            auth_fns = ssh_cfg.get("authorizedkeysfile", '').strip().split()
+            for auth_key_fn in auth_fns:
+                auth_key_fn = auth_key_fn.replace("%h", pw_ent.pw_dir)
+                auth_key_fn = auth_key_fn.replace("%u", username)
+                auth_key_fn = auth_key_fn.replace("%%", '%')
+                if not auth_key_fn.startswith('/'):
+                    auth_key_fn = os.path.join(pw_ent.pw_dir, auth_key_fn)
+                if os.path.isfile(auth_key_fn):
+                    break
+            else:
                 auth_key_fn = "%h/.ssh/authorized_keys"
-            auth_key_fn = auth_key_fn.replace("%h", pw_ent.pw_dir)
-            auth_key_fn = auth_key_fn.replace("%u", username)
-            auth_key_fn = auth_key_fn.replace("%%", '%')
-            if not auth_key_fn.startswith('/'):
-                auth_key_fn = os.path.join(pw_ent.pw_dir, auth_key_fn)
         except (IOError, OSError):
             # Give up and use a default key filename
             auth_key_fn = os.path.join(ssh_dir, 'authorized_keys')