The function AccessOK is used by all of ptserver RPC handlers that
need to do an authorization check, and the last two arguments are set
as such:
- When adding a member to a group, 'mem' is PRP_ADD_MEM and 'any' is
PRP_ADD_ANY
- When removing a member from a group, 'mem' is PRP_REMOVE_MEM and
'any' is 0
- When modifying an entry (setFieldsEntry) or modifying some global
database fields, 'mem' and 'any' are both set to 0
- When reading an entry and not modifying it, 'mem' and/or 'any' are
set to other values (depending on if we're checking membership,
examining the entry itself, etc)
Commit
93ece98c (ptserver-restricted-mode-
20050415) added a check to
AccessOK to make it return false for -restricted mode when we are
adding a member to a group, or when 'mem' and 'any' are both 0. This
didn't catch the case when we are removing a member from a group,
though, when 'mem' is PRP_REMOVE_MEM.
It looks like commit
a614a8d9 (ptutils-restricted-accessok-
20081025)
tried to fix this by adding a check for PRP_REMOVE_MEM, but it also
required 'any' to be set to 0 for the conditional to succeed. This is
true when removing a member from a group, but when adding a member to
a group, 'any' is PRP_ADD_ANY, and so this check fails.
This means that currently, when restricted mode is turned on,
non-admins can still run addToGroup and setFieldsEntry successfully.
Fix this by checking for PRP_ADD_MEM/PRP_REMOVE_MEM separately from
checking if 'mem'/'any' are set to 0. Break up this conditional into
separate if() statements with comments to try to make the checks
more clear.
Change-Id: I7e647865b772c42e70014f48ce9cd53ef511cd5b
Reviewed-on: https://gerrit.openafs.org/13370
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>