[OpenAFS-devel] posix chown again
Michael Meffie
mmeffie@sinenomine.net
Mon, 27 Oct 2008 18:15:53 -0400
Jeffrey Hutzelman wrote:
> --On Monday, October 20, 2008 09:51:15 AM -0400 Michael Meffie
> <mmeffie@sinenomine.net> wrote:
>
>> Since the C acl is documented as having no default meaning,
>> this is conditionally compiled into the fileserver with
>> the --enable-posix-chown option (disabled by default).
>
> As discussed at the recent hackathon, the bit to be used should be
> determined at configure time, rather than being hard coded. This allows
> sites that wish to use this feature to map it onto an ACL bit they are
> not already using. Thus, one would have to configure with an option
> like --enable-posix-chown=C (with legal values being [ABCDEFGH] and
> "no", and maybe even 'a' or 'w', but not "yes").
The attached patch includes the code to set which ACL bit is to
be used. The configure switch has been changed to --enable-permit-chown-acl
which can be used to specify which ACL bit is used and defaults
to disabled. The fileserver logs an additional line on startup to
show which ACL has been set at compile time.
>
>> I have been running this patch against the following tests,
>>
>> 1. Regular users may change ownership of owned files and directories
>> when the w and C ACLs are set.
>>
>> 2. Regular users may not steal ownership of files or directories
>> (regardless of acls).
>>
>> 3. Regular users my change group ownership of owned files when the w
>> and C ACLs are set.
>>
>> 4. Setuid and Setguid mode bits are cleared when a regular user
>> changes ownership or group ownership of a file.
>
> How about:
>
> 5. Regular users may _not_ change ownership of any files or directories
> when the C ACL bit is not set.
>
>
>> Finally, there is an oddity in the current Update_TargetVnodeStatus()
>> function. It seems original intent of the code was to disallow non-admin
>> users to set the suid/sgid bits. However the current implementation
>> also clears the sticky bit as well. Is that a code error, or was
>> disallowing setting of the sticky bit intentional?
>
> It is intentional; the sticky bit has various effects, depending on the
> client platform, which administrators may not want to make available to
> ordinary users. The question of whether the fileserver should continue
> to disallow ordinary users setting this bit is one on which we need to
> have a separate discussion.
Thanks for the info.
--- acinclude.m4 20 Oct 2008 12:35:13 -0000 1.173.2.69
+++ acinclude.m4 27 Oct 2008 01:37:47 -0000
@@ -295,6 +295,12 @@
enabled)])],
,
[enable_optimize_pam="yes"])
+AC_ARG_ENABLE([permit-chown-acl],
+ [AS_HELP_STRING([--enable-permit-chown-acl=@<:@w|i|a|A|B|C|D|E|F|G|H@:>@],
+ [use an ACL to permit chown by non-admin users (defaults to
+ disabled)])],
+ ,
+ [enable_permit_chown_acl="no"])
enable_login="no"
@@ -1586,6 +1592,55 @@
ENABLE_PTHREADED_UBIK=yes
fi
+if test "x$enable_permit_chown_acl" != "x"; then
+ case $enable_permit_chown_acl in
+ w)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 2
+ [define to permit chown by non-admin users])
+ ;;
+ i)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 4,
+ [define to permit chown by non-admin users])
+ ;;
+ a)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 64
+ [define to permit chown by non-admin users])
+ ;;
+ A)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x01000000,
+ [define to permit chown by non-admin users])
+ ;;
+ B)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x02000000,
+ [define to permit chown by non-admin users])
+ ;;
+ C)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x04000000,
+ [define to permit chown by non-admin users])
+ ;;
+ D)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x08000000,
+ [define to permit chown by non-admin users])
+ ;;
+ E)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x10000000,
+ [define to permit chown by non-admin users])
+ ;;
+ F)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x20000000,
+ [define to permit chown by non-admin users])
+ ;;
+ G)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x40000000,
+ [define to permit chown by non-admin users])
+ ;;
+ H)
+ AC_DEFINE(PERMIT_CHOWN_ACL, 0x80000000,
+ [define to permit chown by non-admin users])
+ ;;
+ esac
+fi
+
AC_SUBST(AFS_SYSNAME)
AC_SUBST(AFS_PARAM_COMMON)
AC_SUBST(ENABLE_KERNEL_MODULE)
--- src/viced/afsfileprocs.c 5 Sep 2008 16:57:55 -0000 1.113.2.28
+++ src/viced/afsfileprocs.c 27 Oct 2008 01:38:08 -0000
@@ -923,7 +923,11 @@
if (CHOWN(InStatus, targetptr) || CHGRP(InStatus, targetptr)) {
if (readonlyServer)
return (VREADONLY);
- else if (VanillaUser(client))
+ else if (VanillaUser(client)
+#ifdef PERMIT_CHOWN_ACL
+ && !(rights & PERMIT_CHOWN_ACL) /* ownership already checked */
+#endif
+ )
return (EPERM); /* Was EACCES */
else
osi_audit(PrivilegeEvent, 0, AUD_ID,
@@ -949,7 +953,11 @@
|| CHGRP(InStatus, targetptr)) {
if (readonlyServer)
return (VREADONLY);
- else if (VanillaUser(client))
+ else if (VanillaUser(client)
+#ifdef PERMIT_CHOWN_ACL
+ && (!OWNSp(client,targetptr) || !(rights & PERMIT_CHOWN_ACL))
+#endif
+ )
return (EPERM); /* Was EACCES */
else
osi_audit(PrivilegeEvent, 0, AUD_ID,
@@ -1536,10 +1544,13 @@
if (InStatus->Mask & AFS_SETOWNER) {
/* admin is allowed to do chmod, chown as well as chown, chmod. */
if (VanillaUser(client)) {
- targetptr->disk.modeBits &= ~04000; /* turn off suid for file. */
-#ifdef CREATE_SGUID_ADMIN_ONLY
- targetptr->disk.modeBits &= ~02000; /* turn off sgid for file. */
-#endif
+ /* chown by regular users must clear suid and sgid modebits. */
+ targetptr->disk.modeBits &= ~S_ISUID; /* turn off suid for file. */
+ targetptr->disk.modeBits &= ~S_ISGID; /* turn off sgid for file. */
+ if (InStatus->Mask & AFS_SETMODE) {
+ InStatus->UnixModeBits &= ~S_ISUID;
+ InStatus->UnixModeBits &= ~S_ISGID;
+ }
}
targetptr->disk.owner = InStatus->Owner;
if (VolumeRootVnode(targetptr)) {
--- src/viced/viced.c 24 Sep 2008 21:36:49 -0000 1.75.2.25
+++ src/viced/viced.c 27 Oct 2008 01:38:10 -0000
@@ -1003,6 +1003,49 @@
return mode;
}
+static char *
+ConvertRightsToString(afs_uint32 rights)
+{
+ static char str[16];
+ char *p = str;
+
+ if (rights & PRSFS_READ)
+ *p++ = 'r';
+ if (rights & PRSFS_LOOKUP)
+ *p++ = 'l';
+ if (rights & PRSFS_INSERT)
+ *p++ = 'i';
+ if (rights & PRSFS_DELETE)
+ *p++ = 'd';
+ if (rights & PRSFS_WRITE)
+ *p++ = 'w';
+ if (rights & PRSFS_LOCK)
+ *p++ = 'k';
+ if (rights & PRSFS_ADMINISTER)
+ *p++ = 'a';
+ if (rights & PRSFS_USR0)
+ *p++ = 'A';
+ if (rights & PRSFS_USR1)
+ *p++ = 'B';
+ if (rights & PRSFS_USR2)
+ *p++ = 'C';
+ if (rights & PRSFS_USR3)
+ *p++ = 'D';
+ if (rights & PRSFS_USR4)
+ *p++ = 'E';
+ if (rights & PRSFS_USR5)
+ *p++ = 'F';
+ if (rights & PRSFS_USR6)
+ *p++ = 'G';
+ if (rights & PRSFS_USR7)
+ *p++ = 'H';
+
+ *p = 0;
+ if (!str[0])
+ strcpy(str, "none");
+ return str;
+}
+
/*
* Limit MAX_FILESERVER_THREAD by the system limit on the number of
* pthreads (sysconf(_SC_THREAD_THREADS_MAX)), if applicable and
@@ -1878,6 +1921,7 @@
return code;
}
+
int
main(int argc, char *argv[])
{
@@ -1965,6 +2009,10 @@
ViceLog(0, ("File server starting\n"));
#endif
+#ifdef PERMIT_CHOWN_ACL
+ ViceLog(0, ("Using permit chown acl: %s\n", ConvertRightsToString(PERMIT_CHOWN_ACL)));
+#endif
+
#if defined(AFS_PTHREAD_ENV) && !defined(AFS_NT40_ENV)
/* initialize the pthread soft signal handler thread */
softsig_init();