Skip to content

Commit

Permalink
Only read password from file
Browse files Browse the repository at this point in the history
  • Loading branch information
Christoph Petrausch committed Apr 16, 2020
1 parent 6ae68f4 commit f3b3263
Showing 1 changed file with 4 additions and 8 deletions.
12 changes: 4 additions & 8 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ func main() {
remote := flag.String("remote", "localhost:10011", "remote address of server query port")
listen := flag.String("listen", ":9189", "listen address of the exporter")
user := flag.String("user", "serveradmin", "the serverquery user of the ts3exporter")
password := flag.String("password", "", "the serverquery password of the ts3exporter")
passwordFile := flag.String("passwordfile", "/etc/ts3exporter/password", "file containing the password. Only read if -password not set. Must have 0600 permission.")
passwordFile := flag.String("passwordfile", "/etc/ts3exporter/password", "file containing the password. Must have 0600 permission. The file is not read if the environ")
enableChannelMetrics := flag.Bool("enablechannelmetrics", false, "Enables the channel collector.")
ignoreFloodLimits := flag.Bool("ignorefloodlimits", false, "Disable the server query flood limiter. Use this only if your exporter is whitelisted in the query_ip_whitelist.txt file.")

flag.Parse()
c, err := serverquery.NewClient(*remote, *user, mustReadPassword(*password, *passwordFile), *ignoreFloodLimits)
c, err := serverquery.NewClient(*remote, *user, mustReadPassword(*passwordFile), *ignoreFloodLimits)
if err != nil {
log.Fatalf("failed to init client %v\n", err)
}
Expand All @@ -47,13 +46,10 @@ func main() {
log.Fatal(http.ListenAndServe(*listen, nil))
}

// mustReadPassword reads the password either from the commandline flag or the password file. The password file is only
// mustReadPassword reads the password from the password file. The password file is only
// used if password is the empty string.
// If the file read fails, this function terminates the process.
func mustReadPassword(password, passwordFile string) string {
if password != "" {
return password
}
func mustReadPassword(passwordFile string) string {
fInfo, err := os.Stat(passwordFile)
if err != nil {
log.Fatalf("failed to get fileinfo of password file: %v\n", err)
Expand Down

5 comments on commit f3b3263

@ricardbejarano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this, it doesn't make it more secure just because it reads from a file and makes running this on Kubernetes much more hackier.

@hikhvar
Copy link
Owner

@hikhvar hikhvar commented on f3b3263 Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
I think injecting the password via the command line is more hacky in Kubernetes. The server query password should be stored in a kubernetes secret. To consume this secret via command line you have to inject that password as environment variable into the container. Than use that environment variable in the pod.spec.container.args list.
However mounting a kubernetes secret as file is a native secrets feature: https://kubernetes.io/docs/concepts/configuration/secret/#use-case-dotfiles-in-a-secret-volume

I agree with you that this requires more lines of yaml code but I think that is less hacky. However, I will add a method to consume the password via an environment variable.

Implemented: b5acfc6

@ricardbejarano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, I use my own ts3exporter image that among other improvements to yours, doesn't run the binary as root in the container. So when mounting the secret as a file with 0600 permissions I cannot set the file's owner UID, so the process can't read it. Link to my image, let me know if you want a PR.

I've seen you've added reading the password from an env. variable, thanks!

@hikhvar
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go ahead an create a PR for the static linked build and the scratch image. However I'm not a fan of alpine as builder image. Their busy box and muslc created too much hassle in the past. Those hassles are not worth the size reduction while building.

Regarding the fsgroup of the mounted secret: Have a look at the security context of a Pod. The property fsGroup allows you to change the ownership of mounted volumes.
https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#configure-volume-permission-and-ownership-change-policy-for-pods

@ricardbejarano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make the PR for the glibc-based image then soon.

About fsGroup that sets the group ownership, but ts3exporter demands 0400 permissions, that's user ownership exclusively. If fsUser existed or 0440 were enough, then that would work.

Please sign in to comment.