-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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(secretstores): Add systemd-credentials plugin #13657
Conversation
eb9a2cf
to
5b16fe0
Compare
I have some limited interest in this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code and process looks fine. I've approved but added a number of comments to improve the documentation, but feel free to incorporate them and commit with another review from me.
I forgot to mention,
I then used
I could then add a remove a virtual tpm device with:
This got me to see:
which was cool, but it appears I needed to do more to actually use it:
but I was able to solve that with:
If I shutdown the VM, remove the tpm, start it and try to access the secret, I see (simulates decryption key in /var and /etc/telegraf/credentials being stolen and used on another machine):
Cool! |
```text | ||
[Service] | ||
LoadCredentialEncrypted=http_user:/etc/telegraf/credentials/http_user | ||
LoadCredentialEncrypted=http_passwd:/etc/telegraf/credentials/http_passwd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider documenting the use of PrivateMounts=
as described in https://systemd.io/CREDENTIALS/:
"It is a good idea to also enable mount namespacing for services that process credentials configured this way. If so, the runtime credential directory of the specific service is not visible to any other service. Use PrivateMounts= as minimal option to enable such namespacing. Note that many other sandboxing settings (e.g. ProtectSystem=, ReadOnlyPaths= and similar) imply PrivateMounts=, hence oftentimes it’s not necessary to set this option explicitly."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be safe since the system mounts should be visible to telegraf but new mounts (like the one for the decrypted files) are not propagated outside of telegraf's private mount namespace, but will need testing. From https://manpages.ubuntu.com/manpages/lunar/en/man5/systemd.exec.5.html
PrivateMounts=
Takes a boolean parameter. If set, the processes of this unit will be run in their own
private file system (mount) namespace with all mount propagation from the processes
towards the host's main file system namespace turned off. This means any file system
mount points established or removed by the unit's processes will be private to them
and not be visible to the host. However, file system mount points established or
removed on the host will be propagated to the unit's processes. See
[mount_namespaces](https://manpages.ubuntu.com/manpages/lunar/en/man7/mount_namespaces.7.html)(7) for details on file system namespaces. Defaults to off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can do this as it also means that Telegraf cannot read files in the file-system which is required by various plugins...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the thing; my interpretation of the man page is that the system's private mounts all propagate into the container, but mounts within the container do not propagate into the host. This is also in the man page:
When turned on, this executes three operations for each invoked process: a new
CLONE_NEWNS namespace is created, after which all existing mounts are remounted to
MS_SLAVE to disable propagation from the unit's processes to the host (but leaving
propagation in the opposite direction in effect). Finally, the mounts are remounted
again to the propagation mode configured with MountFlags=, see below.
This is why the flag is meaningful for this feature: without it, the mount is available to others but with it, it is not available to other services (that use PrivateMounts).
I did look into this further and found that:
- when the service is running and
LoadCredentialsEncrypted=...
is used,$CREDENTIALS_DIRECTORY
is/run/credentials/telegraf.service
in the service, and this directory has 0500 permissions withtelegraf
as the owner (decrypted files are 0400 withtelegraf
as owner) - when the service is running and
LoadCredentialsEncrypted=...
is used, the decrypted credentials are in the root-protected/run/credentials/...
directory on the host - when the service is running and
LoadCredentialsEncrypted=...
is used andPrivateMounts
IS NOT set, everything in/run/credentials
on the host is available in/run/credentials
in the service - when the service is running and
LoadCredentialsEncrypted=...
is used andPrivateMounts
IS set, only/run/credentials/telegraf.service
on the host is available in/run/credentials
in the service - when services that specify
LoadCredentialsEncrypted=...
aren't running, then the decrypted credentials for those services are NOT present in/run/credentials
on the host (or any running services)
Importantly, this has a few implications:
- root on the host will always be able to see all the credentials via
/run/credentials
(but also by running thesystemd-creds
command, entering mount namespaces, adjusting unit files, etc). There is no protection against unrestricted root processes on the host - with or without
PrivateMounts
being set, the decrypted credentials have DAC permissions set to theUser=
in the systemd unit. Specifically for telegraf, only processes as thetelegraf
user can see the decrypted credentials in/run/credentials/telegraf.service
on the host or within the service. Other non-root processes cannot see the credentials - with
PrivateMounts
NOT SET in a service's unit file that runs as ROOT, the root user can see all currently decrypted credentials for running services (eg, thefoo.service
doesn't specifyUser=foo
(or specifiesUser=root
), this service can see/run/credentials/foo.service/*
,/run/credentials/telegraf.service/*
and any other decrypted credentials for running services) - with
PrivateMounts
NOT SET in a service's unit file that runs as NON-ROOT, this non-root user can see all currently decrypted credentials for running services that are accessible to this non-root user - with
PrivateMounts
SET in a service's unit file that runs as ROOT, the root user can only see its own decrypted credentials (eg, thefoo.service
doesn't specifyUser=foo
(or specifiesUser=root
), this service can see/run/credentials/foo.service/*
but not/run/credentials/telegraf.service/*
or any other decrypted credentials for running services) - with
PrivateMounts
SET in a service's unit file that runs as NON-ROOT, this service can only see/run/credentials/foo.service/*
but not/run/credentials/telegraf.service/*
or any other decrypted credentials for running services regardless of if they run as the same user or not)
Practically speaking, this feature does not protect against attacker-controlled root processes or attacker-controlled root running services that don't set PrivateMounts
as these processes can always see the decrypted credentials. PrivateMounts
adds some protection to the telegraf service against attacker-controlled non-root running services. It adds considerable protection against attacker-controlled root running services when those root running services also specify PrivateMounts
in their unit files (assuming they can't rewrite the unit file; see ProtectSystem=full
in the man page).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my investigation of PrivateMounts
, I'm now changing this to 'Request changes' so the documentation can be updated to recommend using PrivateMounts
. Feel free to adjust the unit file in the deb and rpm packaging to specify it by default (but that is not a blocker for this PR and can come at a later time). Please also mention in the docs why it is recommended (and ideally that it can offer protections when other units also specify it).
@jdstrand thanks for the review. I hope I addressed all your points. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! I've given my approval so you're not blocked on me, but I gave some more suggestions to the readme.
@jdstrand thanks for the feedback. I used all your suggestions so you might or might not want to take another look and approve!? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the work on this and sorry this took so long. Mainly questions for my education and to verify we are on the same page.
55f647c
to
656ed09
Compare
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take it for v1.29! Thank you for the work and to @jdstrand for the reviews.
The PR adds a secret-store plugin allowing to access systemd credentials from Telegraf (requires systemd 254+). This is helpful when starting Telegraf as a service without manual interaction. There the new plugin can serve as a initial secret-store to unlock others or to provide the secrets itself.
Please checkout https://systemd.io/CREDENTIALS/ for details.
Tested on ArchLinux and Ubuntu Server 23.04.