Merge branch 'development'

debian/changelog

@@ -1,3 +1,10 @@
+google-compute-image-packages (2.8.5-1) stable; urgency=low
+
+  * Remove users from sudoers group on removal.
+  * Remove gsutil dependency for metadata scripts.
+
+ -- Google Cloud Team <[email protected]>  Thu, 05 Oct 2018 12:00:00 -0700
+
 google-compute-image-packages (2.8.4-1) stable; urgency=low
 
   * Remove ntp dependency.

debian/control

@@ -30,7 +30,7 @@ Depends: google-compute-engine-oslogin,
          python3-google-compute-engine (= ${source:Version}),
          system-log-daemon,
          systemd
-Recommends: google-cloud-sdk, rsyslog
+Recommends: rsyslog
 Provides: irqbalance
 Conflicts: google-compute-engine-jessie,
            google-compute-engine-init-jessie,

google_compute_engine/accounts/accounts_utils.py

@@ -27,6 +27,7 @@
 from google_compute_engine import file_utils
 
 USER_REGEX = re.compile(r'\A[A-Za-z0-9._][A-Za-z0-9._-]{0,31}\Z')
+DEFAULT_GPASSWD_CMD = 'gpasswd -d {user} {group}'
 DEFAULT_GROUPADD_CMD = 'groupadd {group}'
 DEFAULT_USERADD_CMD = 'useradd -m -s /bin/bash -p * {user}'
 DEFAULT_USERDEL_CMD = 'userdel -r {user}'
@@ -39,19 +40,21 @@ class AccountsUtils(object):
   google_comment = '# Added by Google'
 
   def __init__(
-      self, logger, groups=None, remove=False, groupadd_cmd=None,
-      useradd_cmd=None, userdel_cmd=None, usermod_cmd=None):
+      self, logger, groups=None, remove=False, gpasswd_cmd=None,
+      groupadd_cmd=None, useradd_cmd=None, userdel_cmd=None, usermod_cmd=None):
     """Constructor.
 
     Args:
       logger: logger object, used to write to SysLog and serial port.
       groups: string, a comma separated list of groups.
       remove: bool, True if deprovisioning a user should be destructive.
+      gpasswd_cmd: string, command to remove a user from a group.
       groupadd_cmd: string, command to add a new group.
       useradd_cmd: string, command to create a new user.
       userdel_cmd: string, command to delete a user.
       usermod_cmd: string, command to modify user's groups.
     """
+    self.gpasswd_cmd = gpasswd_cmd or DEFAULT_GPASSWD_CMD
     self.groupadd_cmd = groupadd_cmd or DEFAULT_GROUPADD_CMD
     self.useradd_cmd = useradd_cmd or DEFAULT_USERADD_CMD
     self.userdel_cmd = userdel_cmd or DEFAULT_USERDEL_CMD
@@ -242,6 +245,27 @@ def _UpdateAuthorizedKeys(self, user, ssh_keys):
     file_utils.SetPermissions(
         authorized_keys_file, mode=0o600, uid=uid, gid=gid)
 
+  def _RemoveSudoer(self, user):
+    """Remove a Linux user account from the sudoers group.
+
+    Args:
+      user: string, the name of the Linux user account.
+
+    Returns:
+      bool, True if user update succeeded.
+    """
+    self.logger.debug('Removing user %s from the Google sudoers group.', user)
+    command = self.gpasswd_cmd.format(
+        user=user, group=self.google_sudoers_group)
+    try:
+      subprocess.check_call(command.split(' '))
+    except subprocess.CalledProcessError as e:
+      self.logger.warning('Could not update user %s. %s.', user, str(e))
+      return False
+    else:
+      self.logger.debug('Removed user %s from the Google sudoers group.', user)
+      return True
+
   def _RemoveAuthorizedKeys(self, user):
     """Remove a Linux user account's authorized keys file to prevent login.
 
@@ -337,6 +361,7 @@ def RemoveUser(self, user):
       user: string, the Linux user account to remove.
     """
     self.logger.info('Removing user %s.', user)
