All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: linux-security-module@vger.kernel.org
Subject: Re: [PATCH] KEYS: prevent creating a different user's keyrings
Date: Mon, 25 Sep 2017 11:54:20 +0000	[thread overview]
Message-ID: <26475.1506340460@warthog.procyon.org.uk> (raw)
In-Reply-To: <20170921220029.GB89627@gmail.com>

Eric Biggers <ebiggers3@gmail.com> wrote:

> Well, maybe.  Whitelists are hard to get right, and it would be a bit ugly
> having to check the name in both add_key() and join_session_keyring().  And
> hopefully that would be everything?

Actually, having thought about it some more, I think your way is better.

> I think there's also a more fundamental problem with how keyring names work.
> If you try to join a keyring with a certain name, how are you supposed to
> know which one you're joining?  There can be many keyrings that have the
> same name; and any unprivileged user can create a keyring with the name, and
> they can grant everyone SEARCH permission so that their keyring can be
> joined.  So it can be the case that a user is wanting to join a particular
> keyring, but they actually get a keyring that a malicious user has crafted
> for them...

Yeah.  With hindsight, I think that firstly, joinable keyrings really need
enablement and, secondly, thread, process, session, user and user-session need
to have to be non-manually-creatable.

However, I'm not sure they can be renamed, since they're searchable and
joinable by name and fixing this might break something in userspace (though I
should hope that this is unlikely).

> Also, if period ('.') is meant to be the reserved character in keyring names,
> why do most of the special names actually start with underscore ('_')?

'.' wasn't a reserved char originally.

David

WARNING: multiple messages have this Message-ID (diff)
From: David Howells <dhowells@redhat.com>
To: Eric Biggers <ebiggers3@gmail.com>
Cc: dhowells@redhat.com, keyrings@vger.kernel.org,
	Michael Halcrow <mhalcrow@google.com>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, Eric Biggers <ebiggers@google.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] KEYS: prevent creating a different user's keyrings
Date: Mon, 25 Sep 2017 12:54:20 +0100	[thread overview]
Message-ID: <26475.1506340460@warthog.procyon.org.uk> (raw)
In-Reply-To: <20170921220029.GB89627@gmail.com>

Eric Biggers <ebiggers3@gmail.com> wrote:

> Well, maybe.  Whitelists are hard to get right, and it would be a bit ugly
> having to check the name in both add_key() and join_session_keyring().  And
> hopefully that would be everything?

Actually, having thought about it some more, I think your way is better.

> I think there's also a more fundamental problem with how keyring names work.
> If you try to join a keyring with a certain name, how are you supposed to
> know which one you're joining?  There can be many keyrings that have the
> same name; and any unprivileged user can create a keyring with the name, and
> they can grant everyone SEARCH permission so that their keyring can be
> joined.  So it can be the case that a user is wanting to join a particular
> keyring, but they actually get a keyring that a malicious user has crafted
> for them...

Yeah.  With hindsight, I think that firstly, joinable keyrings really need
enablement and, secondly, thread, process, session, user and user-session need
to have to be non-manually-creatable.

However, I'm not sure they can be renamed, since they're searchable and
joinable by name and fixing this might break something in userspace (though I
should hope that this is unlikely).

> Also, if period ('.') is meant to be the reserved character in keyring names,
> why do most of the special names actually start with underscore ('_')?

'.' wasn't a reserved char originally.

David

WARNING: multiple messages have this Message-ID (diff)
From: dhowells@redhat.com (David Howells)
To: linux-security-module@vger.kernel.org
Subject: [PATCH] KEYS: prevent creating a different user's keyrings
Date: Mon, 25 Sep 2017 12:54:20 +0100	[thread overview]
Message-ID: <26475.1506340460@warthog.procyon.org.uk> (raw)
In-Reply-To: <20170921220029.GB89627@gmail.com>

Eric Biggers <ebiggers3@gmail.com> wrote:

> Well, maybe.  Whitelists are hard to get right, and it would be a bit ugly
> having to check the name in both add_key() and join_session_keyring().  And
> hopefully that would be everything?

Actually, having thought about it some more, I think your way is better.

> I think there's also a more fundamental problem with how keyring names work.
> If you try to join a keyring with a certain name, how are you supposed to
> know which one you're joining?  There can be many keyrings that have the
> same name; and any unprivileged user can create a keyring with the name, and
> they can grant everyone SEARCH permission so that their keyring can be
> joined.  So it can be the case that a user is wanting to join a particular
> keyring, but they actually get a keyring that a malicious user has crafted
> for them...

Yeah.  With hindsight, I think that firstly, joinable keyrings really need
enablement and, secondly, thread, process, session, user and user-session need
to have to be non-manually-creatable.

However, I'm not sure they can be renamed, since they're searchable and
joinable by name and fixing this might break something in userspace (though I
should hope that this is unlikely).

> Also, if period ('.') is meant to be the reserved character in keyring names,
> why do most of the special names actually start with underscore ('_')?

'.' wasn't a reserved char originally.

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

  parent reply	other threads:[~2017-09-25 11:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18 18:37 [PATCH] KEYS: prevent creating a different user's keyrings Eric Biggers
2017-09-18 18:37 ` Eric Biggers
2017-09-18 18:37 ` Eric Biggers
2017-09-19 16:05 ` David Howells
2017-09-19 16:05   ` David Howells
2017-09-19 16:05   ` David Howells
2017-09-21 22:00   ` Eric Biggers
2017-09-21 22:00     ` Eric Biggers
2017-09-21 22:00     ` Eric Biggers
2017-09-25 11:54   ` David Howells [this message]
2017-09-25 11:54     ` David Howells
2017-09-25 11:54     ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26475.1506340460@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.