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