#3259 Issue closed: ReaR must not carelessly 'source' files

Labels: enhancement, cleanup, fixed / solved / done, severe improvement

jsmeix opened issue at 2024-06-21 05:15:

See
https://github.com/rear/rear/pull/3203#issuecomment-2063439858
(some cases are clear like 'source default.conf'
but some cases look rather obscure)

An attempt to generically mitigate it was not accepted, see
https://github.com/rear/rear/pull/3258

See also the related issue
https://github.com/rear/rear/issues/3260

jsmeix commented at 2024-07-10 12:51:

@rear/contributors

A (rather artificial) example
how ReaR would source user-controlled files by accident, cf.
https://github.com/rear/rear/issues/3260#issuecomment-2219790045
and
https://github.com/rear/rear/issues/3260#issuecomment-2219801028
and
https://github.com/rear/rear/pull/3258#issuecomment-2180230626

In particular I do not know what could be possible
for "git clone" things done by whatever user and
stored under arbitrary directories and then
"just run" that via 'sudo' or something like that.

On the one hand I think it is not ReaR's task
to protect 'root' against any foolish things.

But because ReaR is meant to be run as 'root'
I think we must implement some reasonable protection
to avoid at least too obviously bad things
which can be easily avoided.

When I wrote that I had in particular Ubuntu users in mind
where I fear some may carelessly "sudo COMMAND" all the time.

Right now I did:

johannes@localhost:~> git clone https://github.com/rear/rear.git
Cloning into 'rear'...
...

johannes@localhost:~> mv rear rear.github.master

johannes@localhost:~> cd rear.github.master

johannes@localhost:~> vi etc/rear/local.conf
...

johannes@localhost:~/rear.github.master> su
Password: ...

localhost:/home/johannes/rear.github.master # usr/sbin/rear mkrescue

"rear mkrescue" just worked with user-controlled files
because all ReaR scripts and etc/rear/local.conf
belong to "johannes users".

You could have - if you were interested - tried that
on your own to reproduce and experience it yourself.

Of course now you may tell me that this case
is root's own responsibility and his own fault
to do 'su' carelessly and run user-controlled stuff
but (again and again) you miss my point.

My point is not that this example shows
how root could fool itself.

My point is that this example proves that
there is absolutely noting in ReaR that checks
at runtime if what ReaR executes is trustworthy.

Again and again - but unfortunately it seems
you do not yet pay sufficient attention to what law
(at least German law as far as I understand what it means)
mandates from each of us personally - my fundamental
reson behind all that is what I described in
https://github.com/rear/rear/issues/2967#issuecomment-1498627825
and
https://github.com/rear/rear/issues/2967#issuecomment-1510856524

It is left to the reader as an exercise to find more examples
(perhaps even actual threats that can be exploited right now)
where ReaR "just executes" files from non-trusted users.

jsmeix commented at 2024-07-10 13:09:

For comparison with my SourceTrustworthy implementation from
https://github.com/rear/rear/pull/3258

johannes@localhost:~/rear.github.master> git checkout jsmeix-SourceTrustworthy
...
Switched to a new branch 'jsmeix-SourceTrustworthy'

johannes@localhost:~/rear.github.master> su
Password: ...

localhost:/home/johannes/rear.github.master # usr/sbin/rear mkrescue
Refused 'SourceTrustworthy /home/johannes/rear.github.master/usr/share/rear/conf/Linux-i386.conf' because file owner 'johannes' is not in TRUSTED_FILE_OWNERS
ERROR: 
====================
BUG in usr/sbin/rear line 749:
Failed to Source /home/johannes/rear.github.master/usr/share/rear/conf/Linux-i386.conf
...
Terminated

Of course verly likely my current SourceTrustworthy
implementation is not yet a final ultimate solution
but (hopefully) it could be at least one step forward
into the right direction compared to endless high-level
theoretical questions that I can never answer properly
so that all decays and dies out in nowhere land
since a few month (I think decay started in April)

schlomo commented at 2024-07-10 13:15:

