mirror of
https://github.com/ansible/awx.git
synced 2026-10-29 22:03:20 -02:30
harden notification backends against redirect-based credential leakage (#16673)
* harden notification backends against redirect-based credential leakage * address code review feedback
This commit is contained in:
@@ -97,12 +97,17 @@ class GrafanaBackend(AWXBaseEmailBackend, CustomNotificationBase):
|
|||||||
grafana_headers['Authorization'] = "Bearer {}".format(self.grafana_key)
|
grafana_headers['Authorization'] = "Bearer {}".format(self.grafana_key)
|
||||||
grafana_headers['Content-Type'] = "application/json"
|
grafana_headers['Content-Type'] = "application/json"
|
||||||
r = requests.post(
|
r = requests.post(
|
||||||
"{}/api/annotations".format(m.recipients()[0]), json=grafana_data, headers=grafana_headers, verify=(not self.grafana_no_verify_ssl)
|
"{}/api/annotations".format(m.recipients()[0]),
|
||||||
|
json=grafana_data,
|
||||||
|
headers=grafana_headers,
|
||||||
|
verify=(not self.grafana_no_verify_ssl),
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
if r.status_code >= 400:
|
if r.status_code < 200 or r.status_code >= 300:
|
||||||
logger.error(smart_str(_("Error sending notification grafana: {}").format(r.status_code)))
|
logger.error(smart_str(_("Error sending notification grafana: {}").format(r.status_code)))
|
||||||
if not self.fail_silently:
|
if not self.fail_silently:
|
||||||
raise Exception(smart_str(_("Error sending notification grafana: {}").format(r.status_code)))
|
raise Exception(smart_str(_("Error sending notification grafana: {}").format(r.status_code)))
|
||||||
|
continue
|
||||||
sent_messages += 1
|
sent_messages += 1
|
||||||
return sent_messages
|
return sent_messages
|
||||||
|
|||||||
@@ -41,10 +41,11 @@ class MattermostBackend(AWXBaseEmailBackend, CustomNotificationBase):
|
|||||||
|
|
||||||
payload['text'] = m.subject
|
payload['text'] = m.subject
|
||||||
|
|
||||||
r = requests.post("{}".format(m.recipients()[0]), json=payload, verify=(not self.mattermost_no_verify_ssl))
|
r = requests.post("{}".format(m.recipients()[0]), json=payload, verify=(not self.mattermost_no_verify_ssl), allow_redirects=False)
|
||||||
if r.status_code >= 400:
|
if r.status_code < 200 or r.status_code >= 300:
|
||||||
logger.error(smart_str(_("Error sending notification mattermost: {}").format(r.status_code)))
|
logger.error(smart_str(_("Error sending notification mattermost: {}").format(r.status_code)))
|
||||||
if not self.fail_silently:
|
if not self.fail_silently:
|
||||||
raise Exception(smart_str(_("Error sending notification mattermost: {}").format(r.status_code)))
|
raise Exception(smart_str(_("Error sending notification mattermost: {}").format(r.status_code)))
|
||||||
|
continue
|
||||||
sent_messages += 1
|
sent_messages += 1
|
||||||
return sent_messages
|
return sent_messages
|
||||||
|
|||||||
@@ -39,12 +39,17 @@ class RocketChatBackend(AWXBaseEmailBackend, CustomNotificationBase):
|
|||||||
payload[optval] = optvalue.strip()
|
payload[optval] = optvalue.strip()
|
||||||
|
|
||||||
r = requests.post(
|
r = requests.post(
|
||||||
"{}".format(m.recipients()[0]), data=json.dumps(payload), headers=get_awx_http_client_headers(), verify=(not self.rocketchat_no_verify_ssl)
|
"{}".format(m.recipients()[0]),
|
||||||
|
data=json.dumps(payload),
|
||||||
|
headers=get_awx_http_client_headers(),
|
||||||
|
verify=(not self.rocketchat_no_verify_ssl),
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
if r.status_code >= 400:
|
if r.status_code < 200 or r.status_code >= 300:
|
||||||
logger.error(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code)))
|
logger.error(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code)))
|
||||||
if not self.fail_silently:
|
if not self.fail_silently:
|
||||||
raise Exception(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code)))
|
raise Exception(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code)))
|
||||||
|
continue
|
||||||
sent_messages += 1
|
sent_messages += 1
|
||||||
return sent_messages
|
return sent_messages
|
||||||
|
|||||||
@@ -4,6 +4,8 @@
|
|||||||
import base64
|
import base64
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
|
from urllib.parse import urlparse
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
|
|
||||||
from awx.main.notifications.base import AWXBaseEmailBackend
|
from awx.main.notifications.base import AWXBaseEmailBackend
|
||||||
@@ -70,6 +72,14 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase):
|
|||||||
|
|
||||||
err = None
|
err = None
|
||||||
|
|
||||||
|
def _origin(value):
|
||||||
|
parsed = urlparse(value)
|
||||||
|
scheme = parsed.scheme.lower()
|
||||||
|
port = parsed.port or {"http": 80, "https": 443}.get(scheme)
|
||||||
|
return scheme, parsed.hostname, port
|
||||||
|
|
||||||
|
original_origin = _origin(url)
|
||||||
|
|
||||||
for retries in range(self.MAX_RETRIES):
|
for retries in range(self.MAX_RETRIES):
|
||||||
# Sometimes we hit redirect URLs. We must account for this. We still extract the redirect URL from the response headers and try again. Max retires == 5
|
# Sometimes we hit redirect URLs. We must account for this. We still extract the redirect URL from the response headers and try again. Max retires == 5
|
||||||
resp = chosen_method(
|
resp = chosen_method(
|
||||||
@@ -101,6 +111,11 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase):
|
|||||||
if url is None:
|
if url is None:
|
||||||
err = f"Webhook notification received redirect to a blank URL from {url_log_safe}. Response headers={resp.headers}"
|
err = f"Webhook notification received redirect to a blank URL from {url_log_safe}. Response headers={resp.headers}"
|
||||||
break
|
break
|
||||||
|
|
||||||
|
if _origin(url) != original_origin:
|
||||||
|
logger.warning("Redirect changed origin; stripping credentials")
|
||||||
|
auth = None
|
||||||
|
headers = get_awx_http_client_headers()
|
||||||
else:
|
else:
|
||||||
# no break condition in the loop encountered; therefore we have hit the maximum number of retries
|
# no break condition in the loop encountered; therefore we have hit the maximum number of retries
|
||||||
err = f"Webhook notification max number of retries [{self.MAX_RETRIES}] exceeded. Failed to send webhook notification to {url_log_safe}"
|
err = f"Webhook notification max number of retries [{self.MAX_RETRIES}] exceeded. Failed to send webhook notification to {url_log_safe}"
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ def test_send_messages():
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -62,6 +63,7 @@ def test_send_messages_with_no_verify_ssl():
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
||||||
verify=False,
|
verify=False,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -93,6 +95,7 @@ def test_send_messages_with_dashboardid(dashboardId):
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000, 'dashboardId': dashboardId},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000, 'dashboardId': dashboardId},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -124,6 +127,7 @@ def test_send_messages_with_panelid(panelId):
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': int(panelId), 'time': 60000},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': int(panelId), 'time': 60000},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -154,6 +158,7 @@ def test_send_messages_with_bothids():
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': 42, 'time': 60000, 'dashboardId': 42},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': 42, 'time': 60000, 'dashboardId': 42},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -184,6 +189,7 @@ def test_send_messages_with_emptyids():
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -214,5 +220,6 @@ def test_send_messages_with_tags():
|
|||||||
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'},
|
||||||
json={'tags': ['ansible'], 'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
json={'tags': ['ansible'], 'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ def test_send_messages():
|
|||||||
data='{"text": "test subject"}',
|
data='{"text": "test subject"}',
|
||||||
headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'},
|
headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'},
|
||||||
verify=True,
|
verify=True,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|
||||||
@@ -118,5 +119,6 @@ def test_send_messages_with_no_verify_ssl():
|
|||||||
data='{"text": "test subject"}',
|
data='{"text": "test subject"}',
|
||||||
headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'},
|
headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'},
|
||||||
verify=False,
|
verify=False,
|
||||||
|
allow_redirects=False,
|
||||||
)
|
)
|
||||||
assert sent_messages == 1
|
assert sent_messages == 1
|
||||||
|
|||||||
@@ -377,3 +377,69 @@ def test_send_messages_with_error_status_code():
|
|||||||
error_call_args = logger_mock.error.call_args[0][0]
|
error_call_args = logger_mock.error.call_args[0][0]
|
||||||
assert "Error sending webhook notification: 404" in error_call_args
|
assert "Error sending webhook notification: 404" in error_call_args
|
||||||
assert sent_messages == 0
|
assert sent_messages == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_send_messages_redirect_strips_credentials_on_host_change():
|
||||||
|
"""Auth and custom headers are stripped when a redirect changes the host."""
|
||||||
|
safe_headers = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}
|
||||||
|
with (
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock,
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock,
|
||||||
|
):
|
||||||
|
requests_mock.post.side_effect = [
|
||||||
|
mock.Mock(status_code=301, headers={"Location": "http://other-host.com/hook"}),
|
||||||
|
mock.Mock(status_code=200),
|
||||||
|
]
|
||||||
|
version_mock.return_value = safe_headers
|
||||||
|
backend = webhook_backend.WebhookBackend('POST', {'Authorization': 'Bearer secret-token'}, username='user', password='secret')
|
||||||
|
message = EmailMessage('test subject', {'text': 'test body'}, [], ['http://example.com'])
|
||||||
|
sent_messages = backend.send_messages([message])
|
||||||
|
assert requests_mock.post.call_count == 2
|
||||||
|
first_call = requests_mock.post.call_args_list[0]
|
||||||
|
assert first_call.kwargs['auth'] == ('user', 'secret')
|
||||||
|
assert 'Authorization' in first_call.kwargs['headers']
|
||||||
|
second_call = requests_mock.post.call_args_list[1]
|
||||||
|
assert second_call.kwargs['auth'] is None
|
||||||
|
assert second_call.kwargs['headers'] == safe_headers
|
||||||
|
assert sent_messages == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_send_messages_redirect_strips_credentials_on_scheme_downgrade():
|
||||||
|
"""Auth is stripped when a redirect downgrades from HTTPS to HTTP."""
|
||||||
|
safe_headers = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}
|
||||||
|
with (
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock,
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock,
|
||||||
|
):
|
||||||
|
requests_mock.post.side_effect = [
|
||||||
|
mock.Mock(status_code=301, headers={"Location": "http://example.com/hook"}),
|
||||||
|
mock.Mock(status_code=200),
|
||||||
|
]
|
||||||
|
version_mock.return_value = safe_headers
|
||||||
|
backend = webhook_backend.WebhookBackend('POST', None, username='user', password='secret')
|
||||||
|
message = EmailMessage('test subject', {'text': 'test body'}, [], ['https://example.com'])
|
||||||
|
sent_messages = backend.send_messages([message])
|
||||||
|
assert requests_mock.post.call_count == 2
|
||||||
|
second_call = requests_mock.post.call_args_list[1]
|
||||||
|
assert second_call.kwargs['auth'] is None
|
||||||
|
assert sent_messages == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_send_messages_redirect_keeps_auth_on_same_origin():
|
||||||
|
"""Auth credentials are preserved when a redirect stays on the same origin."""
|
||||||
|
with (
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock,
|
||||||
|
mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock,
|
||||||
|
):
|
||||||
|
requests_mock.post.side_effect = [
|
||||||
|
mock.Mock(status_code=301, headers={"Location": "http://example.com/new-path"}),
|
||||||
|
mock.Mock(status_code=200),
|
||||||
|
]
|
||||||
|
version_mock.return_value = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}
|
||||||
|
backend = webhook_backend.WebhookBackend('POST', None, username='user', password='secret')
|
||||||
|
message = EmailMessage('test subject', {'text': 'test body'}, [], ['http://example.com'])
|
||||||
|
sent_messages = backend.send_messages([message])
|
||||||
|
assert requests_mock.post.call_count == 2
|
||||||
|
for call in requests_mock.post.call_args_list:
|
||||||
|
assert call.kwargs['auth'] == ('user', 'secret')
|
||||||
|
assert sent_messages == 1
|
||||||
|
|||||||
Reference in New Issue
Block a user