+    self._RemoveSudoer(user)
     if self.remove:
       command = self.userdel_cmd.format(user=user)
       try:

google_compute_engine/accounts/tests/accounts_utils_test.py

@@ -31,10 +31,11 @@ def setUp(self):
     self.sudoers_file = '/sudoers/file'
     self.users_dir = '/users'
     self.users_file = '/users/file'
+    self.gpasswd_cmd = 'useradd -m -s /bin/bash -p * {user}'
+    self.groupadd_cmd = 'groupadd {group}'
     self.useradd_cmd = 'useradd -m -s /bin/bash -p * {user}'
     self.userdel_cmd = 'userdel -r {user}'
     self.usermod_cmd = 'usermod -G {groups} {user}'
-    self.groupadd_cmd = 'groupadd {group}'
 
     self.mock_utils = mock.create_autospec(accounts_utils.AccountsUtils)
     self.mock_utils.google_comment = accounts_utils.AccountsUtils.google_comment
@@ -43,10 +44,11 @@ def setUp(self):
     self.mock_utils.google_users_dir = self.users_dir
     self.mock_utils.google_users_file = self.users_file
     self.mock_utils.logger = self.mock_logger
+    self.mock_utils.gpasswd_cmd = self.gpasswd_cmd
+    self.mock_utils.groupadd_cmd = self.groupadd_cmd
     self.mock_utils.useradd_cmd = self.useradd_cmd
     self.mock_utils.userdel_cmd = self.userdel_cmd
     self.mock_utils.usermod_cmd = self.usermod_cmd
-    self.mock_utils.groupadd_cmd = self.groupadd_cmd
 
   @mock.patch('google_compute_engine.accounts.accounts_utils.AccountsUtils._GetGroup')
   @mock.patch('google_compute_engine.accounts.accounts_utils.AccountsUtils._CreateSudoersGroup')
@@ -428,6 +430,35 @@ def testUpdateAuthorizedKeysSymlink(self, mock_islink, mock_permissions):
     self.mock_logger.warning.assert_called_once_with(mock.ANY, user)
     mock_permissions.assert_not_called()
 
+  @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
+  def testRemoveSudoer(self, mock_call):
+    user = 'user'
+    command = self.usermod_cmd.format(user=user, groups=self.sudoers_group)
+
+    self.assertTrue(
+        accounts_utils.AccountsUtils._RemoveSudoer(self.mock_utils, user))
+    mock.call.assert_called_once_with(command.split(' ')),
+    expected_calls = [
+        mock.call.debug(mock.ANY, user),
+        mock.call.debug(mock.ANY, user),
+    ]
+    self.assertEqual(self.mock_logger.mock_calls, expected_calls)
+
+  @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
+  def testRemoveSudoerError(self, mock_call):
+    user = 'user'
+    command = self.usermod_cmd.format(user=user, groups=self.sudoers_group)
+    mock_call.side_effect = subprocess.CalledProcessError(1, 'Test')
+
+    self.assertFalse(
+        accounts_utils.AccountsUtils._RemoveSudoer(self.mock_utils, user))
+    mock.call.assert_called_once_with(command.split(' ')),
+    expected_calls = [
+        mock.call.debug(mock.ANY, user),
+        mock.call.warning(mock.ANY, user, mock.ANY),
+    ]
+    self.assertEqual(self.mock_logger.mock_calls, expected_calls)
+
   @mock.patch('google_compute_engine.accounts.accounts_utils.os.remove')
   @mock.patch('google_compute_engine.accounts.accounts_utils.os.path.exists')
   def testRemoveAuthorizedKeys(self, mock_exists, mock_remove):
@@ -642,6 +673,7 @@ def testRemoveUser(self, mock_call):
     self.mock_utils.remove = False
 
     accounts_utils.AccountsUtils.RemoveUser(self.mock_utils, user)
+    self.mock_utils._RemoveSudoer.assert_called_once_with(user)
     self.mock_utils._RemoveAuthorizedKeys.assert_called_once_with(user)
     mock_call.assert_not_called()
 
