fix: remove stream url from info logs #1341
Open
Conversation
This file contains 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
This solves the most prominent credential leak. With every new connection the passwords were leaked to the logs visible in the UI as well. With this change only the stream name gets logged here. There are other places in the codebase where we still log secret embedded in URLs, but a lot of them lack access to the stream name they could be replaced with. Most of these occurances are debug logs, but some are warning level though. I can try to come up with a fix in a followup PR. This change is a simpler alternative to AlexxIT#1219 to address the issue to rid of the password in the log line emitted whenever a stream is created: [streams] create new stream url=rtsp://stream:xxxxxxxxxx@192.168.42.42:554/stream1 [streams] create new stream garage-cam
martinohmann
commented
Sep 6, 2024
| log zerolog.Logger | ||
| streams = map[string]*Stream{} | ||
| streamsMu sync.Mutex | ||
| ) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updates #238
This solves the most prominent credential leak. With every new connection the passwords were leaked to the logs visible in the UI as well.
With this change only the stream name gets logged here.
There are other places in the codebase where we still log secret embedded in URLs, but a lot of them lack access to the stream name they could be replaced with. Most of these occurances are debug logs (which are not an issue if you're running go2rtc at >= info level), but some are warning level though. I can try to come up with a fix in a followup PR.
This change is a simpler alternative to #1219 to address the issue to rid of the password in the log line emitted whenever a stream is created.
Before:
After: