Re: [PATCH] KEYS: request_key: enforce keyring restrictions on constructed keys

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sat, Oct 31, 2026 at 10:10:36AM +0530, Gallop Permission wrote:
> Hi, Jarkko
> 
> 1. you asked: What does "key becoming positive" mean here broken down?
>     Here key becoming positive” means the transition from
> KEY_IS_UNINSTANTIATED to KEY_IS_POSITIVE
>     after the type’s instantiate() succeeded and the payload is present.

Great use concrete termininology.

> 
> 2. And on the second para
>     For an unrestricted destination, construct_alloc_key() still
> pre-links the under-construction key,
>     so concurrent request_key() calls for the same type/description
> find it and wait instead of
>     starting another helper. That visibility is what deduplicates construction.
> 
>     For a restricted destination, the patch skips that provisional
> link until the key has been
>     constructed and checked. A concurrent request for the same key may
> therefore fail to find
>     the in-progress key and start a second helper. The keyring still
> holds at most one link for
>     a given type/description; each result is offered through
> key_link(), and only a key accepted by
>     the restriction is cached. This is a duplicate-work tradeoff, not
> duplicate links in the keyring.
> 
>     KEY_ALLOC_BYPASS_RESTRICTION callers are excluded from the deferral,
>     they keep the existing provisional-link behavior and bypass the
> restriction check as before.

Ditto.

> 
> 3. the rationale for the code change will be to stop an un priviledge
> actor with write access to
> a restricted keyring from bypassing the restriction checks.

OK, so I did not mean that. That is motivation to do a code change,
which is I guess what previous paragraph try to answer.

It says nothing about solution per se. And neither I see how this
answer would give anything on that.

> 
> As for the points raised by sashiko ai (concerning
> potential outer-search bypasses in request_key_and_link)
> im going to need some more time to verify them and fold any necessary
> fixes for v2.

Address what I said about commit message by writing a proper commit
message.

> 
> Best regards,
> Anvik
> 
> 
> On Fri, 30 Oct 2026 at 23:48, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Fri, Oct 30, 2026 at 11:09:30PM +0530, Anvik Loitongbam wrote:
> > > construct_alloc_key() pre-links a newly allocated key into the destination
> > > keyring with __key_link(), bypassing __key_link_check_restriction(). If
> > > the request-key helper instantiates the key with ringid 0 (or any keyring
> > > other than the restricted destination), the key becomes positive while the
> > > provisional link in a restricted keyring remains, so the restriction never
> > > sees the payload, potentially resulting in a policy violation, where a
> > > user with write access can inject keys which are restricted on the restricted
> > > keyring.
> >
> > What does "key becoming positive" mean here broken down?
> >
> > Also I have really hard time on reading the second paragrah as it is,
> > let me count, over five lines long. I honestly cannot say that I
> > understood what I read. I tried it three times.
> >
> > If you understand your fix, you should be able to nail it.
> >
> > >
> > > Restricted destinations lose the provisional-link deduplication while the
> > > key is being constructed. Callers that pass
> > > KEY_ALLOC_BYPASS_RESTRICTION keep the existing provisional-link behavior.
> >
> > The second paragraph should be reformatted and there is no rationale
> > or descriptio for the code change.
> >
> > Br, Jarkko

Br, Jarkko