@@ -655,6 +687,7 @@ def testRemoveUserForce(self, mock_call):
     mock.call.assert_called_once_with(command.split(' ')),
     expected_calls = [mock.call.info(mock.ANY, user)] * 2
     self.assertEqual(self.mock_logger.mock_calls, expected_calls)
+    self.mock_utils._RemoveSudoer.assert_called_once_with(user)
     self.mock_utils._RemoveAuthorizedKeys.assert_called_once_with(user)
 
   @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
@@ -671,6 +704,7 @@ def testRemoveUserError(self, mock_call):
         mock.call.warning(mock.ANY, user, mock.ANY),
     ]
     self.assertEqual(self.mock_logger.mock_calls, expected_calls)
+    self.mock_utils._RemoveSudoer.assert_called_once_with(user)
     self.mock_utils._RemoveAuthorizedKeys.assert_called_once_with(user)
 
 

google_compute_engine/metadata_scripts/script_retriever.py

@@ -15,6 +15,7 @@
 
 """Retrieve and store user provided metadata scripts."""
 
+import ast
 import re
 import socket
 import subprocess
@@ -23,11 +24,15 @@
 from google_compute_engine import metadata_watcher
 from google_compute_engine.compat import httpclient
 from google_compute_engine.compat import urlerror
+from google_compute_engine.compat import urlrequest
 from google_compute_engine.compat import urlretrieve
 
 
 class ScriptRetriever(object):
   """A class for retrieving and storing user provided metadata scripts."""
+  token_metadata_key = 'instance/service-accounts/default/token'
+  # Cached authentication token to be used when downloading from bucket.
+  token = None
 
   def __init__(self, logger, script_type):
     """Constructor.
@@ -40,8 +45,10 @@ def __init__(self, logger, script_type):
     self.script_type = script_type
     self.watcher = metadata_watcher.MetadataWatcher(logger=self.logger)
 
-  def _DownloadGsUrl(self, url, dest_dir):
-    """Download a Google Storage URL using gsutil.
+  def _DownloadAuthUrl(self, url, dest_dir):
+    """Download a Google Storage URL using an authentication token.
+
+    If the token cannot be fetched, fallback to unauthenticated download.
 
     Args:
       url: string, the URL to download.
@@ -50,29 +57,39 @@ def _DownloadGsUrl(self, url, dest_dir):
     Returns:
       string, the path to the file storing the metadata script.
     """
