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.

Sign up for GitHub

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

Jump to bottom

Security issue with aesbackend.go environment variable #362

Open
Profpatsch opened this issue Dec 6, 2020 · 4 comments · May be fixed by #389
Open

Security issue with aesbackend.go environment variable #362

Profpatsch opened this issue Dec 6, 2020 · 4 comments · May be fixed by #389

Comments

@Profpatsch
Copy link
Contributor

The -encrypt option introduced in #295 allows the config file to be encrypted via the BEEHIVE_CONFIG_PASSWORD environment variable:

beehive/cfg/aesbackend.go

Lines 225 to 239 in 57a4fab

func getPassword(u *url.URL) (string, error) {
p := os.Getenv(PasswordEnvVar)
if p != "" {
return p, nil
}
if u != nil && u.User != nil {
p = u.User.Username()
if p != "" {
return p, nil
}
}
return "", errors.New("password to encrypt or decrypt the config file not available")
}

In general, using an environment variable for a password is more secure than using a command line argument, with one caveat:

You need to unset the variable after reading it. Otherwise it will leak into any process that beehive spawns directly or indirectly and any library that is used by the bees, which is a security risk.
@Profpatsch
Copy link
Contributor Author

Since the save function also needs to read the password, it should probably be saved in a struct after the environment variable is unset. This is still not perfect (it could be read from the memory of the application), but better than leaking it to subprocesses by default.

cc @rubiojr

@rubiojr
Copy link
Collaborator

rubiojr commented Dec 6, 2020

Thanks @Profpatsch, a common and valid concern that we can probably mitigate (see restic/restic#2910 and restic/restic#521 for example for similar discussions and other alternatives), though if you run Beehive in an untrusted environment or executing rogue code (say via exec bee or linked modules), you'll probably have bigger issues.

I need to revisit the cfg package for #310 at some point, so adding it to my list.

Happy to help with a review if you feel like addressing this yourself before that happens.

@Profpatsch
Copy link
Contributor Author

though if you run Beehive in an untrusted environment or executing rogue code (say via exec bee or linked modules), you'll probably have bigger issues.

That is not the problem, the problem is with the environment variable accidentally leaking; you regularly have processes which dump the whole environment into log files for example.

juergenhoetzel added a commit to juergenhoetzel/beehive that referenced this issue Jan 2, 2022
@juergenhoetzel
Add support for reading AES backend password from external command
2051e24 
Fixes muesli#362
@juergenhoetzel juergenhoetzel linked a pull request Jan 2, 2022 that will close this issue
Open
@juergenhoetzel
Copy link

That is not the problem, the problem is with the environment variable accidentally leaking; you regularly have processes which dump the whole environment into log files for example.

Opened a PR to retrieve the secret from an external command: Add support for reading AES backend password from external command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for reading AES backend password from external command juergenhoetzel/beehive
3 participants
@rubiojr @juergenhoetzel @Profpatsch