5b08af
From 2a79d9ca066648feaa29e16e0ab6c2607907352e Mon Sep 17 00:00:00 2001
5b08af
From: Eduardo Otubo <otubo@redhat.com>
5b08af
Date: Wed, 20 May 2020 12:44:07 +0200
5b08af
Subject: [PATCH] ec2: only redact token request headers in logs, avoid
5b08af
 altering request (#230)
5b08af
5b08af
RH-Author: Eduardo Otubo <otubo@redhat.com>
5b08af
Message-id: <20200519110500.21088-1-otubo@redhat.com>
5b08af
Patchwork-id: 96614
5b08af
O-Subject: [RHEL-7.9 cloud-init PATCH] ec2: only redact token request headers in logs, avoid altering request (#230)
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
commit fa1abfec27050a4fb71cad950a17e42f9b43b478
5b08af
Author: Chad Smith <chad.smith@canonical.com>
5b08af
Date:   Tue Mar 3 15:23:33 2020 -0700
5b08af
5b08af
    ec2: only redact token request headers in logs, avoid altering request (#230)
5b08af
5b08af
    Our header redact logic was redacting both logged request headers and
5b08af
    the actual source request. This results in DataSourceEc2 sending the
5b08af
    invalid header "X-aws-ec2-metadata-token-ttl-seconds: REDACTED" which
5b08af
    gets an HTTP status response of 400.
5b08af
5b08af
    Cloud-init retries this failed token request for 2 minutes before
5b08af
    falling back to IMDSv1.
5b08af
5b08af
    LP: #1865882
5b08af
5b08af
Signed-off-by: Eduardo Otubo <otubo@redhat.com>
5b08af
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
5b08af
---
5b08af
 cloudinit/tests/test_url_helper.py | 34 +++++++++++++++++++++++++++++++++-
5b08af
 cloudinit/url_helper.py            | 15 ++++++++-------
5b08af
 2 files changed, 41 insertions(+), 8 deletions(-)
5b08af
5b08af
diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py
5b08af
index 1674120..29b3937 100644
5b08af
--- a/cloudinit/tests/test_url_helper.py
5b08af
+++ b/cloudinit/tests/test_url_helper.py
5b08af
@@ -1,7 +1,8 @@
5b08af
 # This file is part of cloud-init. See LICENSE file for license information.
5b08af
 
5b08af
 from cloudinit.url_helper import (
5b08af
-    NOT_FOUND, UrlError, oauth_headers, read_file_or_url, retry_on_url_exc)
5b08af
+    NOT_FOUND, UrlError, REDACTED, oauth_headers, read_file_or_url,
5b08af
+    retry_on_url_exc)
5b08af
 from cloudinit.tests.helpers import CiTestCase, mock, skipIf
5b08af
 from cloudinit import util
5b08af
 from cloudinit import version
5b08af
@@ -50,6 +51,9 @@ class TestOAuthHeaders(CiTestCase):
5b08af
 
5b08af
 
5b08af
 class TestReadFileOrUrl(CiTestCase):
5b08af
+
5b08af
+    with_logs = True
5b08af
+
5b08af
     def test_read_file_or_url_str_from_file(self):
5b08af
         """Test that str(result.contents) on file is text version of contents.
5b08af
         It should not be "b'data'", but just "'data'" """
5b08af
@@ -71,6 +75,34 @@ class TestReadFileOrUrl(CiTestCase):
5b08af
         self.assertEqual(result.contents, data)
5b08af
         self.assertEqual(str(result), data.decode('utf-8'))
5b08af
 
5b08af
+    @httpretty.activate
5b08af
+    def test_read_file_or_url_str_from_url_redacting_headers_from_logs(self):
5b08af
+        """Headers are redacted from logs but unredacted in requests."""
5b08af
+        url = 'http://hostname/path'
5b08af
+        headers = {'sensitive': 'sekret', 'server': 'blah'}
5b08af
+        httpretty.register_uri(httpretty.GET, url)
5b08af
+
5b08af
+        read_file_or_url(url, headers=headers, headers_redact=['sensitive'])
5b08af
+        logs = self.logs.getvalue()
5b08af
+        for k in headers.keys():
5b08af
+            self.assertEqual(headers[k],  httpretty.last_request().headers[k])
5b08af
+        self.assertIn(REDACTED, logs)
5b08af
+        self.assertNotIn('sekret', logs)
5b08af
+
5b08af
+    @httpretty.activate
5b08af
+    def test_read_file_or_url_str_from_url_redacts_noheaders(self):
5b08af
+        """When no headers_redact, header values are in logs and requests."""
5b08af
+        url = 'http://hostname/path'
5b08af
+        headers = {'sensitive': 'sekret', 'server': 'blah'}
5b08af
+        httpretty.register_uri(httpretty.GET, url)
5b08af
+
5b08af
+        read_file_or_url(url, headers=headers)
5b08af
+        for k in headers.keys():
5b08af
+            self.assertEqual(headers[k], httpretty.last_request().headers[k])
5b08af
+        logs = self.logs.getvalue()
5b08af
+        self.assertNotIn(REDACTED, logs)
5b08af
+        self.assertIn('sekret', logs)
5b08af
+
5b08af
     @mock.patch(M_PATH + 'readurl')
5b08af
     def test_read_file_or_url_passes_params_to_readurl(self, m_readurl):
5b08af
         """read_file_or_url passes all params through to readurl."""
5b08af
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
5b08af
index 3e7de9f..e6188ea 100644
5b08af
--- a/cloudinit/url_helper.py
5b08af
+++ b/cloudinit/url_helper.py
5b08af
@@ -291,13 +291,14 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
5b08af
         for (k, v) in req_args.items():
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
+            if k == 'headers' and headers_redact:
5b08af
+                matched_headers = [k for k in headers_redact if v.get(k)]
5b08af
+                if matched_headers:
5b08af
+                    filtered_req_args[k] = copy.deepcopy(v)
5b08af
+                    for key in matched_headers:
5b08af
+                        filtered_req_args[k][key] = REDACTED
5b08af
+            else:
5b08af
+                filtered_req_args[k] = v
5b08af
         try:
5b08af
 
5b08af
             if log_req_resp:
5b08af
-- 
5b08af
1.8.3.1
5b08af