Hi David,

On Tue, Sep 05, 2017 at 10:54:55AM +0100, David Howells wrote:

Eric Biggers [off-list ref] wrote:

quoted

This patch fixes the flaw by requiring both Search and Setattr permission to
invalidate a key rather than just Search permission.

I'm not sure I agree.  The problem is that you then have to grant someone
Setattr permission for them to be able to do this, which then opens up a whole
host of other things they can also do.

True, but Setattr permission has already been overloaded to allow several
different types of modifications, and it makes *much* more sense than Search
permission which should not allow any modifications.  And in practice I expect
people care more about whether modifications are permitted or not, than the
details of the finer-grained permissions.

quoted

Requiring Setattr permission is appropriate because Setattr permission also
allows revoking (via keyctl_revoke()) or expiring (via keyctl_set_timeout())
the key, which also make the key inaccessible regardless of how many
keyrings it is in.

Note that setting the expiry time is not really equivalent to revokation in
this regard as setting the expiry time is something you do when setting up a
key, whereas revokation is something you do later to kill a key off.

Sort of, but actually keyctl_set_timeout() can be called at any time, and the
timeout can be set to as little as 1 second.  So I don't see how keyctl_revoke()
is that much different, fundamentally.

How about another solution:

 (1) I add a flag to a key to say that it can be invalidated and a keyctl to
     change that flag.

And who would have permission to change that flag?  It seems to be the same
problem again.

 (2) I add a new key type op called ->allow_invalidation() that allows key
     types to govern separately who is allowed to invalidate keys of that
     type.

     So, for instance, DNS record invalidation would require CAP_NET_ADMIN.

What would the behavior be if ->allow_invalidation() was not supplied?  In other
words, would the purpose of this be to lock down invalidation of dns_resolver
keys, or to restrict invalidation to *only* dns_resolver keys?

 (3) Allow keyrings to be cleared by users who don't have Write permission but
     do have other permission, such as CAP_NET_ADMIN.  This would need to be
     granted on a per-keyring basis.

Granted by who, and how?  And do you mean keyctl_clear(), or
keyctl_invalidate()?

- Eric