It all depends what the desired behavior is. When a rootdir is set, do
we want any exports to possibly fall outside the rootdir path? If no, it
is beneficial to use this approach since resolving paths within chrooted
env will fail if the path does not exist within the chroot. Allowing for
a "sandboxing" mechanism of sort. But with this approach you have
proposed, the issue might arise where someone with access to an nfs
export can create a symlink to say "/". In that scenario the kernel will
end up exporting the system root. Which can be bad. You could of course
do as you have stated and check if it is within the boundaries of the
rootdir, but it would result to the same thing. I think the chrooted
thread that can be repurposed is the best approach here.
The majority of the patch is simplifying what I believed to have been an
unmaintainable approach of spawning tasks within the worker thread is
created to run chrooted.
NeilBrown wrote:
(adding back steved and linux-nfs)
On Wed, 08 Jan 2025, Christopher Bii wrote:
I have a $rootdir set.
I export $rootdir/$export_entry
If /$export_entry exists. It will take whatever path that is and prepend
it to rootdir as the export passed to the kernel.
$ rootdir="/export"
$ exportentry_1="/my_export"
# exportutils takes the path at exportentry_1, runs it through
# real_path() and concatenates the result to $rootdir. If it fails and
# the path doesn't exist. It will still concatenate whatever the entry
# was. So in the event realpath($exportentry_1) == NULL.
# $rootdir/$exportentry_1 will be sent to the kernel as the path to
# export. But take this situation
$ file $exportentry_1
> /my_export: symbolic link to "../../../"
# realpath($exportentry_1) == "/". So the entry passed to the kernel is
# $rootdir/
Is it clearer?
Maybe - thanks.
You say
"exportutils takes the path at exportentry_1, runs it through
# real_path() and concatenates the result to $rootdir."
assuming that is correct (I haven't checked the code but have no reason
to doubt you), then it seems that the fix is to prepend $rootdir
*before* running it through real_path() - then checking it still starts
with $rootdir.
Would that fix the problem?
Your patch set seems much more complex than that.
thanks,
NeilBrown
NeilBrown wrote:
On Wed, 08 Jan 2025, Christopher Bii wrote:
Hello,
You are correct, it would be a configuration error. But the issue is
that when a rootdir is set, export entries are assumed to be relative to
the rootdir path, but this isn't the case.
The above statement directly contradicts the documentation which says
that all exports *are* relative to rootdir (more accurately: the path is
prefixed with the rootdir). So if true it is clearly a bug that needs
to be fixed, and would have nothing to do with symlinks.
But I don't think it is true.
What you have been saying is that if the export point - which is at
$rootdir/$exportpath - is a symlink, then that symlink is resolved
without reference to the rootdir. This is true but I don't see why it
is a problem.
When you create /etc/exports (or run exportfs) you should only identify
directories, never symlinks. And all ancestors of these directories
should only be modifiable by privileged processes.
What is your use case for exporting a symlink, or exporting anything in
a directory that is not restricted to privileged users?
Thanks,
NeilBrown
So my rootdir can have a
restrictive set of permissions, but the export entry path relative to
the system* rootdir may have an entirely different set of permissions.
Or even with restrictive permissions it could be accidentally or
maliciously symlinked.
Christopher Bii
NeilBrown wrote:
On Sat, 07 Dec 2024, Christopher Bii wrote:
Hello,
It is hinted in the configuration files that an attacker could gain access
to arbitrary folders by guessing symlink paths that match exported dirs,
but this is not the case. They can get access to the root export with
certainty by simply symlinking to "../../../../../../../", which will
always return "/".
This is due to realpath() being called in the main thread which isn't
chrooted, concatenating the result with the export root to create the
export entry's final absolute path which the kernel then exports.
PS: I already sent this patch to the mailing list about the same subject
but it was poorly formatted. Changes were merged into a single commit. I
have broken it up into smaller commits and made the patch into a single
thread. Pardon the mistake, first contribution.
I'm still not convinced there is a vulnerability here, but I might have
missed part of the conversation...
Could you please spell out in detail the threat scenario that we are
trying to defend against?
From my perspective: if you export a path that a non-privileged user can
modify, then that is a configuration error. We should not try to make
it "safe". We could possibly try to detect that situation and fail the
export when it happens.
Why is that perspective not correct? If this has already been
discussed, please point me to the relevant email.
Thanks,
NeilBrown
