Skip to content

Commit

Permalink
Hide sensitive info from logs (demisto#33224)
Browse files Browse the repository at this point in the history
* Revert "Sanitize Curl Logs (demisto#31702)"

This reverts commit 60a9393.

* init

* more

* try

* try

* fixes

* add tests

* more

* RN

* RN

* fix

* pre commit

* fix tests

* fix

* fixes

* fixes

* add test

* pre commit

* python 2

* Python 2 #2

* Update Packs/Base/Scripts/CommonServerPython/CommonServerPython_test.py

Co-authored-by: Jasmine Beilin <71636766+JasBeilin@users.noreply.github.com>

* add description

* pre commit

---------

Co-authored-by: Jasmine Beilin <71636766+JasBeilin@users.noreply.github.com>
  • Loading branch information
RosenbergYehuda and JasBeilin authored Mar 12, 2024
1 parent 3e76ae9 commit 20dfbbc
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 20 deletions.
6 changes: 6 additions & 0 deletions Packs/Base/ReleaseNotes/1_33_39.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

#### Scripts

##### CommonServerPython

Improved implementation to prevent sensitive information from being logged.
63 changes: 50 additions & 13 deletions Packs/Base/Scripts/CommonServerPython/CommonServerPython.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ def __line__():
ASSETS = "assets"
EVENTS = "events"
DATA_TYPES = [EVENTS, ASSETS]

SECRET_REPLACEMENT_STRING = '<XX_REPLACED>'
MASK = '<XX_REPLACED>'
SEND_PREFIX = "send: b'"


def register_module_line(module_name, start_end, line, wrapper=0):
Expand Down Expand Up @@ -1632,7 +1632,7 @@ def encode(self, message):
else:
res = "Failed encoding message with error: {}".format(exception)
for s in self.replace_strs:
res = res.replace(s, SECRET_REPLACEMENT_STRING)
res = res.replace(s, MASK)
return res

def __call__(self, message):
Expand Down Expand Up @@ -1701,7 +1701,7 @@ def build_curl(self, text):
:rtype: ``None``
"""
http_methods = ['GET', 'POST', 'PUT', 'DELETE', 'PATCH']
data = text.split("send: b'")[1]
data = text.split(SEND_PREFIX)[1]
if data and data[0] in {'{', '<'}:
# it is the request url query params/post body - will always come after we already have the url and headers
# `<` is for xml body
Expand All @@ -1711,7 +1711,6 @@ def build_curl(self, text):
url = ''
headers = []
headers_to_skip = ['Content-Length', 'User-Agent', 'Accept-Encoding', 'Connection']
headers_to_sanitize = ['Authorization', 'Cookie']
request_parts = repr(data).split('\\\\r\\\\n') # splitting lines on repr since data is a bytes-string
for line, part in enumerate(request_parts):
if line == 0:
Expand All @@ -1723,9 +1722,6 @@ def build_curl(self, text):
else:
if any(header_to_skip in part for header_to_skip in headers_to_skip):
continue
if any(header_to_sanitize in part for header_to_sanitize in headers_to_sanitize):
headers.append(part.split(' ')[0] + " " + SECRET_REPLACEMENT_STRING)
continue
headers.append(part)
curl_headers = ''
for header in headers:
Expand Down Expand Up @@ -1757,12 +1753,18 @@ def write(self, msg):
if self.buffering:
self.messages.append(text)
else:
if is_debug_mode():
if text.startswith(('send:', 'header:')):
try:
text = censor_request_logs(text)
except Exception as e: # should fail silently
demisto.debug('Failed censoring request logs - {}'.format(str(e)))
if text.startswith('send:'):
try:
self.build_curl(text)
except Exception as e: # should fail silently
demisto.debug('Failed generating curl - {}'.format(str(e)))
demisto.info(text)
if is_debug_mode() and text.startswith('send:'):
try:
self.build_curl(text)
except Exception as e: # should fail silently
demisto.debug('Failed generating curl - {}'.format(str(e)))
self.write_buf = []

def print_override(self, *args, **kwargs):
Expand Down Expand Up @@ -8387,6 +8389,41 @@ def emit(self, record):
pass


def censor_request_logs(request_log):
"""
Censors the request logs generated from the urllib library directly by replacing sensitive information such as tokens and cookies with a mask.
In most cases, the sensitive value is the first word after the keyword, but in some cases, it is the second one.
:param request_log: The request log to censor
:type request_log: ``str``
:return: The censored request log
:rtype: ``str``
"""
keywords_to_censor = ['Authorization:', 'Cookie', "Token"]
lower_keywords_to_censor = [word.lower() for word in keywords_to_censor]

trimed_request_log = request_log.lstrip(SEND_PREFIX)
request_log_with_spaces = trimed_request_log.replace("\\r\\n", " \\r\\n")
request_log_lst = request_log_with_spaces.split()

for i, word in enumerate(request_log_lst):
# Check if the word is a keyword or contains a keyword (e.g "Cookies" containes "Cookie")
if any(keyword in word.lower() for keyword in lower_keywords_to_censor):
next_word = request_log_lst[i + 1] if i + 1 < len(request_log_lst) else None
if next_word:
# If the next word is "Bearer" or "Basic" then we replace the word after it since thats the token
if next_word.lower() in ["bearer", "basic"] and i + 2 < len(request_log_lst):
request_log_lst[i + 2] = MASK
else:
request_log_lst[i + 1] = MASK

# Rebuild the request log so that the only change is the masked information.
censored_string = SEND_PREFIX + \
' '.join(request_log_lst) if request_log.startswith(SEND_PREFIX) else ' '.join(request_log_lst)
censored_string = censored_string.replace(" \\r\\n", "\\r\\n")
return censored_string


class DebugLogger(object):
"""
Wrapper to initiate logging at logging.DEBUG level.
Expand Down
77 changes: 71 additions & 6 deletions Packs/Base/Scripts/CommonServerPython/CommonServerPython_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
url_to_clickable_markdown, WarningsHandler, DemistoException, SmartGetDict, JsonTransformer, \
remove_duplicates_from_list_arg, DBotScoreType, DBotScoreReliability, Common, send_events_to_xsiam, ExecutionMetrics, \
response_to_context, is_integration_command_execution, is_xsiam_or_xsoar_saas, is_xsoar, is_xsoar_on_prem, \
is_xsoar_hosted, is_xsoar_saas, is_xsiam, send_data_to_xsiam
is_xsoar_hosted, is_xsoar_saas, is_xsiam, send_data_to_xsiam, censor_request_logs, censor_request_logs

EVENTS_LOG_ERROR = \
"""Error sending new events into XSIAM.
Expand Down Expand Up @@ -1452,7 +1452,7 @@ def test_build_curl_post_noproxy():
"Content-Type: application/json\\r\\n\\r\\n'")
ilog.build_curl("send: b'{\"data\": \"value\"}'")
assert ilog.curl == [
'curl -X POST https://demisto.com/api -H "Authorization: <XX_REPLACED>" -H "Content-Type: application/json" '
'curl -X POST https://demisto.com/api -H "Authorization: TOKEN" -H "Content-Type: application/json" '
'--noproxy "*" -d \'{"data": "value"}\''
]

Expand All @@ -1479,7 +1479,7 @@ def test_build_curl_post_xml():
"Content-Type: application/json\\r\\n\\r\\n'")
ilog.build_curl("send: b'<?xml version=\"1.0\" encoding=\"utf-8\"?>'")
assert ilog.curl == [
'curl -X POST https://demisto.com/api -H "Authorization: <XX_REPLACED>" -H "Content-Type: application/json" '
'curl -X POST https://demisto.com/api -H "Authorization: TOKEN" -H "Content-Type: application/json" '
'--noproxy "*" -d \'<?xml version="1.0" encoding="utf-8"?>\''
]

Expand Down Expand Up @@ -1511,7 +1511,7 @@ def test_build_curl_get_withproxy(mocker):
"Content-Type: application/json\\r\\n\\r\\n'")
ilog.build_curl("send: b'{\"data\": \"value\"}'")
assert ilog.curl == [
'curl -X GET https://demisto.com/api -H "Authorization: <XX_REPLACED>" -H "Content-Type: application/json" '
'curl -X GET https://demisto.com/api -H "Authorization: TOKEN" -H "Content-Type: application/json" '
'--proxy http://proxy -k -d \'{"data": "value"}\''
]

Expand Down Expand Up @@ -1548,9 +1548,9 @@ def test_build_curl_multiple_queries():
"Content-Type: application/json\\r\\n\\r\\n'")
ilog.build_curl("send: b'{\"getdata\": \"value\"}'")
assert ilog.curl == [
'curl -X POST https://demisto.com/api/post -H "Authorization: <XX_REPLACED>" -H "Content-Type: application/json" '
'curl -X POST https://demisto.com/api/post -H "Authorization: TOKEN" -H "Content-Type: application/json" '
'--noproxy "*" -d \'{"postdata": "value"}\'',
'curl -X GET https://demisto.com/api/get -H "Authorization: <XX_REPLACED>" -H "Content-Type: application/json" '
'curl -X GET https://demisto.com/api/get -H "Authorization: TOKEN" -H "Content-Type: application/json" '
'--noproxy "*" -d \'{"getdata": "value"}\''
]

Expand Down Expand Up @@ -9554,3 +9554,68 @@ def test_create_clickable_test_wrong_text_value():

assert e.type == AssertionError
assert 'The URL list and the text list must be the same length.' in e.value.args


@pytest.mark.parametrize("request_log, expected_output", [
(
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nmy_authorization: Bearer token123\\r\\n'",
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nmy_authorization: Bearer <XX_REPLACED>\\r\\n'"
),
(
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nSet_Cookie: session_id=123\\r\\n'",
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nSet_Cookie: <XX_REPLACED>\\r\\n'"
),
(
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nAuthorization: token123\\r\\n'",
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nAuthorization: <XX_REPLACED>\\r\\n'"
),
(
"GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nAuthorization: Bearer token123\\r\\n",
"GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\nAuthorization: Bearer <XX_REPLACED>\\r\\n"
),
(
"send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\n'",
str("send: b'GET /api/v1/users HTTP/1.1\\r\\nHost: example.com\\r\\n'")
),
])
def test_censor_request_logs(request_log, expected_output):
"""
Given:
A request log.
case 1: A request log with a sensitive data under the 'Authorization' header, but the 'Authorization' is not capitalized and within a string.
case 2: A request log with a sensitive data under the 'Cookie' header, but with a 'Set_Cookie' prefix.
case 3: A request log with a sensitive data under the 'Authorization' header, but with no 'Bearer' prefix.
case 4: A request log with a sensitive data under the 'Authorization' header, but with no 'send b' prefix at the beginning.
case 5: A request log with no sensitive data.
When:
Running censor_request_logs function.
Then:
Assert the function returns the exactly same log with the sensitive data masked.
"""
assert censor_request_logs(request_log) == expected_output


@pytest.mark.parametrize("request_log", [
('send: hello\n'),
('header: Authorization\n')
])
def test_logger_write__censor_request_logs_has_been_called(mocker, request_log):
"""
Given:
A request log that starts with 'send' or 'header' that may contains sensitive data.
When:
Running logger.write function when using debug-mode.
Then:
Assert the censor_request_logs function has been called.
"""
mocker.patch.object(demisto, 'params', return_value={
'credentials': {'password': 'my_password'},
})
mocker.patch.object(demisto, 'info')
mocker.patch('CommonServerPython.is_debug_mode', return_value=True)
mock_censor = mocker.patch('CommonServerPython.censor_request_logs')
mocker.patch('CommonServerPython.IntegrationLogger.build_curl')
ilog = IntegrationLogger()
ilog.set_buffering(False)
ilog.write(request_log)
assert mock_censor.call_count == 1
2 changes: 1 addition & 1 deletion Packs/Base/pack_metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "Base",
"description": "The base pack for Cortex XSOAR.",
"support": "xsoar",
"currentVersion": "1.33.38",
"currentVersion": "1.33.39",
"author": "Cortex XSOAR",
"serverMinVersion": "6.0.0",
"url": "https://www.paloaltonetworks.com/cortex",
Expand Down

0 comments on commit 20dfbbc

Please sign in to comment.