[RFC] KEYS: Replace uid/gid/perm permissions checking with ACL

David Howells <dhowells@redhat.com> wrote:

> Here's a patch that partially implements ACLs on keys.  I haven't yet done
> KEYCTL_[GS]ET_ACL, but KEYCTL_SETATTR, KEYCTL_DESCRIBE and /proc/keys have
> been modified to deal with it.
> 
> Most of the keyutils testsuite passes, just the permissions tests that don't
> because there isn't a fully compatible mapping from SETATTR -> ACL ->
> DESCRIBE.

Here's an updated patch.  It still doesn't implement GET/SET_ACL, but it
passes almost all the tests in the keyutils testsuite, barring:

 (1) tests/keyctl/dh_compute/badargs: The first wrong-key-type test now
     returns EOPNOTSUPP rather than ENOKEY as READ permission isn't removed
     if the type doesn't have ->read().  You still can't actually read the
     key.

 (2) tests/keyctl/permitting/valid: The view-other-permissions test doesn't
     work as Other has been replaced with Everyone in the ACL.

All other tests still work.
---
commit 2b5fa6ac075006f129b376886a5e8b41d47efad1
Author: David Howells <dhowells@redhat.com>
Date:   Tue Sep 26 09:45:26 2017 +0100

    KEYS: Replace uid/gid/perm permissions checking with ACL

    Replace the uid/gid/perm permissions checking on a key with an ACL to allow
    the SETATTR permission to be split.  The problem is that SETATTR covers a
    slew of things, not all of which should be grouped together.  This
    includes:

     (1) Changing the key ownership.

     (2) Changing the security information.

     (3) Keyring restriction.

     (4) Expiry time.

     (5) Revocation.

    and it has also been proposed to add:

     (6) Invalidation.

    The above can be divided into three groups: Controlling access (1), (2) and
    (3), managing the content at construction time (4) and managing the key (5)
    and (6).

    Add an ACL set & get functions that use a list of elements of the following
    form:

            struct key_ace {
                    union {
                            uid_t           uid;
                            gid_t           gid;
                            unsigned int    special_id;
                    };
                    unsigned int mask;
            };

    where the top four bits of mask indicate the subject being checked:

            KEY_ACE_SPECIAL         A specific value
            KEY_ACE_UID             A nominated UID
            KEY_ACE_GID             A nominated GID

    and if SPECIAL is specified, special_id is one of the following:

            KEY_ACE_POSSESSOR       The possessor of the key
            KEY_ACE_OWNER           The owner of the key
            KEY_ACE_GROUP           The key's group
            KEY_ACE_OTHER           Everyone else

    Each special ID may only occur once in an ACL.

    The bottom 28 bits of mask indicate the permissions being granted:

            KEY_ACE_VIEW            Can view the key metadata
            KEY_ACE_READ            Can read the key content
            KEY_ACE_WRITE           Can update/modify the key content
            KEY_ACE_SEARCH          Can find the key by searching/requesting
            KEY_ACE_LINK            Can make a link to the key
            KEY_ACE_INVAL           Can invalidate
            KEY_ACE_REVOKE          Can revoke
            KEY_ACE_SET_SECURITY    Can set security
            KEY_ACE_JOIN            Can join this keyring
            KEY_ACE_CLEAR           Can clear this keyring

    The ACEs are purely additive; all ACEs that apply are considered.  I figure
    that it's unlikely that you'll give the key's Group permissions that you
    wouldn't give the Owner and Other permissions that you wouldn't give to both
    Group and Owner.  Possessor permissions are already additive.

    The functions are:

            keyctl(KEYCTL_GET_ACL, key_serial_t key,
                   struct key_ace *acl_buffer, unsigned int buflen);

            keyctl(KEYCTL_SET_ACL, key_serial_t key,
                   const struct key_ace *acl, unsigned int len);

    The KEYCTL_SETATTR function is then deprecated.  If called, it will
    construct an ACL to reflect the mask it is given, using possessor, owner,
    group and other ACE's as appropriate if any of those elements are granted
    any permissions.  SETATTR permission turns on all of INVAL, REVOKE and
    SET_SECURITY.  WRITE permission turns on WRITE, REVOKE and, if a keyring,
    CLEAR.  JOIN is turned on if a keyring is being altered.  KEYCTL_SETATTR
    will return an error if SETACL has been called on a key.

    The KEYCTL_DESCRIBE function then creates a permissions mask to return
    depending on possessor, owner, group and other ACEs, indicating SETATTR if
    any of INVAL, REVOKE and SET_SECURITY are set and indicating WRITE if any
    of WRITE, REVOKE or CLEAR are set.

    Note that the value subsequently returned by KEYCTL_DESCRIBE may not match
    the value set with KEYCTL_SETATTR - but this is already true because keys
    that lack ->read() can't have READ set and keys that lack ->write() can't
    have WRITE set.

    The KEYCTL_SET_TIMEOUT function then is permitted if WRITE or SETSEC is
    set, or if the caller has a valid instantiation auth token.

    The KEYCTL_INVALIDATE function then requires INVAL.

    The KEYCTL_REVOKE function then requires REVOKE.

    The KEYCTL_JOIN_SESSION_KEYRING function then requires JOIN to join an
    existing keyring.

    The JOIN permission is enabled by default for session keyrings and manually
    created keyrings only.

    Signed-off-by: David Howells <dhowells@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] KEYS: Replace uid/gid/perm permissions checking with ACL

Commit Message

Comments

Patch