From dfc78d533ef64c8d6daf134e2a0f67c5c16f7369 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Tue, 30 Oct 2018 14:29:24 -0500 Subject: [PATCH] ptserver: Fix AccessOK -restricted for addToGroup 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 Reviewed-by: Mark Vitale Reviewed-by: Benjamin Kaduk --- src/ptserver/ptutils.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/ptserver/ptutils.c b/src/ptserver/ptutils.c index 511f57a..7ea7202 100644 --- a/src/ptserver/ptutils.c +++ b/src/ptserver/ptutils.c @@ -286,8 +286,17 @@ AccessOK(struct ubik_trans *ut, afs_int32 cid, /* caller id */ return 1; if (cid == SYSADMINID) return 1; /* special case fileserver */ - if (restricted && ((mem == PRP_ADD_MEM) || (mem == PRP_REMOVE_MEM)) && (any == 0)) - return 0; + if (restricted) { + if (mem == PRP_ADD_MEM || mem == PRP_REMOVE_MEM) { + /* operation is for adding/removing members from a group */ + return 0; + } + if (mem == 0 && any == 0) { + /* operation is for modifying an entry (or some administrative + * global operations) */ + return 0; + } + } if (tentry) { flags = tentry->flags; oid = tentry->owner; -- 1.9.4