I'd like to clearly take the problem of "security of developer environment" out of scope for this topic. I'd assume that developers only run sudo ./usr/sbin/rear mkrescue on test systems.

I'd also like to take all means of deploying ReaR from source out of scope, meaning the problem of users using git clone to get ReaR and then running it. Or downloading an archive of ReaR from GitHub and running sudo ./usr/sbin/rear mkrescue.

For me the scope is a production system where ReaR was installed via package (including the packages we provide on our GitHub releases page). And on this production system and a subsequently used recovery system we can think about potential attack vectors that ReaR can be part of.

jsmeix commented at 2024-07-10 13:16:

@schlomo !!!

Please follow our instructions that we tell our users:

http://relax-and-recover.org/documentation/getting-started

No excuses!

schlomo commented at 2024-07-10 13:18:

Yes, this is a quick start and not a production deployment. I'm happy to add there that for production deployments people should use packages. Quick start is for a first user to quickly see something happening, nothing more.

jsmeix commented at 2024-07-15 08:27:

Possibly we have some kind of "all-clear" signal for ReaR:

I think meanwhile I found sufficient evidence
which shows that "all other software" behaves
basically same as ReaR currently does.
Details will follow later because currently
things are under investigation via a SUSE internal
bug report that I filed.

So (at least for now) I downgrade this issue
on from "blocker" to "enhancement"
but still with "critical / security / legal" scope.

jsmeix commented at 2024-07-15 13:07:

@schlomo
I would change that "quick start" guide
to do all as 'root' and explicitly describe that
the local "git clone" must be under root's control
to avoid to cross a privilege boundary
from 'root' who runs usr/sbin/rear to a non-root user
by sourcing ReaR's bash scripts when those scripts
were downloaded under control of a non-root user.

jsmeix commented at 2024-09-05 14:34:

Regarding
https://github.com/rear/rear/issues/3259#issuecomment-2220492231

It does not really matter if a security issue exists
"for a first user to quickly see something happening"
or in a "production deployment".
Both "first user" environments and production environments
should be reasonably secure.

Those 'sudo ...' commands in our quick start guides
indicate it is in particular meant for Ubuntu users
what they could do on their (home) systems.
I think we should provide in particular those users
(who are presumably often rather unexperienced)
some reasonably secure quick start guide.

jsmeix commented at 2024-09-17 11:05:

All found cases where ReaR sources files
were checked via
https://github.com/rear/rear/issues/3260
and its subsequent issues
https://github.com/rear/rear/issues/3285
and
https://github.com/rear/rear/issues/3319
so this issue itself is no longer "critical/security/legal"
according to what I wrote in
https://github.com/rear/rear/issues/3260#issuecomment-2355205283
and I also move the milestone of this issue to 'ReaR v3.1'

jsmeix commented at 2024-09-17 11:08:

@didacog
I dared to also assign you to this issue
because your contribution would by much appreciated
in particular regarding "sourcing remote files".

pcahyna commented at 2024-10-01 13:05:

I would like to unblock the issue by proposing a viable path forward.

