Blob Blame History Raw
From f6dc3cf39a4884657478a47894ce8a76ec9a72c5 Mon Sep 17 00:00:00 2001
From: Eduardo Otubo <otubo@redhat.com>
Date: Wed, 24 Jun 2020 07:34:29 +0200
Subject: [PATCH 1/4] ec2: Do not log IMDSv2 token values, instead use REDACTED
 (#219)

RH-Author: Eduardo Otubo <otubo@redhat.com>
Message-id: <20200505082940.18316-1-otubo@redhat.com>
Patchwork-id: 96264
O-Subject: [RHEL-7.9/RHEL-8.3 cloud-init PATCH] ec2: Do not log IMDSv2 token values, instead use REDACTED (#219)
Bugzilla: 1822343
RH-Acked-by: Cathy Avery <cavery@redhat.com>
RH-Acked-by: Mohammed Gamal <mgamal@redhat.com>
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Note: There's no RHEL-8.3/cloud-init-19.4 branch yet, but it should be
queued to be applied on top of it when it's created.

commit 87cd040ed8fe7195cbb357ed3bbf53cd2a81436c
Author: Ryan Harper <ryan.harper@canonical.com>
Date:   Wed Feb 19 15:01:09 2020 -0600

    ec2: Do not log IMDSv2 token values, instead use REDACTED (#219)

    Instead of logging the token values used log the headers and replace the actual
    values with the string 'REDACTED'.  This allows users to examine cloud-init.log
    and see that the IMDSv2 token header is being used but avoids leaving the value
    used in the log file itself.

    LP: #1863943

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 cloudinit/ec2_utils.py                      | 12 ++++++++--
 cloudinit/sources/DataSourceEc2.py          | 35 +++++++++++++++++++----------
 cloudinit/url_helper.py                     | 27 ++++++++++++++++------
 tests/unittests/test_datasource/test_ec2.py | 17 ++++++++++++++
 4 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
index 57708c1..34acfe8 100644
--- a/cloudinit/ec2_utils.py
+++ b/cloudinit/ec2_utils.py
@@ -142,7 +142,8 @@ def skip_retry_on_codes(status_codes, _request_args, cause):
 def get_instance_userdata(api_version='latest',
                           metadata_address='http://169.254.169.254',
                           ssl_details=None, timeout=5, retries=5,
-                          headers_cb=None, exception_cb=None):
+                          headers_cb=None, headers_redact=None,
+                          exception_cb=None):
     ud_url = url_helper.combine_url(metadata_address, api_version)
     ud_url = url_helper.combine_url(ud_url, 'user-data')
     user_data = ''
@@ -155,7 +156,8 @@ def get_instance_userdata(api_version='latest',
                                              SKIP_USERDATA_CODES)
         response = url_helper.read_file_or_url(
             ud_url, ssl_details=ssl_details, timeout=timeout,
-            retries=retries, exception_cb=exception_cb, headers_cb=headers_cb)
+            retries=retries, exception_cb=exception_cb, headers_cb=headers_cb,
+            headers_redact=headers_redact)
         user_data = response.contents
     except url_helper.UrlError as e:
         if e.code not in SKIP_USERDATA_CODES:
@@ -169,11 +171,13 @@ def _get_instance_metadata(tree, api_version='latest',
                            metadata_address='http://169.254.169.254',
                            ssl_details=None, timeout=5, retries=5,
                            leaf_decoder=None, headers_cb=None,
+                           headers_redact=None,
                            exception_cb=None):
     md_url = url_helper.combine_url(metadata_address, api_version, tree)
     caller = functools.partial(
         url_helper.read_file_or_url, ssl_details=ssl_details,
         timeout=timeout, retries=retries, headers_cb=headers_cb,
+        headers_redact=headers_redact,
         exception_cb=exception_cb)
 
     def mcaller(url):
@@ -197,6 +201,7 @@ def get_instance_metadata(api_version='latest',
                           metadata_address='http://169.254.169.254',
                           ssl_details=None, timeout=5, retries=5,
                           leaf_decoder=None, headers_cb=None,
+                          headers_redact=None,
                           exception_cb=None):
     # Note, 'meta-data' explicitly has trailing /.
     # this is required for CloudStack (LP: #1356855)
@@ -204,6 +209,7 @@ def get_instance_metadata(api_version='latest',
                                   metadata_address=metadata_address,
                                   ssl_details=ssl_details, timeout=timeout,
                                   retries=retries, leaf_decoder=leaf_decoder,
+                                  headers_redact=headers_redact,
                                   headers_cb=headers_cb,
                                   exception_cb=exception_cb)
 
@@ -212,12 +218,14 @@ def get_instance_identity(api_version='latest',
                           metadata_address='http://169.254.169.254',
                           ssl_details=None, timeout=5, retries=5,
                           leaf_decoder=None, headers_cb=None,
+                          headers_redact=None,
                           exception_cb=None):
     return _get_instance_metadata(tree='dynamic/instance-identity',
                                   api_version=api_version,
                                   metadata_address=metadata_address,
                                   ssl_details=ssl_details, timeout=timeout,
                                   retries=retries, leaf_decoder=leaf_decoder,
+                                  headers_redact=headers_redact,
                                   headers_cb=headers_cb,
                                   exception_cb=exception_cb)
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index b9f346a..0f2bfef 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -31,6 +31,9 @@ STRICT_ID_DEFAULT = "warn"
 API_TOKEN_ROUTE = 'latest/api/token'
 API_TOKEN_DISABLED = '_ec2_disable_api_token'
 AWS_TOKEN_TTL_SECONDS = '21600'
+AWS_TOKEN_PUT_HEADER = 'X-aws-ec2-metadata-token'
+AWS_TOKEN_REQ_HEADER = AWS_TOKEN_PUT_HEADER + '-ttl-seconds'
+AWS_TOKEN_REDACT = [AWS_TOKEN_PUT_HEADER, AWS_TOKEN_REQ_HEADER]
 
 
 class CloudNames(object):
@@ -158,7 +161,8 @@ class DataSourceEc2(sources.DataSource):
         for api_ver in self.extended_metadata_versions:
             url = url_tmpl.format(self.metadata_address, api_ver)
             try:
-                resp = uhelp.readurl(url=url, headers=headers)
+                resp = uhelp.readurl(url=url, headers=headers,
+                                     headers_redact=AWS_TOKEN_REDACT)
             except uhelp.UrlError as e:
                 LOG.debug('url %s raised exception %s', url, e)
             else:
@@ -180,6 +184,7 @@ class DataSourceEc2(sources.DataSource):
                 self.identity = ec2.get_instance_identity(
                     api_version, self.metadata_address,
                     headers_cb=self._get_headers,
+                    headers_redact=AWS_TOKEN_REDACT,
                     exception_cb=self._refresh_stale_aws_token_cb).get(
                         'document', {})
             return self.identity.get(
@@ -205,7 +210,8 @@ class DataSourceEc2(sources.DataSource):
         LOG.debug('Fetching Ec2 IMDSv2 API Token')
         url, response = uhelp.wait_for_url(
             urls=urls, max_wait=1, timeout=1, status_cb=self._status_cb,
-            headers_cb=self._get_headers, request_method=request_method)
+            headers_cb=self._get_headers, request_method=request_method,
+            headers_redact=AWS_TOKEN_REDACT)
 
         if url and response:
             self._api_token = response
@@ -252,7 +258,8 @@ class DataSourceEc2(sources.DataSource):
             url, _ = uhelp.wait_for_url(
                 urls=urls, max_wait=url_params.max_wait_seconds,
                 timeout=url_params.timeout_seconds, status_cb=LOG.warning,
-                headers_cb=self._get_headers, request_method=request_method)
+                headers_redact=AWS_TOKEN_REDACT, headers_cb=self._get_headers,
+                request_method=request_method)
 
             if url:
                 metadata_address = url2base[url]
