Load systemd encrypted credentials #902

Merged
witten merged 1 commits from cvlc12/borgmatic:cvlc12-patch-1 into main 2024-10-22 15:58:42 +00:00
3 changed files with 39 additions and 1 deletions

View File

@ -231,7 +231,10 @@ properties:
key. Only use on repositories that were initialized with
passcommand/repokey/keyfile encryption. Note that if both
encryption_passcommand and encryption_passphrase are set, then
encryption_passphrase takes precedence. Defaults to not set.
encryption_passphrase takes precedence. This can also be used to
access encrypted systemd service credentials (see "https://torsion\
.org/borgmatic/docs/how-to/provide-your-passwords/#using-systemd-\
service-credentials"). Defaults to not set.
example: "secret-tool lookup borg-repository repo-name"
cvlc12 marked this conversation as resolved Outdated

I don't think you can have two descriptions and two examples for a given option. So maybe just work the additional description into the existing one, maybe including the new example there? And/or relying on documentation rather than the schema to document this feature?

BTW you can test this with scripts/dev-docs if you have Docker Compose installed. Even just running tests with tox will exercise the code path that interprets the schema. (No Docker Compose required that way.)

More info at: https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/

I don't think you can have two descriptions and two examples for a given option. So maybe just work the additional description into the existing one, maybe including the new example there? And/or relying on documentation rather than the schema to document this feature? BTW you can test this with `scripts/dev-docs` if you have Docker Compose installed. Even just running tests with `tox` will exercise the code path that interprets the schema. (No Docker Compose required that way.) More info at: https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/

Indeed the tox tests failed, but they seem to run OK with one description but two examples. Does that look ok ?

Indeed the tox tests failed, but they seem to run OK with one description but two examples. Does that look ok ?

Unfortunately I don't think it works. Even it tests pass, the following won't work with those changes:

$ borgmatic config generate -d test.yaml --overwrite
Generating a configuration file at: test.yaml
...
  File "/home/witten/.local/pipx/venvs/borgmatic/lib/python3.12/site-packages/ruamel/yaml/constructor.py", line 278, in check_mapping_key
    raise DuplicateKeyError(*args)
ruamel.yaml.constructor.DuplicateKeyError: while constructing a mapping
  in "/home/witten/code/borgmatic/borgmatic/config/schema.yaml", line 228, column 9
found duplicate key "example" with value "cat ${CREDENTIALS_DIRECTORY}/borgpw" (original value: "secret-tool lookup borg-repository repo-name")
  in "/home/witten/code/borgmatic/borgmatic/config/schema.yaml", line 236, column 9
...

That error happens because standard YAML doesn't allow duplicate keys. What I recommend doing instead is to work any secondary examples into the description. It's not ideal, but at least it'd be valid YAML. ๐Ÿ˜„ Alternatively, or in addition, your documentation changes might be sufficient for documenting the systemd approach.

