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

feat: Added support for Docker secrets #690

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

durandj
Copy link

@durandj durandj commented Feb 28, 2024

Summary

This adds the ability to use Docker secrets in configuration files. In Docker this is done by creating a secret resource, adding that secret to the container when you're starting it, and specifying an environment variable that has the _FILE suffix. The environment variable should point to the file which contains the secret (/run/secrets/<secret-name>).

Typically Docker images are setup so that they will try and find any environment variables that end in _FILE and set new environment variables with the same name minus the _FILE suffix in the running process. This is beneficial since environment variables that are set by the user when creating the container are visible to anyone who is able to run docker container inspect <container> on the host. For secrets this could be really damaging and leak sensitive information. Instead it is recommended to use Docker secrets.

Because Gatus uses the scratch base image I wasn't able to just use a Bash script to convert the secret file path into a normal environment variable like many other images do. Instead I opted to just modify the configuration logic so that it checks the environment variable name and changes its behavior based on that. This seems to work well enough.

As far as error handling, I opted not to crash the service when it's unable to read the secret file and instead just pretend its a normal environment variable and return an empty string. This follows the conventions of the rest of the configuration handling and leaves the error reporting to the configuration validation.

I've also updated the readme to mention this feature with a link to an example.

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

@onedr0p
Copy link
Contributor

onedr0p commented Mar 24, 2024

This is beneficial since environment variables that are set by the user when creating the container are visible to anyone who is able to run docker container inspect on the host. For secrets this could be really damaging and leak sensitive information. Instead it is recommended to use Docker secrets.

If an attacker can run docker container inspect to get the secrets, surely they also have access to the secret file on disk on the host unless a person is dumb enough to expose the Docker tcp socket over their network.

The major benefit I see here actually is to put your secrets in a separate file that's not in the docker-compose file.

@durandj
Copy link
Author

durandj commented Mar 24, 2024

It depends on how you initialize the secrets. Yes, at minimum you've moved your secrets to a different file that can be owned by some other user and not readable however you can also setup secrets from other sources. I want to not have my secrets live on disk and in an unencrypted form. That's completely nullified if someone just needs to then have access to the Docker daemon and they can now see anything that I've configured.

config/config.go Show resolved Hide resolved
t.Errorf("unable to write secret to temporary file: %s", err.Error())
}

os.Setenv("GATUS_TestExpandingOfFileEnvironmentVariables_FILE", tempFile.Name())
Copy link
Owner

Choose a reason for hiding this comment

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

can you unset the env var after this test using defer? Just to prevent surprises.

}

func TestExpandingOfFileEnvironmentVariablesMissingFile(t *testing.T) {
os.Setenv("GATUS_TestExpandingOfFileEnvironmentVariables_FILE", "TestExpandingOfFileEnvironmentVariablesMissingFile.txt")
Copy link
Owner

Choose a reason for hiding this comment

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

Same here

Comment on lines +1723 to +1730
os.Setenv("GATUS_TestExpandingOfFileEnvironmentVariables_FILE", tempFile.Name())
os.Setenv("GATUS_TestExpandingOfFileEnvironmentVariables", otherSecretValue)
Copy link
Owner

Choose a reason for hiding this comment

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

Here too

@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 80.75%. Comparing base (08742e4) to head (b16b74e).
Report is 13 commits behind head on master.

Files Patch % Lines
config/config.go 69.23% 2 Missing and 2 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #690      +/-   ##
==========================================
+ Coverage   78.62%   80.75%   +2.12%     
==========================================
  Files          58       59       +1     
  Lines        4759     4094     -665     
==========================================
- Hits         3742     3306     -436     
+ Misses        831      596     -235     
- Partials      186      192       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This adds the ability to use [Docker
secrets](https://docs.docker.com/compose/use-secrets/) in configuration
files. In Docker this is done by creating a secret resource, adding that
secret to the container when you're starting it, and specifying an
environment variable that has the `_FILE` suffix. The environment
variable should point to the file which contains the secret
(`/run/secrets/<secret-name>`).

Typically Docker images are setup so that they will try and find any
environment variables that end in `_FILE` and set new environment
variables with the same name minus the `_FILE` suffix in the running
process. This is beneficial since environment variables that are set by
the user when creating the container are visible to anyone who is able
to run `docker container inspect <container>` on the host. For secrets
this could be really damaging and leak sensitive information. Instead it
is recommended to use Docker secrets.

Because Gatus uses the `scratch` base image I wasn't able to just use a
Bash script to convert the secret file path into a normal environment
variable like many other images do. Instead I opted to just modify the
configuration logic so that it checks the environment variable name and
changes its behavior based on that. This seems to work well enough.

As far as error handling, I opted _not_ to crash the service when it's
unable to read the secret file and instead just pretend its a normal
environment variable and return an empty string. This follows the
conventions of the rest of the configuration handling and leaves the
error reporting to the configuration validation.

I've also updated the readme to mention this feature with a link to an
example.
@durandj
Copy link
Author

durandj commented Apr 13, 2024

@TwiN I've addressed your comments, thanks for reviewing!

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

Successfully merging this pull request may close these issues.

4 participants