5b08af
From 0bd7f6b5f393a88b45ced71f1645705b651de9f2 Mon Sep 17 00:00:00 2001
5b08af
From: Eduardo Otubo <otubo@redhat.com>
5b08af
Date: Mon, 11 May 2020 09:24:29 +0200
5b08af
Subject: [PATCH 2/2] ec2: Do not log IMDSv2 token values, instead use REDACTED
5b08af
 (#219)
5b08af
5b08af
RH-Author: Eduardo Otubo <otubo@redhat.com>
5b08af
Message-id: <20200505082940.18316-1-otubo@redhat.com>
5b08af
Patchwork-id: 96264
5b08af
O-Subject: [RHEL-7.9/RHEL-8.3 cloud-init PATCH] ec2: Do not log IMDSv2 token values, instead use REDACTED (#219)
5b08af
Bugzilla: 1821999
5b08af
RH-Acked-by: Cathy Avery <cavery@redhat.com>
5b08af
RH-Acked-by: Mohammed Gamal <mgamal@redhat.com>
5b08af
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
5b08af
5b08af
Note: There's no RHEL-8.3/cloud-init-19.4 branch yet, but it should be
5b08af
queued to be applied on top of it when it's created.
5b08af
5b08af
commit 87cd040ed8fe7195cbb357ed3bbf53cd2a81436c
5b08af
Author: Ryan Harper <ryan.harper@canonical.com>
5b08af
Date:   Wed Feb 19 15:01:09 2020 -0600
5b08af
5b08af
    ec2: Do not log IMDSv2 token values, instead use REDACTED (#219)
5b08af
5b08af
    Instead of logging the token values used log the headers and replace the actual
5b08af
    values with the string 'REDACTED'.  This allows users to examine cloud-init.log
5b08af
    and see that the IMDSv2 token header is being used but avoids leaving the value
5b08af
    used in the log file itself.
5b08af
5b08af
    LP: #1863943
5b08af
5b08af
Signed-off-by: Eduardo Otubo <otubo@redhat.com>
5b08af
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
5b08af
---
5b08af
 cloudinit/ec2_utils.py                      | 12 ++++++++--
5b08af
 cloudinit/sources/DataSourceEc2.py          | 35 +++++++++++++++++++----------
5b08af
 cloudinit/url_helper.py                     | 27 ++++++++++++++++------
5b08af
 tests/unittests/test_datasource/test_ec2.py | 17 ++++++++++++++
5b08af
 4 files changed, 70 insertions(+), 21 deletions(-)
5b08af
5b08af
diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
5b08af
index 57708c1..34acfe8 100644
5b08af
--- a/cloudinit/ec2_utils.py
5b08af
+++ b/cloudinit/ec2_utils.py
5b08af
@@ -142,7 +142,8 @@ def skip_retry_on_codes(status_codes, _request_args, cause):
5b08af
 def get_instance_userdata(api_version='latest',
5b08af
                           metadata_address='http://169.254.169.254',
5b08af
                           ssl_details=None, timeout=5, retries=5,
5b08af
-                          headers_cb=None, exception_cb=None):
5b08af
+                          headers_cb=None, headers_redact=None,
5b08af
+                          exception_cb=None):
5b08af
     ud_url = url_helper.combine_url(metadata_address, api_version)
5b08af
     ud_url = url_helper.combine_url(ud_url, 'user-data')
5b08af
     user_data = ''
5b08af
@@ -155,7 +156,8 @@ def get_instance_userdata(api_version='latest',
5b08af
                                              SKIP_USERDATA_CODES)
5b08af
         response = url_helper.read_file_or_url(
5b08af
             ud_url, ssl_details=ssl_details, timeout=timeout,
5b08af
-            retries=retries, exception_cb=exception_cb, headers_cb=headers_cb)
5b08af
+            retries=retries, exception_cb=exception_cb, headers_cb=headers_cb,
5b08af
+            headers_redact=headers_redact)
5b08af
         user_data = response.contents
5b08af
     except url_helper.UrlError as e:
5b08af
         if e.code not in SKIP_USERDATA_CODES:
5b08af
@@ -169,11 +171,13 @@ def _get_instance_metadata(tree, api_version='latest',
5b08af
                            metadata_address='http://169.254.169.254',
5b08af
                            ssl_details=None, timeout=5, retries=5,
5b08af
                            leaf_decoder=None, headers_cb=None,
5b08af
+                           headers_redact=None,
5b08af
                            exception_cb=None):
5b08af
     md_url = url_helper.combine_url(metadata_address, api_version, tree)
5b08af
     caller = functools.partial(
5b08af
         url_helper.read_file_or_url, ssl_details=ssl_details,
5b08af
         timeout=timeout, retries=retries, headers_cb=headers_cb,
5b08af
+        headers_redact=headers_redact,
5b08af
         exception_cb=exception_cb)
5b08af
 
5b08af
     def mcaller(url):
5b08af
@@ -197,6 +201,7 @@ def get_instance_metadata(api_version='latest',
5b08af
                           metadata_address='http://169.254.169.254',
5b08af
                           ssl_details=None, timeout=5, retries=5,
5b08af
                           leaf_decoder=None, headers_cb=None,
5b08af
+                          headers_redact=None,
5b08af
                           exception_cb=None):
5b08af
     # Note, 'meta-data' explicitly has trailing /.
5b08af
     # this is required for CloudStack (LP: #1356855)
5b08af
@@ -204,6 +209,7 @@ def get_instance_metadata(api_version='latest',
5b08af
                                   metadata_address=metadata_address,
5b08af
                                   ssl_details=ssl_details, timeout=timeout,
5b08af
                                   retries=retries, leaf_decoder=leaf_decoder,
5b08af
+                                  headers_redact=headers_redact,
5b08af
                                   headers_cb=headers_cb,
5b08af
                                   exception_cb=exception_cb)
5b08af
 
5b08af
@@ -212,12 +218,14 @@ def get_instance_identity(api_version='latest',
5b08af
                           metadata_address='http://169.254.169.254',
5b08af
                           ssl_details=None, timeout=5, retries=5,
5b08af
                           leaf_decoder=None, headers_cb=None,
5b08af
+                          headers_redact=None,
5b08af
                           exception_cb=None):
5b08af
     return _get_instance_metadata(tree='dynamic/instance-identity',
5b08af
                                   api_version=api_version,
5b08af
                                   metadata_address=metadata_address,
5b08af
                                   ssl_details=ssl_details, timeout=timeout,
5b08af
                                   retries=retries, leaf_decoder=leaf_decoder,
5b08af
+                                  headers_redact=headers_redact,
5b08af
                                   headers_cb=headers_cb,
5b08af
                                   exception_cb=exception_cb)
5b08af
 # vi: ts=4 expandtab
5b08af
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
5b08af
index b9f346a..0f2bfef 100644
5b08af
--- a/cloudinit/sources/DataSourceEc2.py
5b08af
+++ b/cloudinit/sources/DataSourceEc2.py
5b08af
@@ -31,6 +31,9 @@ STRICT_ID_DEFAULT = "warn"
5b08af
 API_TOKEN_ROUTE = 'latest/api/token'
5b08af
 API_TOKEN_DISABLED = '_ec2_disable_api_token'
5b08af
 AWS_TOKEN_TTL_SECONDS = '21600'
5b08af
+AWS_TOKEN_PUT_HEADER = 'X-aws-ec2-metadata-token'
5b08af
+AWS_TOKEN_REQ_HEADER = AWS_TOKEN_PUT_HEADER + '-ttl-seconds'
5b08af
+AWS_TOKEN_REDACT = [AWS_TOKEN_PUT_HEADER, AWS_TOKEN_REQ_HEADER]
5b08af
 
5b08af
 
5b08af
 class CloudNames(object):
5b08af
@@ -158,7 +161,8 @@ class DataSourceEc2(sources.DataSource):
5b08af
         for api_ver in self.extended_metadata_versions:
5b08af
             url = url_tmpl.format(self.metadata_address, api_ver)
5b08af
             try:
5b08af
-                resp = uhelp.readurl(url=url, headers=headers)
5b08af
+                resp = uhelp.readurl(url=url, headers=headers,
5b08af
+                                     headers_redact=AWS_TOKEN_REDACT)
5b08af
             except uhelp.UrlError as e:
5b08af
                 LOG.debug('url %s raised exception %s', url, e)
5b08af
             else:
5b08af
@@ -180,6 +184,7 @@ class DataSourceEc2(sources.DataSource):
5b08af
                 self.identity = ec2.get_instance_identity(
5b08af
                     api_version, self.metadata_address,
5b08af
                     headers_cb=self._get_headers,
5b08af
+                    headers_redact=AWS_TOKEN_REDACT,
5b08af
                     exception_cb=self._refresh_stale_aws_token_cb).get(
5b08af
                         'document', {})
5b08af
             return self.identity.get(
5b08af
@@ -205,7 +210,8 @@ class DataSourceEc2(sources.DataSource):
5b08af
         LOG.debug('Fetching Ec2 IMDSv2 API Token')
5b08af
         url, response = uhelp.wait_for_url(
5b08af
             urls=urls, max_wait=1, timeout=1, status_cb=self._status_cb,
5b08af
-            headers_cb=self._get_headers, request_method=request_method)
5b08af
+            headers_cb=self._get_headers, request_method=request_method,
5b08af
+            headers_redact=AWS_TOKEN_REDACT)
5b08af
 
5b08af
         if url and response:
5b08af
             self._api_token = response
5b08af
@@ -252,7 +258,8 @@ class DataSourceEc2(sources.DataSource):
5b08af
             url, _ = uhelp.wait_for_url(
5b08af
                 urls=urls, max_wait=url_params.max_wait_seconds,
5b08af
                 timeout=url_params.timeout_seconds, status_cb=LOG.warning,
5b08af
-                headers_cb=self._get_headers, request_method=request_method)
5b08af
+                headers_redact=AWS_TOKEN_REDACT, headers_cb=self._get_headers,
5b08af
+                request_method=request_method)
5b08af
 
5b08af
             if url:
5b08af
                 metadata_address = url2base[url]
5b08af
@@ -420,6 +427,7 @@ class DataSourceEc2(sources.DataSource):
5b08af
         if not self.wait_for_metadata_service():
5b08af
             return {}
5b08af
         api_version = self.get_metadata_api_version()
5b08af
+        redact = AWS_TOKEN_REDACT
5b08af
         crawled_metadata = {}
5b08af
         if self.cloud_name == CloudNames.AWS:
5b08af
             exc_cb = self._refresh_stale_aws_token_cb
5b08af
@@ -429,14 +437,17 @@ class DataSourceEc2(sources.DataSource):
5b08af
         try:
5b08af
             crawled_metadata['user-data'] = ec2.get_instance_userdata(
5b08af
                 api_version, self.metadata_address,
5b08af
-                headers_cb=self._get_headers, exception_cb=exc_cb_ud)
5b08af
+                headers_cb=self._get_headers, headers_redact=redact,
5b08af
+                exception_cb=exc_cb_ud)
5b08af
             crawled_metadata['meta-data'] = ec2.get_instance_metadata(
5b08af
                 api_version, self.metadata_address,
5b08af
-                headers_cb=self._get_headers, exception_cb=exc_cb)
5b08af
+                headers_cb=self._get_headers, headers_redact=redact,
5b08af
+                exception_cb=exc_cb)
5b08af
             if self.cloud_name == CloudNames.AWS:
