link() security

Hi all,

If this subject has been beaten to death in the past, a link to the discussion is really all I need...

Is there a good reason why a user can successfully link() a file to which they do not have any access?

Here's a scenario:

Many linux systems, like Slackware and SuSE, favor the permissions 1777 for the mail spool directory. This is a good policy from a security perspective, as it prevents mail utilities from requiring SUID/SGID root or mail privileges to create a user's spool and/or lock files.

Now, people often point to several problems with this scheme. Such as the following:

* users can create an unlimited number of files in this directory, potentially using up all availabile inodes

* users can use the spool as scratch space, and potentially fill up the spool filesystem

* users can create links to other users' spool files.

The first two really are very minor problems in comparison to a potential security breach (priviledge elevation), and can be solved with quota and/or site policy. Troublesome users who ignore policy can be dealt with appropriately.

The last one is a bit sticky though. As a regular user, I can create a hard link to someone's spool file, and call it whatever I want. This isn't a major problem, because the resulting link retains the permissions and ownership of the original file; therefore a user who does this can't gain access to anything they didn't already have access to... However, it certainly can constitute an abuse of resources and/or site policy.

There are cases where this can be a security problem though; and what's more, there's no apparent way to identify who created the link, since the link retains ownership of the original file. If, for example, a malicious user noticed that a particular new user's spool file had not been created, he could link(mymailbox, newusermailbox). And now (ignoring locking issues in the spool area), one of two things will happen:

a) if the mail delivery programs run with privileges (despite not needing to with 1777 permissions on the spool), the new user's mail will be delivered to the malicious user's spool.

b) if they don't run with privileges, a mail delivery error should result, alerting the system administrators to the problem.

Either way, this isn't really a great situation. But here's the worst
part: a malicious user can frame someone, say another user they hold a grudge against, as having done this. Since the link() call succeeds regardless of permissions on the file, the malicious user can also call link(framedusermailbox, newusermailbox)! The system administration team has no way to identify who created the file. They can't even look and see who was on at time of creation, because the creation time is the same as that of the original file. Worse still, system administrators who are not aware of this behavior of link() may mistakenly believe that the file was created by the framed user, and recommend expulsion or termination of their employment/access/whatever on that basis.

I've tested that this works on Red Hat 7.1 with pristine 2.4.17 sources and using the ln command. I have not actually written a program that calls link() in this manner. However the ln command is not SUID, so I can imagine no other way this would work...

