Date: Mon, 15 Feb 2016 13:32:13 +0100 From: "Jason A. Donenfeld" <Jason@...c4.com> To: oss-security <oss-security@...ts.openwall.com>, harlowja@...il.com, smoser@...ckies.net Subject: cloud-init follows symlinks for ssh authorized_keys Hi folks, Cloud-init is a service run on "cloud" distributions that takes data provided by the VM administrator and uses it to configure the system. It can be used to add or update users and related SSH keys. FWIW, the SSH key handling code (and possibly other places too, haven't checked) appears to follow symlinks. This means that a local user could "ln -s somewhere/else ~/.ssh/authorized_keys" or perhaps even "ln -s somewhere/else ~/.ssh" (for the auto-chown/chmod). Then, sometime later, say the administrator updates this malicious user's SSH key using the metadata available to cloud-init, and reboots. Uh oh speghettio. I didn't really check these findings in practice, so you might want to take a closer look, but it doesn't appear pretty. http://bazaar.launchpad.net/~cloud-init-dev/cloud-init/trunk/view/head:/cloudinit/ssh_util.py Here are a few places where this happens: def parse_authorized_keys(fname): lines =  try: if os.path.isfile(fname): lines = util.load_file(fname).splitlines() except (IOError, OSError): util.logexc(LOG, "Error reading lines from %s", fname) lines =  parser = AuthKeyLineParser() contents =  for line in lines: contents.append(parser.parse(line)) return contents According to the python documentation: os.path.isfile(path) Return True if path is an existing regular file. This follows symbolic links, so both islink() and isfile() can be true for the same path. def setup_user_keys(keys, username, options=None): # Make sure the users .ssh dir is setup accordingly (ssh_dir, pwent) = users_ssh_info(username) if not os.path.isdir(ssh_dir): util.ensure_dir(ssh_dir, mode=0o700) util.chownbyid(ssh_dir, pwent.pw_uid, pwent.pw_gid) # Turn the 'update' keys given into actual entries parser = AuthKeyLineParser() key_entries =  for k in keys: key_entries.append(parser.parse(str(k), options=options)) # Extract the old and make the new (auth_key_fn, auth_key_entries) = extract_authorized_keys(username) with util.SeLinuxGuard(ssh_dir, recursive=True): content = update_authorized_keys(auth_key_entries, key_entries) util.ensure_dir(os.path.dirname(auth_key_fn), mode=0o700) util.write_file(auth_key_fn, content, mode=0o600) util.chownbyid(auth_key_fn, pwent.pw_uid, pwent.pw_gid) Again, os.path.isdir follows symlinks, and so do chown and chmod, and also the functions underlying write_file. By the way there are some more race condition situations happening in the latter function, among others, in which directories can be removed or changed around after the "ensure" check. Whether or not that constitutes a security issue remains to be seen. Anyway, make of this what you will. Is this a vector? Is this not a vector? It's certainly not very robust code in any case. Regards, Jason
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.