Re: [PATCH] can_hardlink sysctl ported from FreeBSD

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:
:This is a multi-part message in MIME format.
:--------------080800080400060200050304
:Content-Type: text/plain; charset=ISO-8859-1; format=flowed
:Content-Transfer-Encoding: 7bit
:
:Hi,
:
:I ported the two FreeBSD sysctl's
:
:- security.bsd.hardlink_check_uid
:- security.bsd.hardlink_check_gid
:
:to DragonFly.  If this sysctls are active unprivileged users cannot 
:create hard links to files owned by other users/groups.  I added the 
:sysctl's under kern, not security.bsd ...
:
:Greets
:
:	Matthias
   
    I like the concept, but the (FreeBSD) implementation does not look
    quite correct.

:+	if (suser_cred(cred, PRISON_ROOT) == 0)
:+		return (0);
:+	
:+	if (!hardlink_check_uid)
:+		return (0);

	^^^^^^^^^^^^^^^^^^^^^
	This doesn't make sense to me.  There's no need to check 
	hardlink_check_uid below if we are returning above, but even
	more odd is why are we not allowing hardlink_check_gid to be
	tested when hardlink_check_uid is 0?  


:+	error = VOP_GETATTR(vp, &va, td);
:+	if (error != 0)
:+		return (error);
:+	
:+	if (hardlink_check_uid) {
:+		if (cred->cr_uid != va.va_uid)
:+			return (EPERM);
:+	}
:+	
:+	if (hardlink_check_gid) {
:+		if (!groupmember(va.va_gid, cred))
:+			return (EPERM);
:+	}
:+
:+	return (0);


    I'm thinking we want something like this.  Notice the change I made
    to the hardlink_check_gid code?

	if (suser_cred(cred, PRISON_ROOT) == 0)
		return (0);
	
	error = VOP_GETATTR(vp, &va, td);
	if (error != 0)
		return (error);

	if (hardlink_check_uid) {
		if (cred->cr_uid != va.va_uid)
			return (EPERM);
	}
	
	if (hardlink_check_gid) {
		if (cred->cr_uid != va.va_uid && !groupmember(va.va_gid, cred))
			return (EPERM);
	}


						-Matt