Hello again Ingo.
On Mon, Jul 25, 2016 at 10:13:29AM +0200, Ingo wrote:
> Am 24.07.2016 um 22:07 schrieb Andreas Henriksson:
> >
> > Are you sure this is the correct syntax? I would expect that you
> > should specify the mountpoint (target directory) rather than the
> > source of the mount. eg. mount /home/ingo/leo.Bilder
> > Do using that still give you the same problem?
>
> Great, at least that works as expected if target directory is used.
>
> But "man mount" explicitely states:
[...]
You're right, the manpage explicitly says in multiple places
that what you're doing should work... I'm still thinking
using the mountpoint is always preferrable/recommended though. ;)
I got a chance to discuss this with upstream and I'll try to summarize
some of the useful information here for the record (and as a personal
reminder for the future):
In sys-utils/mount.c in the main() function there's this code
that catches when you only provide one argument as in e.g.
"mount leo:/Bilder" :
} else if (argc == 1 && (!mnt_context_get_source(cxt) ||
!mnt_context_get_target(cxt))) {
...
else
mnt_context_set_target(cxt, argv[0]);
Which means the argument you provide (leo:/Bilder) always ends up
in the "target" field, but there's code in libmount which handles
trying it both as the source and target for fstab matching in
libmount/src/context.c apply_table() function:
if (!fs && mnt_context_is_swapmatch(cxt)) {
...
if (!fs && tgt)
fs = mnt_table_find_source(tb, tgt, direction);
Unfortunately when you run the mount command as non-root you never
get that far, because you get stuck much earlier where there's
security sensitive code that tries to prevent you from using mount
to check if directories exists which your user has no access to check.
See again sys-utils/mount.c main() function:
if (mnt_context_is_restricted(cxt))
sanitize_paths(cxt);
The sanitize_paths() will call into canonicalize_path_restricted()
and error out when the path you gave can't be expanded using
realpath().
The canonicalize_path_restricted() function can be found in
lib/canonicalize.c.
The seteuid/setegid calls in canonicalize_path_restricted() can
be seen in the strace output (followed by the error message):
...
| 09:54:06.494259 getgid() = 1000
| 09:54:06.494287 setresgid(-1, 1000, -1) = 0
| 09:54:06.494318 getuid() = 1000
| 09:54:06.494345 setresuid(-1, 1000, -1) = 0
| 09:54:06.494384 getcwd("/home/ingo", 4096) = 11
| 09:54:06.494415 lstat("/home/ingo/leo:", 0x7ffca08ba150) = -1 ENOENT (No such
file or directory)
| 09:54:06.494450 setresgid(-1, 1000, -1) = 0
| 09:54:06.494478 setresuid(-1, 1000, -1) = 0
| 09:54:06.494515 write(2, "mount: ", 7mount: ) = 7
| 09:54:06.494547 write(2, "leo:/Bilder", 11leo:/Bilder) = 11
| 09:54:06.494577 write(2, ": ", 2: ) = 2
...
| 09:54:06.495331 write(2, "Datei oder Verzeichnis nicht gef"..., 38Datei
| oder Verzeichnis nicht gefunden
| ) = 38
...
(Btw, the strace you provided proved to be very useful!)
(The reason this all works when running as root is because then
sanitize_paths is not called at all because of the condition
on mnt_context_is_restricted(cxt) in that case.)
Despite knowing where we stumble, it's not easy to come up with a solution
that suites both the people who wants to avoid path disclosure and your
usecase. Given you now know about using the mountpoint (which also
upstream said is really the way to go when specifying mounts, rather
than the source) would you agree that this isn't strictly Release Critical
severity anymore?
Hopefully everything works as you expect it to when you mount as non-root
using "mount /mountpoint" or did you see any additional problems with that?
(Btw, if you're seeing problems with umount maybe this recent post on
the upstream mailing list and the followup is interesting to you:
http://www.spinics.net/lists/util-linux-ng/msg13078.html )
Upstream mentioned further investigating this problem is now on his TODO.
Regards,
Andreas Henriksson
PS. fwiw, the mount utility in >= Jessie is a complete rewrite compared to
the one shipped in <= Wheezy. It's now based on the new libmount which is
why the new utility is smaller than the old one (not counting size of
dynamically linked libraries).
