Skip to content

Commit cb77141

Browse files
jaytripathMritunjay Tripathi
andauthored
Fixing protected settings key leak (#2198)
Co-authored-by: Mritunjay Tripathi <mrtripathi@microsoft.com>
1 parent 07aa79f commit cb77141

2 files changed

Lines changed: 19 additions & 6 deletions

File tree

AzureMonitorAgent/agent.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,9 +2447,13 @@ def parse_context(operation):
24472447
output = 'Logrotate removal failed with error: {0}\nStacktrace: {1}'.format(ex, traceback.format_exc())
24482448
hutil_log_info(output)
24492449
else:
2450-
if not os.path.exists(AMAExtensionLogRotateFilePath):
2451-
logrotateFilePath = os.path.join(os.getcwd(), 'azuremonitoragentextension.logrotate')
2452-
copyfile(logrotateFilePath,AMAExtensionLogRotateFilePath)
2450+
if not os.path.exists(AMAExtensionLogRotateFilePath):
2451+
try:
2452+
logrotateFilePath = os.path.join(os.getcwd(), 'azuremonitoragentextension.logrotate')
2453+
copyfile(logrotateFilePath,AMAExtensionLogRotateFilePath)
2454+
except Exception as ex:
2455+
output = 'Logrotate config copy failed with error: {0}\nStacktrace: {1}'.format(ex, traceback.format_exc())
2456+
hutil_log_info(output)
24532457

24542458
# parse_context may throw KeyError if necessary JSON key is not
24552459
# present in settings
@@ -3060,9 +3064,13 @@ def get_settings():
30603064
protected_settings_str = None
30613065
for decrypt_cmd in [cms_cmd, smime_cmd]:
30623066
try:
3067+
# stderr is intentionally captured separately from stdout (and never
3068+
# logged verbatim) so that non-fatal openssl warnings (e.g. a missing
3069+
# openssl.cnf) can't get concatenated with the decrypted output, corrupt the
3070+
# JSON payload, and risk the decrypted secret being echoed in error handling.
30633071
session = subprocess.Popen([decrypt_cmd], shell=True,
30643072
stdin=subprocess.PIPE,
3065-
stderr=subprocess.STDOUT,
3073+
stderr=subprocess.PIPE,
30663074
stdout=subprocess.PIPE)
30673075
output = session.communicate(decoded_settings)
30683076
# success only if return code is 0 and we have output

Utils/HandlerUtil.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,13 @@ def _parse_config(self, ctxt):
222222
jctxt = ''
223223
try:
224224
jctxt = json.loads(protected_settings_str)
225-
except:
226-
self.error('JSON exception decoding ' + HandlerUtility.redact_protected_settings(protected_settings_str))
225+
except Exception as e:
226+
# Do not log protected_settings_str here: it is the decrypted
227+
# plaintext of protectedSettings and may contain secrets (e.g.
228+
# workspace keys). redact_protected_settings() only knows how to
229+
# redact the outer (still-encrypted) envelope fields, so it would
230+
# not mask secrets that live inside this already-decrypted payload.
231+
self.error('JSON exception decoding protectedSettings: {0}'.format(e))
227232
handlerSettings['protectedSettings']=jctxt
228233
self.log('Config decoded correctly.')
229234
return config

0 commit comments

Comments
 (0)