The issue does not have a high priority for me
(cf. "downgrade this issue on from "blocker" to "enhancement" "
https://github.com/rear/rear/issues/3259#issuecomment-2227955337 )
but it would still be good to have an idea how to progress.

My starting point is the observation that "all other software" behaves basically same as ReaR currently does.
(Another starting point is abandoning the "same-author policy"
idea from
https://github.com/rear/rear/issues/3260#issuecomment-2220864142,
as there is has been no follow-up to explain it.)

So, other software executes potentially untrusted code all the time.

The shell executes commands that it finds in essentially arbitrary directories.

The Python interpreter loads modules again from essentially arbitrary directories.

The dynamic linker (ELF binary interpreter) loads shared libraries from essentially arbitrary directories and so on.

How are security issues prevented in these systems?

It is not by using any "same-author policy"
(no software I know about is doing that),
it is by controlling the set of directories
that the code can be loaded from.

The shell uses $PATH,
the Python interpreter sys.path,
the dynamic linker uses its defaults and the directories configured in /etc/ld.so.conf and so on.

If any of these sets contain a world-writable directory,
the security impact is disastrous and it is a job
of the distribution and of the system administrator
to ensure that this is not the case.
(. got removed from the shell $PATH
to prevent one of such issues.)

Moreover, if higher security is required,
there are usually mechanisms to restrict the set of directories
and prevent altering it.

For the shell, it is the restricted mode
(see "The Restricted Shell" in the Bash manual, especially:

the following are disallowed or not performed:
• Setting or unsetting the values of the ‘SHELL’, ‘PATH’,
‘HISTFILE’, ‘ENV’, or ‘BASH_ENV’ variables.
• Specifying command names containing slashes.
• Specifying a filename containing a slash as an argument
to the ‘.’ builtin command.

).

For the dynamic linker it is the "Secure-execution mode",
see the Secure-execution mode section
of the ld-linux.so(8) manual page.

Basically, this is the observation at
https://github.com/rear/rear/issues/3260#issuecomment-2219801028

Currently the only "line of defense" is the file name.
E.g. /etc/os-release is hopefully safe to be read.

and my point is that this has been actually pretty sufficient
in other software, so why not do the same in ReaR.

So, my proposal is:

Let SourceTrustworthy (or whatever the safe source wrapper
function would be named) check the name of the file to be sourced
and have a list of allowed directories to source from.

Furthermore, I believe that there should be
multiple source wrappers for different use cases.

In particular, one for the shell-style config files (#3203)
that should contain no executable code,
and another for internal ReaR scripts that are called
as part of the ReaR's shell scripting framework.

These are two very different use cases and it makes sense
to keep them separated (for the former we may use
a restricted shell, this makes no sense for the latter).

For the former the set of allowed directories should be
system directories like /etc, /lib and /usr,
while for the latter it should contain $SHARE_DIR.

There may also be a third command to source ReaR's config files,
as this falls into neither category, with yet another set of
trusted directories ($CONFIG_DIR).

Perhaps there could be even a generic source wrapper
for sourcing scripts that are not part of ReaR,
but do not fall under the "shell-style config files" category.

Such details can be added or adjusted according to the needs.

pcahyna commented at 2024-10-01 13:07:

@didacog can you please specify what is the "sourcing remote files" functionality that you need? I am not sure whether it will fall under the same umbrella.

jsmeix commented at 2024-10-01 13:29:

@pcahyna
regarding "sourcing remote files" see
https://github.com/rear/rear/issues/3294

github-actions commented at 2025-02-14 02:35:

Stale issue message

jsmeix commented at 2025-03-17 15:45:

With https://github.com/rear/rear/pull/3424 merged
this issue should be "fixed/solved/done"
at least to some reasonable initial extent.

With https://github.com/rear/rear/pull/3424 merged
there will be regressions when third-party scripts
use '.' for sourcing because currently sourcing via '.'
is always forbidden in ReaR.

I would like to wait a bit and see if sourcing via '.'
is sometimes actually needed in practice.
If sourcing via '.' is sometimes needed,
I will further enhance things to make sourcing via '.'
again possible for the user by explicitly specifying
in another config array which specific file paths
he allows to be sourced via '.' by ReaR.

I could not wait because this regression is a bug in ReaR
because 'source' is not POSIX compliant because
'.' is the POSIX standard command
so ReaR must not forbid using '.' in any case
but allow it when really needed
so I implemented trusted sourcing via '.' in
https://github.com/rear/rear/pull/3434

jsmeix commented at 2025-03-17 15:56:

With https://github.com/rear/rear/pull/3424 merged
my other attempts in my 'jsmeix-SourceTrustworthy'
and 'jsmeix-source-wrapper' branches should now be obsoleted.
I will check those branches if they are completely obsolete
and if yes I will remove them in the next days.

jsmeix commented at 2025-03-18 10:05:

I closed https://github.com/rear/rear/pull/3379
and https://github.com/rear/rear/pull/3258
as superseded by https://github.com/rear/rear/pull/3424
and I deleted the outdated and superseded branches
'jsmeix-source-wrapper' and 'jsmeix-SourceTrustworthy'.


[Export of Github issue for rear/rear.]