@@ -420,6 +427,7 @@ class DataSourceEc2(sources.DataSource):
         if not self.wait_for_metadata_service():
             return {}
         api_version = self.get_metadata_api_version()
+        redact = AWS_TOKEN_REDACT
         crawled_metadata = {}
         if self.cloud_name == CloudNames.AWS:
             exc_cb = self._refresh_stale_aws_token_cb
@@ -429,14 +437,17 @@ class DataSourceEc2(sources.DataSource):
         try:
             crawled_metadata['user-data'] = ec2.get_instance_userdata(
                 api_version, self.metadata_address,
-                headers_cb=self._get_headers, exception_cb=exc_cb_ud)
+                headers_cb=self._get_headers, headers_redact=redact,
+                exception_cb=exc_cb_ud)
             crawled_metadata['meta-data'] = ec2.get_instance_metadata(
                 api_version, self.metadata_address,
-                headers_cb=self._get_headers, exception_cb=exc_cb)
+                headers_cb=self._get_headers, headers_redact=redact,
+                exception_cb=exc_cb)
             if self.cloud_name == CloudNames.AWS:
                 identity = ec2.get_instance_identity(
                     api_version, self.metadata_address,
-                    headers_cb=self._get_headers, exception_cb=exc_cb)
+                    headers_cb=self._get_headers, headers_redact=redact,
+                    exception_cb=exc_cb)
                 crawled_metadata['dynamic'] = {'instance-identity': identity}
         except Exception:
             util.logexc(
@@ -455,11 +466,12 @@ class DataSourceEc2(sources.DataSource):
         if self.cloud_name != CloudNames.AWS:
             return None
         LOG.debug("Refreshing Ec2 metadata API token")
-        request_header = {'X-aws-ec2-metadata-token-ttl-seconds': seconds}
+        request_header = {AWS_TOKEN_REQ_HEADER: seconds}
         token_url = '{}/{}'.format(self.metadata_address, API_TOKEN_ROUTE)
         try:
-            response = uhelp.readurl(
-                token_url, headers=request_header, request_method="PUT")
+            response = uhelp.readurl(token_url, headers=request_header,
+                                     headers_redact=AWS_TOKEN_REDACT,
+                                     request_method="PUT")
         except uhelp.UrlError as e:
             LOG.warning(
                 'Unable to get API token: %s raised exception %s',
@@ -500,8 +512,7 @@ class DataSourceEc2(sources.DataSource):
                                                  API_TOKEN_DISABLED):
             return {}
         # Request a 6 hour token if URL is API_TOKEN_ROUTE
-        request_token_header = {
-            'X-aws-ec2-metadata-token-ttl-seconds': AWS_TOKEN_TTL_SECONDS}
+        request_token_header = {AWS_TOKEN_REQ_HEADER: AWS_TOKEN_TTL_SECONDS}
         if API_TOKEN_ROUTE in url:
             return request_token_header
         if not self._api_token:
@@ -511,7 +522,7 @@ class DataSourceEc2(sources.DataSource):
             self._api_token = self._refresh_api_token()
             if not self._api_token:
                 return {}
-        return {'X-aws-ec2-metadata-token': self._api_token}
+        return {AWS_TOKEN_PUT_HEADER: self._api_token}
 
 
 class DataSourceEc2Local(DataSourceEc2):
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
index 1496a47..3e7de9f 100644
--- a/cloudinit/url_helper.py
+++ b/cloudinit/url_helper.py
@@ -8,6 +8,7 @@
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import copy
 import json
 import os
 import requests
@@ -41,6 +42,7 @@ else:
 SSL_ENABLED = False
 CONFIG_ENABLED = False  # This was added in 0.7 (but taken out in >=1.0)
 _REQ_VER = None
+REDACTED = 'REDACTED'
 try:
     from distutils.version import LooseVersion
     import pkg_resources
@@ -199,9 +201,9 @@ 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,
+            headers=None, headers_cb=None, headers_redact=None,
+            ssl_details=None, check_status=True, allow_redirects=True,
+            exception_cb=None, session=None, infinite=False, log_req_resp=True,
             request_method=None):
     """Wrapper around requests.Session to read the url and retry if necessary
 
@@ -217,6 +219,7 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
     :param headers: Optional dict of headers to send during request
     :param headers_cb: Optional callable returning a dict of values to send as
         headers during request
+    :param headers_redact: Optional list of header names to redact from the log
     :param ssl_details: Optional dict providing key_file, ca_certs, and
         cert_file keys for use on in ssl connections.
     :param check_status: Optional boolean set True to raise when HTTPError
@@ -243,6 +246,8 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
     req_args['method'] = request_method
     if timeout is not None:
         req_args['timeout'] = max(float(timeout), 0)
+    if headers_redact is None:
+        headers_redact = []
     # 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...
@@ -287,6 +292,12 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
             if k == 'data':
                 continue
             filtered_req_args[k] = v
+            if k == 'headers':
+                for hkey, _hval in v.items():
+                    if hkey in headers_redact:
+                        filtered_req_args[k][hkey] = (
+                            copy.deepcopy(req_args[k][hkey]))
+                        filtered_req_args[k][hkey] = REDACTED
         try:
 
             if log_req_resp:
@@ -339,8 +350,8 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
     return None  # Should throw before this...
 
 
-def wait_for_url(urls, max_wait=None, timeout=None,
-                 status_cb=None, headers_cb=None, sleep_time=1,
+def wait_for_url(urls, max_wait=None, timeout=None, status_cb=None,
+                 headers_cb=None, headers_redact=None, sleep_time=1,
                  exception_cb=None, sleep_time_cb=None, request_method=None):
     """
     urls:      a list of urls to try
