Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mask the cli flags related to username in the logs #6064

Open
wasim-nihal opened this issue Apr 4, 2024 · 2 comments
Open

mask the cli flags related to username in the logs #6064

wasim-nihal opened this issue Apr 4, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@wasim-nihal
Copy link
Contributor

Currently, flags related to the username are getting printed in the logs as plain text. In my organization username is treated as sensitive data and cannot be exposed.

I'd like to introduce a new boolean flag maskUsernameFlags which when set to true will mask the content of such flags and just print secret. Please do let me know if this is acceptable, would be happy to open a PR.

func IsSecretFlag(s string) bool {

@hagen1778 hagen1778 added the enhancement New feature or request label Apr 23, 2024
@hagen1778
Copy link
Collaborator

Hello @wasim-nihal!

I'd like to introduce a new boolean flag maskUsernameFlags which when set to true will mask the content of such flags and just print secret.

If those flags are URLs, we may not have that much control over what is printed. If the URL is passed to Go standard lib function (like http.Do()) then returned error may contain the full unmasked URL. The caller of this function won't be able to detect this without adding some type of parser for checking error messages for sensitive info. This might complicate the code, introduce a lot of changes for little gains.

Have you considered adding such sanitizations to logs collector/driver instead?

@wasim-nihal
Copy link
Contributor Author

Hi @hagen1778, I do not fully understand on how username flags can be passed as URLs. What I intended here is not to support the configuration of username flags as URLs (unlike password where we can give http://).

Instead, the proposed change is just to mask the flags from the logs similar to those below. Here if we see, httpAuth.password gets logged as secret whereas httpAuth.username gets logged as plain text.

$ ./victoria-metrics --httpAuth.username=hello --httpAuth.password=world
2024-04-24T06:00:07.506Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:12 build version: victoria-metrics-20240402-115328-heads-master-0-gdaa1326b9-dirty-e12c1b95
2024-04-24T06:00:07.515Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:13 command-line flags
2024-04-24T06:00:07.515Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:20   -httpAuth.password="secret"
2024-04-24T06:00:07.517Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:20   -httpAuth.username="hello"


$ ./vmagent -remoteWrite.url=https://127.0.0.1:8428/api/v1/write  -remoteWrite.basicAuth.username=hello  -remoteWrite.basicAuth.password=world -promscrape.config=./prom.yaml
2024-04-24T06:40:40.405Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:12 build version: vmagent-20240424-062430-heads-vmbackup-secure-url-0-g3445ee396-dirty-d8b89610
2024-04-24T06:40:40.406Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:13 command-line flags
2024-04-24T06:40:40.406Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:20   -promscrape.config="./prom.yaml"
2024-04-24T06:40:40.406Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:20   -remoteWrite.basicAuth.password="secret"
2024-04-24T06:40:40.406Z        info    /mnt/c/oss/VictoriaMetrics-VictoriaMetrics/lib/logger/flag.go:20   -remoteWrite.basicAuth.username="hello"

So, to mask such username flags at startup, the proposed solution is as follows to the file VictoriaMetrics/lib/flagutil/secret.go

var maskUsernameFlags= flag.Bool("maskUsernameFlags", false, "Whether to mask flags related to username from logs")

// IsSecretFlag returns true of s contains flag name with secret value, which shouldn't be exposed.
func IsSecretFlag(s string) bool {
	if strings.Contains(s, "pass") || strings.Contains(s, "key") || strings.Contains(s, "secret") || strings.Contains(s, "token") {
		return true
	}
        if *maskUsernameFlags && strings.Contains(s, "username"){
                return true
        }
	return secretFlags[s]
}

Please let me know if my understanding is not right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants