Bugfix: Remove users keys when gone from metadata

Previously, the manage accounts daemon relied solely on the metadata
server to know which accounts to examine. It ignored all other accounts.
This means that, when the last reference to an account was removed from
the metadata server, the final SSH key would linger untouched.

Now, enumerate all accounts on the system (as revealed by the system's
passwd database). If an account has a ~/.ssh/authorized_keys file but
was not reflected in the metadata server, run our key update routine
with an empty list of SSH keys to remove any added-by-Google keys that
may be present.

I successfully tested this on a backports Debian image, by killing the
running daemon, copying over the files, starting the daemon, ensuring it
could create a new user as before, and ensuring it deletes that user's
only key after the user is removed from the metadata server.

2 files changed, 45 insertions, 21 deletions

diff --git a/google-daemon/usr/share/google/google_daemon/accounts.py b/google-daemon/usr/share/google/google_daemon/accounts.py
index c691ba6..fcc93d9 100755
--- a/google-daemon/usr/share/google/google_daemon/accounts.py
+++ b/google-daemon/usr/share/google/google_daemon/accounts.py
@@ -13,9 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Create accounts needed on this GCE instance.
+"""Update accounts needed on this GCE instance.
 
-Creates accounts based on the contents of ACCOUNTS_URL, which should contain a
+Update accounts based on the contents of ACCOUNTS_URL, which should contain a
 newline-delimited file of accounts and SSH public keys. Each line represents a
 SSH public key which should be allowed to log in to that account.
 
@@ -78,7 +78,7 @@ class Accounts(object):
     self.default_user_groups = self.GroupsThatExist(
         ['adm', 'video', 'dip', 'plugdev', 'sudo'])
 
-  def CreateUser(self, username, ssh_keys):
+  def UpdateUser(self, username, ssh_keys):
     """Create username on the system, with authorized ssh_keys."""
 
     if not self.IsValidUsername(username):
@@ -91,7 +91,11 @@ class Accounts(object):
       self.system.UserAdd(username, self.default_user_groups)
 
     if self.UserExists(username):
-      self.MakeUserSudoer(username)
+      # If we're just removing keys from a user who may have been in the
+      # metadata server but isn't currently, we should never increase their
+      # privileges. Therefore, only grant sudo access if we have ssh keys.
+      if ssh_keys:
+        self.MakeUserSudoer(username)
       self.AuthorizeSshKeys(username, ssh_keys)
 
   def IsValidUsername(self, username):
diff --git a/google-daemon/usr/share/google/google_daemon/accounts_manager.py b/google-daemon/usr/share/google/google_daemon/accounts_manager.py
index 6ca9dd5..0e9897b 100644
--- a/google-daemon/usr/share/google/google_daemon/accounts_manager.py
+++ b/google-daemon/usr/share/google/google_daemon/accounts_manager.py
@@ -17,6 +17,7 @@
 import json
 import logging
 import os
+import pwd
 import time
 
 LOCKFILE = '/var/lock/manage-accounts.lock'
@@ -43,11 +44,11 @@ class AccountsManager(object):
     # Otherwise, run the actual operations in a subprocess so that any
     # errors don't kill the long-lived process.
     if self.single_pass:
-      self.RegenerateKeysAndCreateAccounts()
+      self.RegenerateKeysAndUpdateAccounts()
       return
     # Run this forever in a loop.
     while True:
-      # Fork and run the key regeneration and account creation while the
+      # Fork and run the key regeneration and account update while the
       # parent waits for the subprocess to finish before continuing.
       
       # Create a pipe used to get the new etag value from child
@@ -71,10 +72,10 @@ class AccountsManager(object):
         os.close(reader)
         writer = os.fdopen(writer, 'w')
         try:
-          self.RegenerateKeysAndCreateAccounts()
+          self.RegenerateKeysAndUpdateAccounts()
         except Exception as e:
-          logging.warning('error while trying to create accounts: %s', e)
-          # An error happened while trying to create the accounts. Lets sleep a
+          logging.warning('error while trying to update accounts: %s', e)
+          # An error happened while trying to update the accounts. Lets sleep a
           # bit to avoid getting stuck in a loop for intermittent errors.
           time.sleep(5)
 
@@ -91,23 +92,42 @@ class AccountsManager(object):
         # once by the subprocess and once by the parent process.
         os._exit(0)
 
-  def RegenerateKeysAndCreateAccounts(self):
-    """Regenerate the keys and create accounts as needed."""
-    logging.debug('RegenerateKeysAndCreateAccounts')
+  def RegenerateKeysAndUpdateAccounts(self):
+    """Regenerate the keys and update accounts as needed."""
+    logging.debug('RegenerateKeysAndUpdateAccounts')
     if self.system.IsExecutable('/usr/share/google/first-boot'):
       self.system.RunCommand('/usr/share/google/first-boot')
 
-    self.lock_file.RunExclusively(self.lock_fname, self.CreateAccounts)
+    self.lock_file.RunExclusively(self.lock_fname, self.UpdateAccounts)
 
-  def CreateAccounts(self):
-    """Create all accounts that should be present."""
+  def UpdateAccounts(self):
+    """Update all accounts that should be present or exist already."""
+
+    # Note GetDesiredAccounts() returns a dict of username->sshKeys mappings.
     desired_accounts = self.desired_accounts.GetDesiredAccounts()
+
+    # Plan a processing pass for extra accounts existing on the system with a
+    # ~/.ssh/authorized_keys file, even if they're not otherwise in the metadata
+    # server; this will only ever remove the last added-by-Google key from
+    # accounts which were formerly in the metadata server but are no longer.
+    all_accounts = pwd.getpwall()
+    keyfile_suffix = os.path.join('.ssh', 'authorized_keys')
+    sshable_usernames = [
+        entry.pw_name
+        for entry in all_accounts
+        if os.path.isfile(os.path.join(entry.pw_dir, keyfile_suffix))]
+    extra_usernames = set(sshable_usernames) - set(desired_accounts.keys())
     
-    if not desired_accounts:
-      return
+    if desired_accounts:
+      for username, ssh_keys in desired_accounts.iteritems():
+        if not username:
+          continue
 
-    for username, ssh_keys in desired_accounts.iteritems():
-      if not username:
-        continue
+        self.accounts.UpdateUser(username, ssh_keys)
 
-      self.accounts.CreateUser(username, ssh_keys)
+    for username in extra_usernames:
+      # If a username is present in extra_usernames, it is no longer reflected
+      # in the metadata server but has an authorized_keys file. Therefore, we
+      # should pass the empty list for sshKeys to ensure that any Google-managed
+      # keys are no longer authorized.
+      self.accounts.UpdateUser(username, [])