@@ -352,6 +363,7 @@ def wait_for_url(urls, max_wait=None, timeout=None,
     status_cb: call method with string message when a url is not available
     headers_cb: call method with single argument of url to get headers
                 for request.
+    headers_redact: a list of header names to redact from the log
     exception_cb: call method with 2 arguments 'msg' (per status_cb) and
                   'exception', the exception that occurred.
     sleep_time_cb: call method with 2 arguments (response, loop_n) that
@@ -415,8 +427,9 @@ def wait_for_url(urls, max_wait=None, timeout=None,
                     headers = {}
 
                 response = readurl(
-                    url, headers=headers, timeout=timeout,
-                    check_status=False, request_method=request_method)
+                    url, headers=headers, headers_redact=headers_redact,
+                    timeout=timeout, check_status=False,
+                    request_method=request_method)
                 if not response.contents:
                     reason = "empty response [%s]" % (response.code)
                     url_exc = UrlError(ValueError(reason), code=response.code,
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 34a089f..bd5bd4c 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -429,6 +429,23 @@ class TestEc2(test_helpers.HttprettyTestCase):
         self.assertTrue(ds.get_data())
         self.assertFalse(ds.is_classic_instance())
 
+    def test_aws_token_redacted(self):
+        """Verify that aws tokens are redacted when logged."""
+        ds = self._setup_ds(
+            platform_data=self.valid_platform_data,
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
+            md={'md': DEFAULT_METADATA})
+        self.assertTrue(ds.get_data())
+        all_logs = self.logs.getvalue().splitlines()
+        REDACT_TTL = "'X-aws-ec2-metadata-token-ttl-seconds': 'REDACTED'"
+        REDACT_TOK = "'X-aws-ec2-metadata-token': 'REDACTED'"
+        logs_with_redacted_ttl = [log for log in all_logs if REDACT_TTL in log]
+        logs_with_redacted = [log for log in all_logs if REDACT_TOK in log]
+        logs_with_token = [log for log in all_logs if 'API-TOKEN' in log]
+        self.assertEqual(1, len(logs_with_redacted_ttl))
+        self.assertEqual(79, len(logs_with_redacted))
+        self.assertEqual(0, len(logs_with_token))
+
     @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
     def test_valid_platform_with_strict_true(self, m_dhcp):
         """Valid platform data should return true with strict_id true."""
-- 
1.8.3.1