[Rennie deGraaf]

When cryptmount mounts a filesystem, the path names for
the device and mount point that go into /etc/mtab are
canonicalized. When unmounting, is_mounted() compares
the original paths stored in configuration to the
canonicalized versions in mtab; if the two are
different, then unmounting fails.

See support request 1543189
(http://sourceforge.net/tracker/index.php?func=detail&aid=1543189&group_id=154099&atid=790423)
for more details on this issue.

The attached patch against cryptmount-1.1 canonicalizes
all paths in configuration while loading it. If any
errors are encountered while canonicalizing (the path
doesn't exist, it can't be accessed, etc.), then the
offending configuration entry will be deleted. Besides
the 'dev' and 'dir' configuration entries, I also
canonicalize the 'loop' and 'keyfile' paths, just in
case there are any related issues hiding in there.

The patched version works on my system (Fedora Core 5
Linux, running kernel 2.6.17-1.2174_FC5 with SELinux
disabled on an AthlonXP), but I have not checked it for
portability, nor have I made any attempt to
internationalize the error messages.

Rennie deGraaf

Patch against cryptmount-1.1 to canonicalize paths before use

Logged In: YES
user_id=1226045

Thanks for submitting the patch.
Unfortunately, I don't think this is a good solution as it
is based on the 'realpath()' system-call.

The manual pages on my system declare that realpath() "is
broken by design", and while the version in glibc-2.3.4 is
almost certainly pretty solid (and supports the use of a
NULL second argument), it is declared to be either a BSD or
XOPEN_EXTENDED extension, which doesn't suggest that it's a
very portable routine. In addition the realpath.c file in
util-linux-12q includes another implementation of realpath
on the grounds that "the libc version has some security
flaws". This all suggests that using realpath() is not a
good move.

I suspect a better solution might simply be to adjust the
'is_mounted()' routine within cryptmount so that it doesn't
check BOTH the device & mount-point fields returned by
getmntent(), or to advise users ONLY to use canonicalized
paths within the cmtab. I suspect the latter would avoid
risks of opening extra security loopholes for little
compelling benefit.

• priority: 5 --> 3
• status: open --> pending-rejected

Logged In: YES
user_id=1226045

I've attached a patch that alters the tests in is_mounted()
to avoid checking the mount-point returned by getmntent()
against that in the cmtab. Hopefully this will make the
problem go away, without significantly reducing robustness
or security.

Patch against cryptmount-1.1 for is_mounted() to mitigate pathname canonicalization issue

Logged In: YES
user_id=707284

You're right that people aren't likely to put '.', '..' or
double slashes in paths (unless you ever add something like
a "base path" option or home directory-relative paths), and
the trailing slash is easily stripped. However, the
existing code /also/ fails if there is a symbolic link in
the paths of either the image or the mount point - and
that's a case that is much more likely to appear and cannot
be detected and corrected so easily. Checking only one of
the two paths when unmounting (as your patch does) won't
help here.

What would you think of adding a safe re-implementation of
realpath() to your source tree? I could whip together
another patch if you'd like. Alternately, even an autoconf
check to use realpath() on systems which have a safe
implementation, plus a documentation update, would be better
than nothing.

By the way, the security flaws in realpath() exist only when
using a non-null 2nd argument. (Of course, this is
non-standard and not supported by all systems.) If the
second argument is used, then realpath() is prone to buffer
overflows, since there is no way to predict an upper bound
on a canonicalized path. The version in util-linux adds a
buffer length argument to make it safe to use with a
pre-allocated buffer. There's another always-safe
realpath() replacement out there as a GNU libc extension
called canonicalize_file_name(); this one only takes one
argument and always malloc()s the result.

• priority: 3 --> 5
• status: pending-rejected --> open

• priority: 5 --> 3
• status: open --> open-postponed

Logged In: YES
user_id=1226045

I take your point about symbolic links, however from my
experiments, the one-line patch that I've already attached
to this bug-report (02Sep06) deals perfectly well with
mount-points that contain symbolic links (as well as './'
'//' oddities). This only seems to leave the issue of
/dev/mapper/[target-name] containing symbolic links, which
seems rather less likely to occur than for mount-directories.

Thanks for the offer of a realpath() patch. However, this
sounds like it's based on a fundamentally dubious approach -
trying to emulate the canonicalization performed by
util-linux/realpath.c through a routine that would be
written/maintained completely independently.

I'd recommend that a period of quiet contemplation is needed
to identify a more portable and maintainable solution that
goes beyond the patch I've already suggested. I believe this
patch already considerably reduces the possible impact of
the canonicalization issue.

Thanks.

• status: open-postponed --> closed-fixed

This design is spectacular! You most certainly know how to keep a reader entertained. Between your wit and your videos, I was almost moved to start my own blog (well, almost...HaHa!) Fantastic job. I really loved what you had to say, and more than that, how you presented it. Too cool!
north face jackets clearance http://northfacecoatoutlet.hpage.com/