Re: [PATCH v2] KEYS: make keyctl_invalidate() also require Setattr permission

[David Howells <dhowells@redhat.com>]

Eric Biggers <ebiggers3@gmail.com> wrote:

> True, but Setattr permission has already been overloaded to allow several
> different types of modifications,

Perhaps therein lies the problem.  Setattr is too overloaded and really needs
splitting along the following lines:

 (1) Security: ownership, access, security label, restrictions.

 (2) Content: alter/update, timeout.

 (3) Revocation and invalidation.

Maybe I should create some sort of ACL for keys and map setperm onto that.

I wonder if this could also intersect with user namespaces in some way so that
you can make an ACL that has users from multiple namespaces - might be tricky,
though.

> 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.

I disagree still.  To allow users to invalidate a key you would *also* have to
give them the ability to muck around with the permissions.

> 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.

The timeout is a property of the key content and revoked is one of the states
the key can be in.

> >  (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.

No.  Setperm would be required to change the flag, but not to apply the
invalidate operation if the flag is set.  Think of Invalidate as being a
mass-unlink operation effected by the garbage collector.

Kernel-created DNS keys, for example, don't grant Setperm to anyone.

> What would the behavior be if ->allow_invalidation() was not supplied?

The obvious would be to check that you're the owner of the key.

> 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?

To restrict who could invalidate keys of a particular type.  Actually, it
wants to be per-key not per-key-type.  Hmmm...

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

keyctl_clear().  Empty the keyring, not render it unusable.


David