Hi David, On Tue, Sep 05, 2017 at 10:54:55AM +0100, David Howells wrote: > Eric Biggers <ebiggers3@xxxxxxxxx> wrote: > > > 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. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