-    try:
-      subprocess.check_call(
-          ['which', 'gsutil'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    except subprocess.CalledProcessError:
-      self.logger.warning(
-          'gsutil is not installed, cannot download items from Google Storage.')
-      return None
-
     dest_file = tempfile.NamedTemporaryFile(dir=dest_dir, delete=False)
     dest_file.close()
     dest = dest_file.name
 
-    self.logger.info('Downloading url from %s to %s using gsutil.', url, dest)
+    self.logger.info(
+        'Downloading url from %s to %s using authentication token.', url, dest)
+
+    if not self.token:
+      response = self.watcher.GetMetadata(
+          self.token_metadata_key, recursive=False, retry=False)
+
+      if not response:
+        self.logger.info(
+            'Authentication token not found. Attempting unauthenticated '
+            'download.')
+        return self._DownloadUrl(url, dest_dir)
+
+      self.token = '%s %s' % (
+          response.get('token_type', ''), response.get('access_token', ''))
+
     try:
-      subprocess.check_call(['gsutil', 'cp', url, dest])
-      return dest
-    except subprocess.CalledProcessError as e:
-      self.logger.warning(
-          'Could not download %s using gsutil. %s.', url, str(e))
-    except Exception as e:
-      self.logger.warning(
-          'Exception downloading %s using gsutil. %s.', url, str(e))
-    return None
+      request = urlrequest.Request(url)
+      request.add_unredirected_header('Metadata-Flavor', 'Google')
+      request.add_unredirected_header('Authorization', self.token)
+      content = urlrequest.urlopen(request).read()
+    except (httpclient.HTTPException, socket.error, urlerror.URLError) as e:
+      self.logger.warning('Could not download %s. %s.', url, str(e))
+      return None
+
+    with open(dest, 'w') as f:
+      f.write(content)
+
+    return dest
 
   def _DownloadUrl(self, url, dest_dir):
     """Download a script from a given URL.
@@ -111,7 +128,9 @@ def _DownloadScript(self, url, dest_dir):
     # Check for the preferred Google Storage URL format:
     # gs://<bucket>/<object>
     if url.startswith(r'gs://'):
-      return self._DownloadGsUrl(url, dest_dir)
+      # Convert the string into a standard URL.
+      url = re.sub('^gs://', 'https://storage.googleapis.com/', url)
+      return self._DownloadAuthUrl(url, dest_dir)
 
     header = r'http[s]?://'
     domain = r'storage\.googleapis\.com'
@@ -122,10 +141,6 @@ def _DownloadScript(self, url, dest_dir):
     bucket = r'(?P<bucket>[a-z0-9][-_.a-z0-9]*[a-z0-9])'
 
     # Accept any non-empty string that doesn't contain a wildcard character
-    # gsutil interprets some characters as wildcards.
-    # These characters in object names make it difficult or impossible
-    # to perform various wildcard operations using gsutil
-    # For a complete list use "gsutil help naming".
     obj = r'(?P<obj>[^\*\?]+)'
 
     # Check for the Google Storage URLs:
@@ -134,10 +149,7 @@ def _DownloadScript(self, url, dest_dir):
     gs_regex = re.compile(r'\A%s%s\.%s/%s\Z' % (header, bucket, domain, obj))
     match = gs_regex.match(url)
     if match:
-      gs_url = r'gs://%s/%s' % (match.group('bucket'), match.group('obj'))
-      # In case gsutil is not installed, continue as a normal URL.
-      return (self._DownloadGsUrl(gs_url, dest_dir) or
-              self._DownloadUrl(url, dest_dir))
+      return self._DownloadAuthUrl(url, dest_dir)
 
     # Check for the other possible Google Storage URLs:
     # http://storage.googleapis.com/<bucket>/<object>
@@ -150,10 +162,7 @@ def _DownloadScript(self, url, dest_dir):
         r'\A%s(commondata)?%s/%s/%s\Z' % (header, domain, bucket, obj))
     match = gs_regex.match(url)
     if match:
-      gs_url = r'gs://%s/%s' % (match.group('bucket'), match.group('obj'))
-      # In case gsutil is not installed, continue as a normal URL.
-      return (self._DownloadGsUrl(gs_url, dest_dir) or
-              self._DownloadUrl(url, dest_dir))
+      return self._DownloadAuthUrl(url, dest_dir)
 
     # Unauthenticated download of the object.
     return self._DownloadUrl(url, dest_dir)
@@ -173,7 +182,7 @@ def _GetAttributeScripts(self, attribute_data, dest_dir):
     metadata_key = '%s-script' % self.script_type
     metadata_value = attribute_data.get(metadata_key)
     if metadata_value:
-      self.logger.info('Found %s in metadata.' % metadata_key)
+      self.logger.info('Found %s in metadata.', metadata_key)
       with tempfile.NamedTemporaryFile(
           mode='w', dir=dest_dir, delete=False) as dest:
         dest.write(metadata_value.lstrip())
@@ -182,8 +191,9 @@ def _GetAttributeScripts(self, attribute_data, dest_dir):
     metadata_key = '%s-script-url' % self.script_type
     metadata_value = attribute_data.get(metadata_key)
     if metadata_value:
-      self.logger.info('Found %s in metadata.' % metadata_key)
-      script_dict[metadata_key] = self._DownloadScript(metadata_value, dest_dir)
+      self.logger.info('Found %s in metadata.', metadata_key)
+      script_dict[metadata_key] = self._DownloadScript(
+          metadata_value, dest_dir)
 
     return script_dict