In my opinion, the solution to this problem is to check that the file being linked is owned by the UID of the calling process, or that the UID is root (or that the process has the appropriate capabilities -- a facility that I'm not overly familiar with). If not, the call to link() should fail with EACCES or EPERM.

Thanks,
Xy

Other than the fact that it's standard behaviour? ;-) Well, the SUS actually makes an allowance for this:

"The implementation may require that the calling process has permission to access the existing file."

If you are interested, the Openwall patch does just this (among other things)
http://openwall.com. Work based on Solar Designer's Openwall patch has been brought forward to more recent 2.4 and 2.5 kernels. Both the following projects implement the Openwall secure link feature:

http://grsecurity.net
http://lsm.immunix.org

This can break some applications that make assumptions wrt. link(2) (Courier MTA for example).

cheers,
-chris

Perhaps two options: one to allow creation of the link only when the UIDs match; and the other to allow the link when GIDs match, to keep Courier happy?

How practical is it to make this a mount option and to do so cleanly ?

Well, it's not too bad. Below is a patch (albeit quick and dirty) that adds a MS_SECURE_LINK mount option to enforce this behaviour. To simplify testing, I cheated and piggy backed the MS_SECURE_LINK option on the MS_NOSUID option. So a simple 'mount -o nosuid,remount /foo' enables this. The secure link test allows a hard link if you own the target file or are in the same group _and_ can (by group) write to the file, or have proper capability, of course. Patch is against 2.4.19-pre5-ac3.

cheers,
-chris

@@ -707,8 +707,10 @@
 		return -EINVAL;
 
 	/* Separate the per-mountpoint flags */
-	if (flags & MS_NOSUID)
+	if (flags & MS_NOSUID) {
 		mnt_flags |= MNT_NOSUID;
+		flags |= MS_SECURE_LINK;
+	}
 	if (flags & MS_NODEV)
 		mnt_flags |= MNT_NODEV;
 	if (flags & MS_NOEXEC)

@@ -1614,6 +1614,11 @@
 	error = -EPERM;
 	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
 		goto exit_lock;
+	if (IS_SECURE_LINK(inode) && (!capable(CAP_FOWNER) &&
+				current->fsuid != inode->i_uid &&
+			       	(current->fsgid != inode->i_gid ||
+				!(inode->i_mode & S_IWGRP))))
+		goto exit_lock;
 	if (!dir->i_op || !dir->i_op->link)
 		goto exit_lock;
 

@@ -111,6 +111,7 @@
 #define MS_MOVE		8192
 #define MS_REC		16384
 #define MS_VERBOSE	32768
+#define MS_SECURE_LINK  65536
 #define MS_ACTIVE	(1<<30)
 #define MS_NOUSER	(1<<31)
 

@@ -118,7 +119,7 @@
  * Superblock flags that can be altered by MS_REMOUNT
  */
 #define MS_RMT_MASK	(MS_RDONLY|MS_SYNCHRONOUS|MS_MANDLOCK|MS_NOATIME|\
-			 MS_NODIRATIME)
+			 MS_NODIRATIME|MS_SECURE_LINK)
 
 /*
  * Old magic mount flag and mask

@@ -161,6 +162,7 @@
 #define IS_IMMUTABLE(inode)	((inode)->i_flags & S_IMMUTABLE)
 #define IS_NOATIME(inode)	(__IS_FLG(inode, MS_NOATIME) || ((inode)->i_flags & S_NOATIME))
 #define IS_NODIRATIME(inode)	__IS_FLG(inode, MS_NODIRATIME)
+#define IS_SECURE_LINK(inode)	__IS_FLG(inode, MS_SECURE_LINK)
 
 #define IS_DEADDIR(inode)	((inode)->i_flags & S_DEAD)
 

Actually, that is a horrible policy from a security perspective. The shared mail spool itself is a poor design and always has been.

A better design is to use a separate spool directory for each user (/var/spool/mail/user/ or ~user/mail/ or somesuch), and only allow that user to access it at all. This solves *all* of the security problems you mention:

*) It avoids attacks based on race conditions, because you cannot
create files in somebody else's spool.

*) Admins can manage space with quotas or partitions just like they
do for user home directories (i.e., it is a solved problem).

*) You cannot link() to somebody else's spool file because you
cannot even read the directory in which it resides.

The solution to a fundamentally broken spool design is to fix that design, not to patch the kernel in nonstandard ways to plug just one of its multiple flaws.

And yes, there are MTAs which use a directory per user by default. Fix your MTA, do not hack the kernel.

All just My Opinion, of course.

- Pat

Followup to:  <[email protected]>
By author:    "Patrick J. LoPresti" <[email protected]>
In newsgroup: linux.dev.kernel

Not to mention the fact that the single file mailbox design is itself flawed. Mailboxes are fundamentally directories, which news server authors quickly realized.

-hpa

<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<[email protected]>

And then unrealized when they hit performance limitations. Its a trade off and one that most news systems seem to prefer to use a custom database for

Well, a database is basically a custom filesystem.

-hpa

I would have to disagree. There are fundamentally different transaction semantics between the two as well as indexing constraints. I can't for example find commit() and rollback() in posix.1 8)

OK, perhaps I should have been more explicit...

A filesystem is *one kind* of database.

The operations that various databases implement differ -- not all databases have commit()/rollback(), nor do all of them implement relationals, object linking, etc.

The point was mostly that storing mail in a (basically) unstructured flat-file format isn't really consistent with the operations you want to perform on it. I didn't mean the directory/file format was necessarily the ultimate solution, only that (a) it works better than mbox, (b) it's been around for a long time.

-hpa

Scribbling feverishly on April 15, H. Peter Anvin managed to emit:

[schnip]

[nod]

I suppose it made sense back when we made our 1s and 0s by hand, but whoever (would dare) take credit for "designing" the UNIX mbox "format" needs a remedial course if file format design.

K

Are you ever going to do the dishes?  Or will you change your major to biology?

I'll agree with the above, however consider that there are other reasons to have drwxrwxrwt directories besides a mail spool. My point was not that link() should be modified because it makes mail spools that use this feature less secure; my point was that (IMO) link should be modified because it does not make sense to allow users to create hard links to files they have no access to, in general. The mail spool example was simply one common example.

IMO, if I have created a file, and I own the file, then there are only two users who should get to decide whether that file gets deleted or
not: me, and root. Regular users should not be able to create hard links to my files, potentially without me knowing about it. Allowing them to do so means that you allow users who do not own a resource, and have no access to that resource, to potentially manage control of that resource to some extent. I don't see how this policy makes any sense. It allows that a file I created may be hanging around despite the fact that I think it's been deleted. And that just seems like a very bad idea to me.

Rephrased, your argument is basically that it is unwise to continue a behavior which is fundamentally flawed just because it is a standard behavior. That is precisely my argument WRT the current behavior of link().

Ditto. :)

Xy

It is a bad idea, especially since POSIX doesn't require it! The 2001 UNIX/POSIX standard says:

The implementation may require that the calling process has permission to access the existing file.

Then for error codes, EACCES means:

A component of either path prefix denies search permission, or the requested link requires writing in a directory that denies write permission, or the calling process does not have permission to access the existing file and this is required by the implementation.

Maybe this should even apply to the owner. ("chmod 000 foo") Let's say that either read or write permission is enough by default. By mount option, both or neither may be required.

Pretty much any other restriction is OK as well, due to the way the UNIX/POSIX standard uses the term "access". For example:

An implementation may include other security mechanisms in addition to those specified in POSIX.1, and an access attempt may fail because of those additional mechanisms, even though it would succeed according to the rules given in this section. (For example, the user's security level might be lower than that of the object of the access attempt.)

The standard defines an "additional file access control mechanism" as pretty much any restriction that one would care to add.

So restricting link() is 100% standards compliant.

How about: fail if (ctime^mtime+atime*owner)%3

:-)