fix: redact passwords in logs

[martinohmann]

Fixes #238

Replaces URLs of the format rtsp://user:password@localhost:8554 with rtsp://user:xxxxx@localhost:8554 in logs.

This is best-effort for now and does not handle cases where passwords appear in query strings. It should be fairly easy to extend the RedactPassword function in the future in case there are other common password pattern that are worth handling.

Let me know if this should be better placed in a different package. I couldn't find a more suitable place than streams and also didn't want to introduce a new package like utils just for a single function.

[martinohmann]

Theoretically, this could be private since it's only used in this module so far.

[AlexxIT]

I thought about this feature. Problem not only with logs, but also with another places.
URLs not always just urls, but also ffmpeg and exec sources with url.
Also we have REST API for streams (json) and for graph (dot) and for logs and for config!

[martinohmann]

Yeah, some places might be tricky. Redacting passwords for the /api/streams.dot endpoint should be easy and I can add that here as well if you like.

However, we cannot really redact them from the /api/streams endpoint as it would make it impossible to consume these streams then. Also, it's not possible for /api/config because of the edit functionality we have in the UI and redacting passwords there would break the config on save. These would require API auth to properly secure them which is already supported.

For me it would be already helpful if these passwords do not end up in the logs. Having them exposed via the API is less of a concern for me because I restrict access to this to trusted services only (home assistant in my case).

[martinohmann]

FWIW, I found these examples from the README, that I could probably also redact here:

streams:
  # Custom header
  custom_header: "https://mjpeg.sanford.io/count.mjpeg#header=Authorization: Bearer XXX"

streams:
  # OAuth credentials
  nest-doorbell: nest:?client_id=***&client_secret=***&refresh_token=***&project_id=***&device_id=***

streams:
  # link to external Hass with Long-Lived Access Tokens
  hass-webrtc2: hass://192.168.1.123:8123?entity_id=camera.nest_doorbell&token=eyXYZ...

I'll have a look how to handle sources like ffmpeg and nest in the redact logic. My current impl only works for URLs without an explict source.

[AlexxIT]

streams.dot it's just reformat streams.json. Not necessary to fix it separately.

[martinohmann]

@AlexxIT I pushed a new commit which expands the logic to also redact sensitive values in query parameters, # stream options and URLs within exec sources. It also is aware of redirects now. You can have a look at the added test cases to see what's supported now.

Let me know what you think.

[skrashevich]

Maybe, it be useful to detect passwords in string by entropy calculation?

func entropy(text string) float64 {
	textLength := len(text)
	if textLength == 0 {
		return 0.0
	}

	uniqueCharacters := make(map[rune]int, textLength)
	for _, r := range text {
		uniqueCharacters[r]++
	}

	var entropy float64
	textLengthFloat := float64(textLength)
	log2 := math.Log(2)
	for _, count := range uniqueCharacters {
		probability := float64(count) / textLengthFloat
		entropy -= probability * (math.Log(probability) / log2)
	}

	return entropy
}

[martinohmann]

@skrashevich Nice idea, but still this wouldn't catch cases where people use low-entropy passwords.

@AlexxIT If you're not satisfied with the solution in this PR there's also another simpler alternative to this problem. Right now, the main issue is that stream urls (which potentially contain passwords) end up in the logs at Info level, which is the default. We could change these log statements to Debug/Trace instead to have them not show up in the logs by default.

That would solve the issue for me at least. Within certain modules we could probably switch from logging the stream url to logging the stream name instead (at Info level) to not lose too much information.

I can provide a PR to update the log levels if that sounds reasonable for you.

I'm personally also not 100% happy with this PR here as it's adding quite a bit of code for a minor thing that does not really contribute to the core logic to go2rtc.

[AlexxIT]

Just haven't time to check it yet