fix(rtmp): setLogs(false) does not silence the command logs, which print the stream key - #2204
Open
aakashthirteen wants to merge 1 commit into
Open
aakashthirteen wants to merge 1 commit into
aakashthirteen wants to merge 1 commit into
Conversation
RtmpClient.setLogs() only reached RtmpSender, so the command logs in
CommandsManager and CommandsManagerImp were emitted unconditionally.
Those lines print the message as it goes on the wire:
send $connect -> includes tcUrl
send $releaseStream / $fcPublish / $publish -> include the stream name
read $message -> the server's onStatus reply,
which echoes the stream name
On YouTube Live, Twitch and most other services the stream name IS the
stream key, so an app has no way to keep a publishing credential out of
logcat.
CommandsManager gains isEnableLogs, defaulting to true so nothing
changes unless an app opts out, the info-level command logs are guarded
by it, and setLogs() now forwards to the commands manager as well as the
sender. Log.e is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
RtmpClient.setLogs(false)only reachesRtmpSender, so the command logs inCommandsManagerandCommandsManagerImpare emitted unconditionally. There is no way for an app to turn them off.Those lines print the message as it goes on the wire:
send $connecttcUrlsend $releaseStream,send $fcPublish,send $publishread $messageonStatusreply, which echoes the stream name backOn YouTube Live, Twitch and most other services the stream name is the stream key. Measured on a device publishing to YouTube with a deliberately distinctive key: it appears 4 times in logcat, plus the ingest URL once.
Since Android 4.1 logcat is readable only by the owning app, so this is not remotely exploitable. But it still writes a publishing credential into the device log, where a bug report or a phone with USB debugging enabled will pick it up, and an app currently has no way to prevent it.
Change
CommandsManagergainsisEnableLogs, defaulting totrue, so behaviour is unchanged unless an app opts out.CommandsManagerandCommandsManagerImpare guarded by it.RtmpClient.setLogs()forwards to the commands manager as well as the sender.Log.eis deliberately untouched.Honesty about verification
I could not build the module locally, as Gradle could not reach its distribution server from my environment. The change is 13 single-line guards plus one property and one line of wiring, each reviewed by hand, but please let CI confirm it rather than taking my word for it.
Happy to go further
If you would rather the stream name never appear even with logs enabled, I can redact it in those lines instead of, or as well as, gating them. Say the word and I will update this PR.