5b08af
                 identity = ec2.get_instance_identity(
5b08af
                     api_version, self.metadata_address,
5b08af
-                    headers_cb=self._get_headers, exception_cb=exc_cb)
5b08af
+                    headers_cb=self._get_headers, headers_redact=redact,
5b08af
+                    exception_cb=exc_cb)
5b08af
                 crawled_metadata['dynamic'] = {'instance-identity': identity}
5b08af
         except Exception:
5b08af
             util.logexc(
5b08af
@@ -455,11 +466,12 @@ class DataSourceEc2(sources.DataSource):
5b08af
         if self.cloud_name != CloudNames.AWS:
5b08af
             return None
5b08af
         LOG.debug("Refreshing Ec2 metadata API token")
5b08af
-        request_header = {'X-aws-ec2-metadata-token-ttl-seconds': seconds}
5b08af
+        request_header = {AWS_TOKEN_REQ_HEADER: seconds}
5b08af
         token_url = '{}/{}'.format(self.metadata_address, API_TOKEN_ROUTE)
5b08af
         try:
5b08af
-            response = uhelp.readurl(
5b08af
-                token_url, headers=request_header, request_method="PUT")
5b08af
+            response = uhelp.readurl(token_url, headers=request_header,
5b08af
+                                     headers_redact=AWS_TOKEN_REDACT,
5b08af
+                                     request_method="PUT")
5b08af
         except uhelp.UrlError as e:
5b08af
             LOG.warning(
5b08af
                 'Unable to get API token: %s raised exception %s',
5b08af
@@ -500,8 +512,7 @@ class DataSourceEc2(sources.DataSource):
5b08af
                                                  API_TOKEN_DISABLED):
5b08af
             return {}
5b08af
         # Request a 6 hour token if URL is API_TOKEN_ROUTE
5b08af
-        request_token_header = {
5b08af
-            'X-aws-ec2-metadata-token-ttl-seconds': AWS_TOKEN_TTL_SECONDS}
5b08af
+        request_token_header = {AWS_TOKEN_REQ_HEADER: AWS_TOKEN_TTL_SECONDS}
5b08af
         if API_TOKEN_ROUTE in url:
5b08af
             return request_token_header
5b08af
         if not self._api_token:
5b08af
@@ -511,7 +522,7 @@ class DataSourceEc2(sources.DataSource):
5b08af
             self._api_token = self._refresh_api_token()
5b08af
             if not self._api_token:
5b08af
                 return {}
5b08af
-        return {'X-aws-ec2-metadata-token': self._api_token}
5b08af
+        return {AWS_TOKEN_PUT_HEADER: self._api_token}
5b08af
 
5b08af
 
5b08af
 class DataSourceEc2Local(DataSourceEc2):
5b08af
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
5b08af
index 1496a47..3e7de9f 100644
5b08af
--- a/cloudinit/url_helper.py
5b08af
+++ b/cloudinit/url_helper.py
5b08af
@@ -8,6 +8,7 @@
5b08af
 #
5b08af
 # This file is part of cloud-init. See LICENSE file for license information.
5b08af
 
5b08af
+import copy
5b08af
 import json
5b08af
 import os
5b08af
 import requests
5b08af
@@ -41,6 +42,7 @@ else:
5b08af
 SSL_ENABLED = False
5b08af
 CONFIG_ENABLED = False  # This was added in 0.7 (but taken out in >=1.0)
5b08af
 _REQ_VER = None
5b08af
+REDACTED = 'REDACTED'
5b08af
 try:
5b08af
     from distutils.version import LooseVersion
5b08af
     import pkg_resources
5b08af
@@ -199,9 +201,9 @@ def _get_ssl_args(url, ssl_details):
5b08af
 
5b08af
 
5b08af
 def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
-            headers=None, headers_cb=None, ssl_details=None,
5b08af
-            check_status=True, allow_redirects=True, exception_cb=None,
5b08af
-            session=None, infinite=False, log_req_resp=True,
5b08af
+            headers=None, headers_cb=None, headers_redact=None,
5b08af
+            ssl_details=None, check_status=True, allow_redirects=True,
5b08af
+            exception_cb=None, session=None, infinite=False, log_req_resp=True,
5b08af
             request_method=None):
5b08af
     """Wrapper around requests.Session to read the url and retry if necessary
5b08af
 
5b08af
@@ -217,6 +219,7 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
     :param headers: Optional dict of headers to send during request
5b08af
     :param headers_cb: Optional callable returning a dict of values to send as
5b08af
         headers during request
5b08af
+    :param headers_redact: Optional list of header names to redact from the log
5b08af
     :param ssl_details: Optional dict providing key_file, ca_certs, and
5b08af
         cert_file keys for use on in ssl connections.
5b08af
     :param check_status: Optional boolean set True to raise when HTTPError
5b08af
@@ -243,6 +246,8 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
     req_args['method'] = request_method
5b08af
     if timeout is not None:
5b08af
         req_args['timeout'] = max(float(timeout), 0)
5b08af
+    if headers_redact is None:
5b08af
+        headers_redact = []
5b08af
     # It doesn't seem like config
5b08af
     # was added in older library versions (or newer ones either), thus we
5b08af
     # need to manually do the retries if it wasn't...
5b08af
@@ -287,6 +292,12 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
             if k == 'data':
5b08af
                 continue
5b08af
             filtered_req_args[k] = v
5b08af
+            if k == 'headers':
5b08af
+                for hkey, _hval in v.items():
5b08af
+                    if hkey in headers_redact:
5b08af
+                        filtered_req_args[k][hkey] = (
5b08af
+                            copy.deepcopy(req_args[k][hkey]))
5b08af
+                        filtered_req_args[k][hkey] = REDACTED
5b08af
         try:
5b08af
 
5b08af
             if log_req_resp:
5b08af
@@ -339,8 +350,8 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
     return None  # Should throw before this...
5b08af
 
5b08af
 
5b08af
-def wait_for_url(urls, max_wait=None, timeout=None,
5b08af
-                 status_cb=None, headers_cb=None, sleep_time=1,
5b08af
+def wait_for_url(urls, max_wait=None, timeout=None, status_cb=None,
5b08af
+                 headers_cb=None, headers_redact=None, sleep_time=1,
5b08af
                  exception_cb=None, sleep_time_cb=None, request_method=None):
5b08af
     """
