Mask JWT tokens in logs in case of post-auth errors #1457
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The JWT tokens are valid for 1 minute, but still can be enough to steal the tokens from logs (if a hacker has access to the log stream or the log service) and use the token to access the data, potentially to change the user's credentials — if done in an automated way.
Codecov Report
@@ Coverage Diff @@
## main #1457 +/- ##
==========================================
+ Coverage 81.84% 81.89% +0.05%
==========================================
Files 60 60
Lines 8599 8596 -3
Branches 1271 1271
==========================================
+ Hits 7038 7040 +2
+ Misses 1230 1223 -7
- Partials 331 333 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
sfc-gh-aalam
requested review from
a team,
sfc-gh-jdu and
sfc-gh-stan
and removed request for
a team
sfc-gh-sfan
approved these changes
Mar 2, 2023
| data = json.dumps(decoded_data) | ||
| except Exception: | ||
| logger.info("data is not JSON") | ||
| _, masked_data, err_str = SecretDetector.mask_secrets(data) |
There was a problem hiding this comment.
Should we also check the first returned value to see if the data is masked? Or is this more like a best effort?
There was a problem hiding this comment.
if data is not masked, then original data is returned. It basically means that no secrets were detected. I'm not sure how we would use the masked value
There was a problem hiding this comment.
I see. Thanks for the explanation :)
7 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test pr for #1455
The JWT tokens are valid for 1 minute, but still can be enough to steal the tokens from logs (if a hacker has access to the log stream or the log service) and use the token to access the data, potentially to change the user's credentials — if done in an automated way.
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-750898: Connector is leaking JWT tokens to logs #1454
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Fixes a bug where JWT tokens got leaked into logs in case of post-auth errors.