(BTW even if unit/integrations tests pass with the duplicate key, I'd expect end-to-end tests to properly catch it and fail.)

Unfortunately I don't think it works. Even it tests pass, the following won't work with those changes: ``` $ borgmatic config generate -d test.yaml --overwrite Generating a configuration file at: test.yaml ... File "/home/witten/.local/pipx/venvs/borgmatic/lib/python3.12/site-packages/ruamel/yaml/constructor.py", line 278, in check_mapping_key raise DuplicateKeyError(*args) ruamel.yaml.constructor.DuplicateKeyError: while constructing a mapping in "/home/witten/code/borgmatic/borgmatic/config/schema.yaml", line 228, column 9 found duplicate key "example" with value "cat ${CREDENTIALS_DIRECTORY}/borgpw" (original value: "secret-tool lookup borg-repository repo-name") in "/home/witten/code/borgmatic/borgmatic/config/schema.yaml", line 236, column 9 ... ``` That error happens because standard YAML doesn't allow duplicate keys. What I recommend doing instead is to work any secondary examples into the description. It's not ideal, but at least it'd be valid YAML. ๐Ÿ˜„ Alternatively, or in addition, your documentation changes might be sufficient for documenting the systemd approach. (BTW even if unit/integrations tests pass with the duplicate key, I'd expect [end-to-end tests](https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/#end-to-end-tests) to properly catch it and fail.)

Ok, so I removed the example and linked to the docs. I used the following to fit the URL in:

https://yaml.org/spec/1.2-old/spec.html#style/flow/double-quoted

Ok, so I removed the example and linked to the docs. I used the following to fit the URL in: https://yaml.org/spec/1.2-old/spec.html#style/flow/double-quoted

Sorry to keep going back and forth on this, but unfortunately I don't think this works either! Here's what ends up in the generated config file:

# The standard output of this command is used to unlock the encryption
# key. Only use on repositories that were initialized with
# passcommand/repokey/keyfile encryption. Note that if both
# encryption_passcommand and encryption_passphrase are set, then
# encryption_passphrase takes precedence. This can also be used to
# access encrypted systemd service credentials (see "https://torsion\
# .org/borgmatic/docs/how-to/provide-your-passwords/#using-systemd-\
# service-credentials"). Defaults to not set.
# encryption_passcommand: secret-tool lookup borg-repository repo-name

I think the problem is that that the double quotes are already embedded in a |-defined string. (YAML is kind of complex...) Rather than try to fight YAML on this, I'd instead recommend doing something like:

            ...
            encryption_passphrase takes precedence. This can also be used to
            access encrypted systemd service credentials. Defaults to not set.
            For more details, see:
            https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/

It's not ideal in that it doesn't have the direct link to the section in question, but it has the distinct benefit of resulting in a working URL in the generated config.

Sorry to keep going back and forth on this, but unfortunately I don't think this works either! Here's what ends up in the generated config file: ```yaml # The standard output of this command is used to unlock the encryption # key. Only use on repositories that were initialized with # passcommand/repokey/keyfile encryption. Note that if both # encryption_passcommand and encryption_passphrase are set, then # encryption_passphrase takes precedence. This can also be used to # access encrypted systemd service credentials (see "https://torsion\ # .org/borgmatic/docs/how-to/provide-your-passwords/#using-systemd-\ # service-credentials"). Defaults to not set. # encryption_passcommand: secret-tool lookup borg-repository repo-name ``` I think the problem is that that the double quotes are already embedded in a `|`-defined string. (YAML is kind of complex...) Rather than try to fight YAML on this, I'd instead recommend doing something like: ``` ... encryption_passphrase takes precedence. This can also be used to access encrypted systemd service credentials. Defaults to not set. For more details, see: https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/ ``` It's not ideal in that it doesn't have the direct link to the section in question, but it has the distinct benefit of resulting in a working URL in the generated config.
encryption_passphrase:
type: string

View File

@ -29,6 +29,37 @@ For example, to ask the *Pass* password manager to provide the passphrase:
encryption_passcommand: pass path/to/borg-repokey
```
### Using systemd service credentials
cvlc12 marked this conversation as resolved Outdated

Nice docs! I appreciate you writing this up.

Nice docs! I appreciate you writing this up.
Borgmatic supports using encrypted [credentials](https://systemd.io/CREDENTIALS/).
Save your password as an encrypted credential to `/etc/credstore.encrypted/borgmatic.pw`, e.g.,
```
# systemd-ask-password -n | systemd-creds encrypt - /etc/credstore.encrypted/borgmatic.pw
```
Note that the name `borgmatic.pw` is hardcoded in the systemd service file.
If you use multiple different passwords, save them as encrypted credentials to `/etc/credstore.encrypted/borgmatic/`, e.g.,
```
# mkdir /etc/credstore.encrypted/borgmatic
# systemd-ask-password -n | systemd-creds encrypt --name=borgmatic_backupserver1 - /etc/credstore.encrypted/borgmatic/backupserver1
# systemd-ask-password -n | systemd-creds encrypt --name=borgmatic_pw2 - /etc/credstore.encrypted/borgmatic/pw2
...
```
Ensure that the file names, (e.g. `backupserver1`) match the corresponding part of
the `--name` option *after* the underscore (_). The `borgmatic` folder is hardcoded in the systemd service file.
Then uncomment or use one of the following in your configuration file. Adjust `borgmatic_backupserver1`
according to the name given to the credential.
```yaml
encryption_passcommand: "cat ${CREDENTIALS_DIRECTORY}/borgmatic.pw"
encryption_passcommand: "cat ${CREDENTIALS_DIRECTORY}/borgmatic_backupserver1"
cvlc12 marked this conversation as resolved Outdated

This does make me wonder if you can just put all credentials inside a directory, even in the single credential case. Then your instructions/procedure would be the same regardless of the number of credentials.

Do not feel strongly.

This does make me wonder if you can just put _all_ credentials inside a directory, even in the single credential case. Then your instructions/procedure would be the same regardless of the number of credentials. Do not feel strongly.

Yes I considered that as well. I decided not to because I guess most people just use a single password, and the "foldername_filename" thing systemd came up with for the credential name when using multiple credentials might throw some people off. But I don't mind reconsidering if you think that's easier.

Yes I considered that as well. I decided not to because I guess most people just use a single password, and the "foldername_filename" thing systemd came up with for the credential name when using multiple credentials might throw some people off. But I don't mind reconsidering if you think that's easier.

The other reason is that it makes it easier to propagate the password/credential if using only one, since the path is not hardcoded. So borgmatic.pw will automatically be checked for in e.g. /run/credstore.encrypted in addition to /etc/credstore.encrypted/, etc..

The other reason is that it makes it easier to propagate the password/credential if using only one, since the path is not hardcoded. So `borgmatic.pw` will automatically be checked for in e.g. `/run/credstore.encrypted` in addition to `/etc/credstore.encrypted/`, etc..

I'm totally fine with it as is!

I'm totally fine with it as is!
```
### Environment variable interpolation
<span class="minilink minilink-addedin">New in version 1.6.4</span> borgmatic

View File

@ -9,6 +9,10 @@ ConditionACPower=true
[Service]
Type=oneshot
# Load encrypted credentials.
LoadCredentialEncrypted=borgmatic:/etc/credstore.encrypted/borgmatic/
LoadCredentialEncrypted=borgmatic.pw
# Security settings for systemd running as root, optional but recommended to improve security. You
# can disable individual settings if they cause problems for your use case. For more details, see
# the systemd manual: https://www.freedesktop.org/software/systemd/man/systemd.exec.html