Re: Missing warning on revokes with grant options

On Thu, May 18, 2023 at 7:17 PM Joseph Koshakow <koshy44(at)gmail(dot)com> wrote:
>
> I looked into this function and that is correct. We fail to find a
> match for the revoked privilege here:
>
> /*
> * Search the ACL for an existing entry for this grantee and grantor. If
> * one exists, just modify the entry in-place (well, in the same
position,
> * since we actually return a copy); otherwise, insert the new entry at
> * the end.
> */
>
> for (dst = 0; dst < num; ++dst)
> {
> if (aclitem_match(mod_aip, old_aip + dst))
> {
> /* found a match, so modify existing item */
> new_acl = allocacl(num);
> new_aip = ACL_DAT(new_acl);
> memcpy(new_acl, old_acl, ACL_SIZE(old_acl));
> break;
> }
> }
>
> Seeing that there was no match, we add a new empty privilege to the end
> of the existing ACL list here:
>
> if (dst == num)
> {
> /* need to append a new item */
> new_acl = allocacl(num + 1);
> new_aip = ACL_DAT(new_acl);
> memcpy(new_aip, old_aip, num * sizeof(AclItem));
>
> /* initialize the new entry with no permissions */
> new_aip[dst].ai_grantee = mod_aip->ai_grantee;
> new_aip[dst].ai_grantor = mod_aip->ai_grantor;
> ACLITEM_SET_PRIVS_GOPTIONS(new_aip[dst],
> ACL_NO_RIGHTS, ACL_NO_RIGHTS);
> num++; /* set num to the size of new_acl */
> }
>
> We then try and revoke the specified privileges from the new empty
> privilege, leaving it empty (modechg will equal ACL_MODECHG_DEL here):
>
> old_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
> old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
>
> /* apply the specified permissions change */
> switch (modechg)
> {
> case ACL_MODECHG_ADD:
> ACLITEM_SET_RIGHTS(new_aip[dst],
> old_rights | ACLITEM_GET_RIGHTS(*mod_aip));
> break;
> case ACL_MODECHG_DEL:
> ACLITEM_SET_RIGHTS(new_aip[dst],
> old_rights & ~ACLITEM_GET_RIGHTS(*mod_aip));
> break;
> case ACL_MODECHG_EQL:
> ACLITEM_SET_RIGHTS(new_aip[dst],
> ACLITEM_GET_RIGHTS(*mod_aip));
> break;
> }
>
> Then since the new privilege remains empty, we remove it from the ACL
> list:
>
> new_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
> new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
>
> /*
> * If the adjusted entry has no permissions, delete it from the list.
> */
> if (new_rights == ACL_NO_RIGHTS)
> {
> memmove(new_aip + dst,
> new_aip + dst + 1,
> (num - dst - 1) * sizeof(AclItem));
> /* Adjust array size to be 'num - 1' items */
> ARR_DIMS(new_acl)[0] = num - 1;
> SET_VARSIZE(new_acl, ACL_N_SIZE(num - 1));
> }

Sorry about the unformatted code, here's the entire quoted section
again with proper formatting:

I looked into this function and that is correct. We fail to find a
match for the revoked privilege here:

/*
* Search the ACL for an existing entry for this grantee and grantor. If
* one exists, just modify the entry in-place (well, in the same
position,
* since we actually return a copy); otherwise, insert the new entry at
* the end.
*/

for (dst = 0; dst < num; ++dst)
{
if (aclitem_match(mod_aip, old_aip + dst))
{
/* found a match, so modify existing item */
new_acl = allocacl(num);
new_aip = ACL_DAT(new_acl);
memcpy(new_acl, old_acl, ACL_SIZE(old_acl));
break;
}
}

Seeing that there was no match, we add a new empty privilege to the end
of the existing ACL list here:

if (dst == num)
{
/* need to append a new item */
new_acl = allocacl(num + 1);
new_aip = ACL_DAT(new_acl);
memcpy(new_aip, old_aip, num * sizeof(AclItem));

/* initialize the new entry with no permissions */
new_aip[dst].ai_grantee = mod_aip->ai_grantee;
new_aip[dst].ai_grantor = mod_aip->ai_grantor;
ACLITEM_SET_PRIVS_GOPTIONS(new_aip[dst],
ACL_NO_RIGHTS, ACL_NO_RIGHTS);
num++; /* set num to the size of new_acl */
}

We then try and revoke the specified privileges from the new empty
privilege, leaving it empty (modechg will equal ACL_MODECHG_DEL here):

old_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);

/* apply the specified permissions change */
switch (modechg)
{
case ACL_MODECHG_ADD:
ACLITEM_SET_RIGHTS(new_aip[dst],
old_rights | ACLITEM_GET_RIGHTS(*mod_aip));
break;
case ACL_MODECHG_DEL:
ACLITEM_SET_RIGHTS(new_aip[dst],
old_rights & ~ACLITEM_GET_RIGHTS(*mod_aip));
break;
case ACL_MODECHG_EQL:
ACLITEM_SET_RIGHTS(new_aip[dst],
ACLITEM_GET_RIGHTS(*mod_aip));
break;
}

Then since the new privilege remains empty, we remove it from the ACL
list:

new_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);

/*
* If the adjusted entry has no permissions, delete it from the list.
*/
if (new_rights == ACL_NO_RIGHTS)
{
memmove(new_aip + dst,
new_aip + dst + 1,
(num - dst - 1) * sizeof(AclItem));
/* Adjust array size to be 'num - 1' items */
ARR_DIMS(new_acl)[0] = num - 1;
SET_VARSIZE(new_acl, ACL_N_SIZE(num - 1));
}

Thanks,
Joe Koshakow