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

Passphrase env vars with special characters no longer work with 1.8.13 #346

Open
witten opened this issue Aug 11, 2024 · 9 comments · May be fixed by #359
Open

Passphrase env vars with special characters no longer work with 1.8.13 #346

witten opened this issue Aug 11, 2024 · 9 comments · May be fixed by #359

Comments

@witten
Copy link
Collaborator

witten commented Aug 11, 2024

As of docker-borgmatic 1.8.13, some Borg passphrases that worked fine with 1.8.12 no longer work. For instance:

$ docker run --rm -e BORG_PASSPHRASE='`' ghcr.io/borgmatic-collective/borgmatic:1.8.13
[custom-init] Docker CLI variable not set, skipping...
[custom-init] No custom packages found, skipping...
/etc/s6-overlay/env/cron-env: line 1: unexpected EOF while looking for matching ``'
-----------------------------------
Software Versions:
...

Which, if cron is actually configured and runs, leads to a Borg error about: passphrase supplied in BORG_PASSPHRASE, by BORG_PASSCOMMAND or via BORG_PASSPHRASE_FD is incorrect.

Whereas with 1.8.12:

$ docker run --rm -e BORG_PASSPHRASE='`' ghcr.io/borgmatic-collective/borgmatic:1.8.12
[custom-init] Docker CLI variable not set, skipping...
[custom-init] No custom packages found, skipping...
-----------------------------------
Software Versions:

And then if cron is actually configured, the passphrase comes through correctly and there's no error from Borg.

I believe the problem was likely introduced in #331 and specifically in https://github.com/borgmatic-collective/docker-borgmatic/pull/331/files#diff-df5fc4ba71d9f961534c2d65215e50da1befb1f2a16c4a03c58eba442f8c5dc3

It's possible that a backtick is not the only special character broken in this way by 1.8.13.
@YabaiKai
Copy link

I agree. I've had significant issues with Borgmatic starting to declare my password incorrect, after having worked for the past 6 months at least. I'm convinced this is part of the problem. After painstaking review, I figured out that the issue was the BORG_PASSPHRASE environment variable in my docker compose file. In the past I had it set to BORG_PASSPHRASE="mypassword", but not it only works when I take out the quotes, i.e. BORG_PASSPHRASE=mypassword.

I'm guessing this is related. The README and documentation still has quotes, and I think this needs to be changed.

@917huB
Copy link

917huB commented Aug 17, 2024 •
edited
Loading

My backups started failing when called from CRON with 1.8.13 too, 'passphrase supplied in BORG_PASSPHRASE, by BORG_PASSCOMMAND or via BORG_PASSPHRASE_FD is incorrect.'. Passcode fails regardless whether its included in quotes or not.
If I log into the container, echo $BORG_PASSPHRASE shows the correct string that includes the following special characters &^*$#@. If i call borg from the command line it works fine, i.e docker exec borgmatic sh -c "borgmatic -c /etc/borgmatic.d/example.yaml --verbosity 1 --files"

EDIT: 9/5/24 failing in 1.8.14 too.

@SECtim
Copy link

SECtim commented Sep 11, 2024

I observe similar issues with passphrases that contain something like $0.

However, the root cause depends on the special character.
With characters like " and backticks, the following line fails immediately:

docker-borgmatic/root/etc/s6-overlay/s6-rc.d/secrets/run

Line 22 in 8adec5d

var_value=$(eval echo \$$var_name)

Whereas with stuff like $0, the issue is in the following lines (when using BORG_* environment variables, but a similar issue seems to exist for *_FILE variables):

docker-borgmatic/root/etc/s6-overlay/s6-rc.d/secrets/run

Lines 58 to 59 in 8adec5d

# Directly add non-_FILE variables to the environment file
echo "export $var_name=\"$var_value\"" >> "$ENV_FILE"

While the contents of the resulting /etc/s6-overlay/env/cron-env look correct in the second case (e.g., export BORG_PASSPHRASE="my$0passphrase") the double quotes around the value lead to another round of shell expansion for the cron environment - in the example above, this results in a BORG_PASSPHRASE value of my/runpassphrase.

I am unsure whether this expansion behavior is on purpose, maybe @Psycho0verload can comment on that - depending on the intended behavior, either code or documentation should be adjusted. My personal take is: the borgmatic image should make no assumptions at all about the passphrase (which requires some very careful handling of the passphrase in shell scripts).

@Psycho0verload
Copy link
Contributor

Psycho0verload commented Sep 14, 2024 •
edited
Loading

@SECtim It's been a few days now. I suspect that I also had problems with the special characters and that's why I put it in quotation marks.

I've seen that Linuxserver.io also works mainly with S6-overlay, and there are Secrets implemented by default. It's very cleanly written there and I'll test if that's not the better option for borgmatic as well.

@SECtim
Copy link

SECtim commented Sep 16, 2024

I am by no means a shell guru, but I suppose single quotes instead of double ones in the cron-env file would already solve most of the issues - except of course when the value itself contains single quotes, then these have to be escaped (I am not aware of a ready-to-use POSIX function to do so, but once again, I am not that familiar with shell programming).

@Psycho0verload
Copy link
Contributor

No problem, I am currently testing a different type of Secrets integration. The way it is used by linuxserver.io. that could help :)

@Psycho0verload
Copy link
Contributor

Just for your information. I am still on vacation. I will start working on this ticket again next week ;-)

@eeak
Copy link

eeak commented Oct 21, 2024

I encountered this problem. I spent a lot of time trying to find where I went wrong. Until I found this issue. I rolled back to 1.8.12.
Is there any news?

@Psycho0verload
Copy link
Contributor

I am current working on this

Psycho0verload added a commit to Psycho0verload/docker-borgmatic-s6 that referenced this issue Oct 26, 2024
@Psycho0verload
Delete old dependency
05e9c0b 
Fixing: 
```
fatal: during dependency resolution for service svc-cron: undefined service name secrets
```

Working on borgmatic-collective#346
Psycho0verload added a commit to Psycho0verload/docker-borgmatic-s6 that referenced this issue Oct 26, 2024
@Psycho0verload
Fix(Dockerfile): Casing-Warning
0dc584b 
Working on borgmatic-collective#346
Psycho0verload added a commit to Psycho0verload/docker-borgmatic-s6 that referenced this issue Oct 26, 2024
@Psycho0verload
Update(doku): Adapting the README to the new Secrets method
5acf08d 
Working on: borgmatic-collective#346
Psycho0verload added a commit to Psycho0verload/docker-borgmatic-s6 that referenced this issue Oct 26, 2024
@Psycho0verload
Update(Docu): Adapting the README to the new Secrets method
278b385 
Working on borgmatic-collective#346
Psycho0verload added a commit to Psycho0verload/docker-borgmatic-s6 that referenced this issue Oct 26, 2024
@Psycho0verload
Fix(docu): Add export to README
7620655 
Working on borgmatic-collective#346
@Psycho0verload Psycho0verload linked a pull request Oct 26, 2024 that will close this issue
Open
@witten witten mentioned this issue Oct 28, 2024
Closed
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.

Revision of the handling of Docker secrets in S6 Psycho0verload/docker-borgmatic-s6
6 participants
@witten @YabaiKai @917huB @Psycho0verload @eeak @SECtim