In spirit, POSIX considers `echo` rather obsolete (it was just kept because of
its widespread use).
It’s also not possible to use `echo` portably unless it’s `-n`-option (as the
first argument) and escape sequences are omitted.
While neither was the case here, it’s better style to just always use `printf`
in order to avoid any future confusion when both are used.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
This commit finishes the work from the previous one and converts
ssh_filter_btrbk.sh to (mostly) pure POSIX Shell Command Language.
Instead of bash’s `=~`-operator for its `[[ … ]]`-compound-command it uses
`grep`.
At the time of writing, bash has at least the `nocasematch`-shell-option which
would have a negatve security impact for this program. While it’s not enabled
per default single users could potentially change that, not realising the
consequences.
Thus, moving away from this may also provide some hardening.
Unlike bash’s `=~`-operator, which matches against the whole string at once,
`grep` matches the pattern against each line of input.
This would allow for attacks by including a newline in the SSH command like in:
SSH_ORIGINAL_COMMAND="readlink /dev/stdout
cat /etc/shadow"
but is prevented by the general exclusion of newlines in commit TODO.
`grep` may return an exit status of `0` when used with its `-q`-option, even
when an error occurred.
Since this program is intended specifically for security purposes this shall be
avoided, even if such case is unlikely, and therefore its standard output and
standard error are redirected to `/dev/null` instead.
Further, using just:
local formatted_restrict_path_list="$(printf '%s' "$restrict_path_list" | sed 's/|/", "/g')"
rather than:
local formatted_restrict_path_list=""; formatted_restrict_path_list="$(printf '%s' "$restrict_path_list" | sed 's/|/", "/g')"
prevent `set -e` to take effect if the pipeline within the command substitution
fails, as the returned exit status of the whole command is the result of
`local`, not that of the assignment.
This is however no security problem here, as `formatted_restrict_path_list` is
only used for informative pruposes.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
ssh_filter_btrbk.sh is mainly intended for security purposes and should
therefore itself be written with that in mind.
It is written for bash, which greatly extends the POSIX Shell Command Language
and is incompatible with it in some niche cases.
For several reasons, it seems a good idea to convert the program to (mostly)
pure POSIX Shell Command Language:
• People may try to use the program with other shells (for example when bash is
not available) and make errors.
More pure `sh` implementations like dash …
• … have far less code and also less dependencies, which possibly also reduces
the chance for bugs or exploits,
• … are less dynamic in development (and have thus possibly a lower chance of
incompatible changes) …
• … and may run faster.
This commit replaces any unnecessary “bashishms” with purely POSIX compatible
code, with the exception of the `local`-built-in, which is however supported by
most POSIX compatible shells (including dash, klibc-utils’s `sh` and BusyBox’
`sh`) in some way.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
Double quote any variable expansions that might ever contain field separators.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
In strings that don’t contain `'` nor do any expansions, use single quotes to
avoid any future unintended expansions or escapes.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
This disallows newline (that is: LF characters) in the SSH command, which could
have been exploited for arbitrary code execution, since commit
77a39282de.
Example:
# export SSH_ORIGINAL_COMMAND=$'readlink /dev/stdout\ncat /etc/shadow'
# ssh_filter_btrbk.sh
Since `readlink` is a generally allowed command, this works with any of
ssh_filter_btrbk.sh’s options.
But most likely, other commands that are “added” via `allow_cmd()` can be used,
too.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date: Wed Nov 30 04:29:53 2022 +0100
#
# On branch fix-remote-code-execution
# Your branch and 'origin/fix-remote-code-execution' have diverged,
# and have 1 and 1 different commits each, respectively.
# (use "git pull" to merge the remote branch into yours)
#
# Changes to be committed:
# modified: ssh_filter_btrbk.sh
#
# Untracked files:
# ORIG
#
While most functionality works fine, raw backups fail to write correct
"FILE=" information in info sidecar.
Disallowing newlines in files is a good idea in general.
This adds support for bzip3 [1].
[1] https://github.com/kspalaiologos/bzip3
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
Cosmetics: swap order pbzip2 / bzip3
Signed-off-by: Axel Burri <axel@tty0.ch>
`mydomain.com` is actually a real domain and shouln’t be used in examples.
RFC 2606 (respectively RFC 6761) reserves `example.org` (and others) for that
purpose.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
Since `btrbk` executes only commands, it shouldn’t need any of what’s currently
disabled with the `restrict` flag in the `authorized_keys` file, that is:
Port-, agent- and X11-forwarding as well as PTY allocation and execution of
`~/.ssh/rc`.
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
A redirection (e.g. `echo foo > bar.info`) can cause empty (zero-size)
files in some circumstances.
We still write INCOMPLETE=1 to the info file before send/receive, but
instead of re-creating it without the INCOMPLETE flag, we append
INCOMPLETE=0 (keeping up compatibility with old versions of btrbk).
Ref: 4e5ae975d8 btrbk: ignore zero-size info files
When backuping from devices that have configured to use raw backup and
that might disconnect from the network (ie. laptops) you end up once in
a while with 0 size info file (and backup file).
btrbk don't know how to handle 0 file and stop backing up until the zero
size file is removed.
With this change 0 size info file will be ignored, and hence the backup
for the given backup will be redone.
Signed-off-by: Matthieu Patou <mat@matws.net>
Warning for btrfs_commit_delete is always printed, regardless of the
(possibly valid) values.
regression in btrbk-0.32.3
687e0508b7 btrbk: tidy deprecation warnings
It is perfectly ok to run btrbk without ssh_identity (using ssh
defaults), printing a warning if the option is not set is wrong.
Instead, hackily check for ssh_identity on ssh errors, and give a hint
in the error message.
Christoph Anton Mitterer