5b08af
     urls:      a list of urls to try
5b08af
@@ -352,6 +363,7 @@ def wait_for_url(urls, max_wait=None, timeout=None,
5b08af
     status_cb: call method with string message when a url is not available
5b08af
     headers_cb: call method with single argument of url to get headers
5b08af
                 for request.
5b08af
+    headers_redact: a list of header names to redact from the log
5b08af
     exception_cb: call method with 2 arguments 'msg' (per status_cb) and
5b08af
                   'exception', the exception that occurred.
5b08af
     sleep_time_cb: call method with 2 arguments (response, loop_n) that
5b08af
@@ -415,8 +427,9 @@ def wait_for_url(urls, max_wait=None, timeout=None,
5b08af
                     headers = {}
5b08af
 
5b08af
                 response = readurl(
5b08af
-                    url, headers=headers, timeout=timeout,
5b08af
-                    check_status=False, request_method=request_method)
5b08af
+                    url, headers=headers, headers_redact=headers_redact,
5b08af
+                    timeout=timeout, check_status=False,
5b08af
+                    request_method=request_method)
5b08af
                 if not response.contents:
5b08af
                     reason = "empty response [%s]" % (response.code)
5b08af
                     url_exc = UrlError(ValueError(reason), code=response.code,
5b08af
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
5b08af
index 34a089f..bd5bd4c 100644
5b08af
--- a/tests/unittests/test_datasource/test_ec2.py
5b08af
+++ b/tests/unittests/test_datasource/test_ec2.py
5b08af
@@ -429,6 +429,23 @@ class TestEc2(test_helpers.HttprettyTestCase):
5b08af
         self.assertTrue(ds.get_data())
5b08af
         self.assertFalse(ds.is_classic_instance())
5b08af
 
5b08af
+    def test_aws_token_redacted(self):
5b08af
+        """Verify that aws tokens are redacted when logged."""
5b08af
+        ds = self._setup_ds(
5b08af
+            platform_data=self.valid_platform_data,
5b08af
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
5b08af
+            md={'md': DEFAULT_METADATA})
5b08af
+        self.assertTrue(ds.get_data())
5b08af
+        all_logs = self.logs.getvalue().splitlines()
5b08af
+        REDACT_TTL = "'X-aws-ec2-metadata-token-ttl-seconds': 'REDACTED'"
5b08af
+        REDACT_TOK = "'X-aws-ec2-metadata-token': 'REDACTED'"
5b08af
+        logs_with_redacted_ttl = [log for log in all_logs if REDACT_TTL in log]
5b08af
+        logs_with_redacted = [log for log in all_logs if REDACT_TOK in log]
5b08af
+        logs_with_token = [log for log in all_logs if 'API-TOKEN' in log]
5b08af
+        self.assertEqual(1, len(logs_with_redacted_ttl))
5b08af
+        self.assertEqual(79, len(logs_with_redacted))
5b08af
+        self.assertEqual(0, len(logs_with_token))
5b08af
+
5b08af
     @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
5b08af
     def test_valid_platform_with_strict_true(self, m_dhcp):
5b08af
         """Valid platform data should return true with strict_id true."""
5b08af
-- 
5b08af
1.8.3.1
5b08af