Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference
@ 2026-04-29 18:16 Weiming Shi
  2026-05-01  5:37 ` Ignat Korchagin
  0 siblings, 1 reply; 4+ messages in thread
From: Weiming Shi @ 2026-04-29 18:16 UTC (permalink / raw)
  To: David Howells, Lukas Wunner, Ignat Korchagin, Herbert Xu,
	David S . Miller
  Cc: Jarkko Sakkinen, Andrew Zaborowski, keyrings, linux-crypto,
	Xiang Mei, Weiming Shi

asymmetric_key_ids() returns key->payload.data[asym_key_ids], which can
be NULL for keys parsed by the PKCS#8 parser (pkcs8_parser.c explicitly
stores NULL in prep->payload.data[asym_key_ids]).

key_or_keyring_common() in restrict.c and find_asymmetric_key() in
asymmetric_type.c both dereference this return value without checking
for NULL. An unprivileged user can trigger a NULL pointer dereference
in key_or_keyring_common() by creating a PKCS#8 key, restricting a
keyring with key_or_keyring:<pkcs8_serial>, and adding an X.509 cert
to the restricted keyring. CONFIG_PKCS8_PRIVATE_KEY_PARSER=y is
required.

 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000
 KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
 RIP: 0010:key_or_keyring_common (crypto/asymmetric_keys/restrict.c:205 crypto/asymmetric_keys/restrict.c:279)
 Call Trace:
  <TASK>
  __key_create_or_update (security/keys/key.c:884)
  key_create_or_update (security/keys/key.c:1021)
  __do_sys_add_key (security/keys/keyctl.c:134)
  do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
  </TASK>
 Kernel panic - not syncing: Fatal exception

Add a NULL check in find_asymmetric_key(), mirroring the existing
pattern in asymmetric_match_key_ids() and asymmetric_key_describe().
In key_or_keyring_common(), skip the trusted key matching when it
has no key IDs and fall through to the check_dest path.

Fixes: 7d30198ee24f ("keys: X.509 public key issuer lookup without AKID")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 crypto/asymmetric_keys/asymmetric_type.c | 2 ++
 crypto/asymmetric_keys/restrict.c        | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -109,6 +109,8 @@ struct key *find_asymmetric_key(struct key *keyring,
 	if (id_0 && id_1) {
 		const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);

+		if (!kids)
+			goto reject;
 		if (!kids->id[1]) {
 			pr_debug("First ID matches, but second is missing\n");
 			goto reject;
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -243,10 +243,14 @@ static int key_or_keyring_common(struct key *dest_keyring,
 			if (IS_ERR(key))
 				key = NULL;
 		} else if (trusted->type == &key_type_asymmetric) {
+			const struct asymmetric_key_ids *kids;
 			const struct asymmetric_key_id **signer_ids;

-			signer_ids = (const struct asymmetric_key_id **)
-				asymmetric_key_ids(trusted)->id;
+			kids = asymmetric_key_ids(trusted);
+			if (!kids)
+				goto skip_trusted;
+
+			signer_ids = (const struct asymmetric_key_id **)kids->id;

 			/*
 			 * The auth_ids come from the candidate key (the
@@ -290,6 +294,7 @@ static int key_or_keyring_common(struct key *dest_keyring,
 		}
 	}

+skip_trusted:
 	if (check_dest && !key) {
 		/* See if the destination has a key that signed this one. */
 		key = find_asymmetric_key(dest_keyring, sig->auth_ids[0],
--
2.39.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference
  2026-04-29 18:16 [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference Weiming Shi
@ 2026-05-01  5:37 ` Ignat Korchagin
  2026-05-01  7:48   ` Weiming Shi
  0 siblings, 1 reply; 4+ messages in thread
From: Ignat Korchagin @ 2026-05-01  5:37 UTC (permalink / raw)
  To: Weiming Shi
  Cc: David Howells, Lukas Wunner, Herbert Xu, David S . Miller,
	Jarkko Sakkinen, Andrew Zaborowski, keyrings, linux-crypto,
	Xiang Mei

Hi,

Thanks for the report.

On Wed, Apr 29, 2026 at 7:17 PM Weiming Shi <bestswngs@gmail.com> wrote:
>
> asymmetric_key_ids() returns key->payload.data[asym_key_ids], which can
> be NULL for keys parsed by the PKCS#8 parser (pkcs8_parser.c explicitly
> stores NULL in prep->payload.data[asym_key_ids]).
>
> key_or_keyring_common() in restrict.c and find_asymmetric_key() in
> asymmetric_type.c both dereference this return value without checking
> for NULL. An unprivileged user can trigger a NULL pointer dereference
> in key_or_keyring_common() by creating a PKCS#8 key, restricting a
> keyring with key_or_keyring:<pkcs8_serial>, and adding an X.509 cert
> to the restricted keyring. CONFIG_PKCS8_PRIVATE_KEY_PARSER=y is

Could you add a simple bash script for this to the commit message?

> required.
>
>  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000
>  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>  RIP: 0010:key_or_keyring_common (crypto/asymmetric_keys/restrict.c:205 crypto/asymmetric_keys/restrict.c:279)
>  Call Trace:
>   <TASK>
>   __key_create_or_update (security/keys/key.c:884)
>   key_create_or_update (security/keys/key.c:1021)
>   __do_sys_add_key (security/keys/keyctl.c:134)
>   do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>   </TASK>
>  Kernel panic - not syncing: Fatal exception
>
> Add a NULL check in find_asymmetric_key(), mirroring the existing
> pattern in asymmetric_match_key_ids() and asymmetric_key_describe().
> In key_or_keyring_common(), skip the trusted key matching when it
> has no key IDs and fall through to the check_dest path.
>
> Fixes: 7d30198ee24f ("keys: X.509 public key issuer lookup without AKID")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  crypto/asymmetric_keys/asymmetric_type.c | 2 ++
>  crypto/asymmetric_keys/restrict.c        | 9 ++++++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> --- a/crypto/asymmetric_keys/asymmetric_type.c
> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> @@ -109,6 +109,8 @@ struct key *find_asymmetric_key(struct key *keyring,
>         if (id_0 && id_1) {
>                 const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>
> +               if (!kids)
> +                       goto reject;
>                 if (!kids->id[1]) {
>                         pr_debug("First ID matches, but second is missing\n");
>                         goto reject;
> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> --- a/crypto/asymmetric_keys/restrict.c
> +++ b/crypto/asymmetric_keys/restrict.c
> @@ -243,10 +243,14 @@ static int key_or_keyring_common(struct key *dest_keyring,
>                         if (IS_ERR(key))
>                                 key = NULL;
>                 } else if (trusted->type == &key_type_asymmetric) {
> +                       const struct asymmetric_key_ids *kids;
>                         const struct asymmetric_key_id **signer_ids;
>
> -                       signer_ids = (const struct asymmetric_key_id **)
> -                               asymmetric_key_ids(trusted)->id;
> +                       kids = asymmetric_key_ids(trusted);
> +                       if (!kids)
> +                               goto skip_trusted;
> +
> +                       signer_ids = (const struct asymmetric_key_id **)kids->id;
>
>                         /*
>                          * The auth_ids come from the candidate key (the
> @@ -290,6 +294,7 @@ static int key_or_keyring_common(struct key *dest_keyring,
>                 }
>         }
>
> +skip_trusted:
>         if (check_dest && !key) {
>                 /* See if the destination has a key that signed this one. */
>                 key = find_asymmetric_key(dest_keyring, sig->auth_ids[0],
> --
> 2.39.0
>

Ignat

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference
  2026-05-01  5:37 ` Ignat Korchagin
@ 2026-05-01  7:48   ` Weiming Shi
  2026-05-02  8:40     ` Ignat Korchagin
  0 siblings, 1 reply; 4+ messages in thread
From: Weiming Shi @ 2026-05-01  7:48 UTC (permalink / raw)
  To: Ignat Korchagin, g
  Cc: David Howells, Lukas Wunner, Herbert Xu, David S . Miller,
	Jarkko Sakkinen, Andrew Zaborowski, keyrings, linux-crypto,
	Xiang Mei

On 26-05-01 06:37, Ignat Korchagin wrote:
> Hi,
> 
> Thanks for the report.
> 
> On Wed, Apr 29, 2026 at 7:17 PM Weiming Shi <bestswngs@gmail.com> wrote:
> >
> > asymmetric_key_ids() returns key->payload.data[asym_key_ids], which can
> > be NULL for keys parsed by the PKCS#8 parser (pkcs8_parser.c explicitly
> > stores NULL in prep->payload.data[asym_key_ids]).
> >
> > key_or_keyring_common() in restrict.c and find_asymmetric_key() in
> > asymmetric_type.c both dereference this return value without checking
> > for NULL. An unprivileged user can trigger a NULL pointer dereference
> > in key_or_keyring_common() by creating a PKCS#8 key, restricting a
> > keyring with key_or_keyring:<pkcs8_serial>, and adding an X.509 cert
> > to the restricted keyring. CONFIG_PKCS8_PRIVATE_KEY_PARSER=y is
> 
> Could you add a simple bash script for this to the commit message?
> 

Hi Ignat,

Sure, here is a bash reproducer:

```
#!/bin/bash
modprobe pkcs8_key_parser 2>/dev/null
openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:1024 \
    -out /tmp/poc.pem 2>/dev/null
openssl pkcs8 -topk8 -nocrypt -in /tmp/poc.pem \
    -outform DER -out /tmp/poc.p8
openssl req -new -x509 -key /tmp/poc.pem -outform DER \
    -out /tmp/poc.der -days 365 -subj "/CN=Test" \
    -addext "subjectKeyIdentifier=hash" \
    -addext "authorityKeyIdentifier=keyid:always" 2>/dev/null
PKCS8_ID=$(keyctl padd asymmetric pkcs8key @s < /tmp/poc.p8)
KR=$(keyctl newring test_kr @s)
keyctl restrict_keyring $KR asymmetric "key_or_keyring:$PKCS8_ID"
keyctl padd asymmetric trigger $KR < /tmp/poc.der
rm -f /tmp/poc.pem /tmp/poc.p8 /tmp/poc.der
```
If you'd prefer it in the commit message I can send a v2.

Thanks,
Weiming Shi


> > required.
> >
> >  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000
> >  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> >  RIP: 0010:key_or_keyring_common (crypto/asymmetric_keys/restrict.c:205 crypto/asymmetric_keys/restrict.c:279)
> >  Call Trace:
> >   <TASK>
> >   __key_create_or_update (security/keys/key.c:884)
> >   key_create_or_update (security/keys/key.c:1021)
> >   __do_sys_add_key (security/keys/keyctl.c:134)
> >   do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> >   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> >   </TASK>
> >  Kernel panic - not syncing: Fatal exception
> >
> > Add a NULL check in find_asymmetric_key(), mirroring the existing
> > pattern in asymmetric_match_key_ids() and asymmetric_key_describe().
> > In key_or_keyring_common(), skip the trusted key matching when it
> > has no key IDs and fall through to the check_dest path.
> >
> > Fixes: 7d30198ee24f ("keys: X.509 public key issuer lookup without AKID")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  crypto/asymmetric_keys/asymmetric_type.c | 2 ++
> >  crypto/asymmetric_keys/restrict.c        | 9 ++++++---
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> > --- a/crypto/asymmetric_keys/asymmetric_type.c
> > +++ b/crypto/asymmetric_keys/asymmetric_type.c
> > @@ -109,6 +109,8 @@ struct key *find_asymmetric_key(struct key *keyring,
> >         if (id_0 && id_1) {
> >                 const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> >
> > +               if (!kids)
> > +                       goto reject;
> >                 if (!kids->id[1]) {
> >                         pr_debug("First ID matches, but second is missing\n");
> >                         goto reject;
> > diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > --- a/crypto/asymmetric_keys/restrict.c
> > +++ b/crypto/asymmetric_keys/restrict.c
> > @@ -243,10 +243,14 @@ static int key_or_keyring_common(struct key *dest_keyring,
> >                         if (IS_ERR(key))
> >                                 key = NULL;
> >                 } else if (trusted->type == &key_type_asymmetric) {
> > +                       const struct asymmetric_key_ids *kids;
> >                         const struct asymmetric_key_id **signer_ids;
> >
> > -                       signer_ids = (const struct asymmetric_key_id **)
> > -                               asymmetric_key_ids(trusted)->id;
> > +                       kids = asymmetric_key_ids(trusted);
> > +                       if (!kids)
> > +                               goto skip_trusted;
> > +
> > +                       signer_ids = (const struct asymmetric_key_id **)kids->id;
> >
> >                         /*
> >                          * The auth_ids come from the candidate key (the
> > @@ -290,6 +294,7 @@ static int key_or_keyring_common(struct key *dest_keyring,
> >                 }
> >         }
> >
> > +skip_trusted:
> >         if (check_dest && !key) {
> >                 /* See if the destination has a key that signed this one. */
> >                 key = find_asymmetric_key(dest_keyring, sig->auth_ids[0],
> > --
> > 2.39.0
> >
> 
> Ignat

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference
  2026-05-01  7:48   ` Weiming Shi
@ 2026-05-02  8:40     ` Ignat Korchagin
  0 siblings, 0 replies; 4+ messages in thread
From: Ignat Korchagin @ 2026-05-02  8:40 UTC (permalink / raw)
  To: Weiming Shi
  Cc: g, David Howells, Lukas Wunner, Herbert Xu, David S . Miller,
	Jarkko Sakkinen, Andrew Zaborowski, keyrings, linux-crypto,
	Xiang Mei

On Fri, May 1, 2026 at 8:48 AM Weiming Shi <bestswngs@gmail.com> wrote:
>
> On 26-05-01 06:37, Ignat Korchagin wrote:
> > Hi,
> >
> > Thanks for the report.
> >
> > On Wed, Apr 29, 2026 at 7:17 PM Weiming Shi <bestswngs@gmail.com> wrote:
> > >
> > > asymmetric_key_ids() returns key->payload.data[asym_key_ids], which can
> > > be NULL for keys parsed by the PKCS#8 parser (pkcs8_parser.c explicitly
> > > stores NULL in prep->payload.data[asym_key_ids]).
> > >
> > > key_or_keyring_common() in restrict.c and find_asymmetric_key() in
> > > asymmetric_type.c both dereference this return value without checking
> > > for NULL. An unprivileged user can trigger a NULL pointer dereference
> > > in key_or_keyring_common() by creating a PKCS#8 key, restricting a
> > > keyring with key_or_keyring:<pkcs8_serial>, and adding an X.509 cert
> > > to the restricted keyring. CONFIG_PKCS8_PRIVATE_KEY_PARSER=y is
> >
> > Could you add a simple bash script for this to the commit message?
> >
>
> Hi Ignat,
>
> Sure, here is a bash reproducer:
>
> ```
> #!/bin/bash
> modprobe pkcs8_key_parser 2>/dev/null
> openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:1024 \
>     -out /tmp/poc.pem 2>/dev/null
> openssl pkcs8 -topk8 -nocrypt -in /tmp/poc.pem \
>     -outform DER -out /tmp/poc.p8
> openssl req -new -x509 -key /tmp/poc.pem -outform DER \
>     -out /tmp/poc.der -days 365 -subj "/CN=Test" \
>     -addext "subjectKeyIdentifier=hash" \
>     -addext "authorityKeyIdentifier=keyid:always" 2>/dev/null
> PKCS8_ID=$(keyctl padd asymmetric pkcs8key @s < /tmp/poc.p8)
> KR=$(keyctl newring test_kr @s)
> keyctl restrict_keyring $KR asymmetric "key_or_keyring:$PKCS8_ID"
> keyctl padd asymmetric trigger $KR < /tmp/poc.der
> rm -f /tmp/poc.pem /tmp/poc.p8 /tmp/poc.der
> ```
> If you'd prefer it in the commit message I can send a v2.

Yes, please. So if anyone tries to "optimise" it later they would have
a clear test case

> Thanks,
> Weiming Shi

Ignat

>
> > > required.
> > >
> > >  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000
> > >  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> > >  RIP: 0010:key_or_keyring_common (crypto/asymmetric_keys/restrict.c:205 crypto/asymmetric_keys/restrict.c:279)
> > >  Call Trace:
> > >   <TASK>
> > >   __key_create_or_update (security/keys/key.c:884)
> > >   key_create_or_update (security/keys/key.c:1021)
> > >   __do_sys_add_key (security/keys/keyctl.c:134)
> > >   do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> > >   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> > >   </TASK>
> > >  Kernel panic - not syncing: Fatal exception
> > >
> > > Add a NULL check in find_asymmetric_key(), mirroring the existing
> > > pattern in asymmetric_match_key_ids() and asymmetric_key_describe().
> > > In key_or_keyring_common(), skip the trusted key matching when it
> > > has no key IDs and fall through to the check_dest path.
> > >
> > > Fixes: 7d30198ee24f ("keys: X.509 public key issuer lookup without AKID")
> > > Reported-by: Xiang Mei <xmei5@asu.edu>
> > > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > > ---
> > >  crypto/asymmetric_keys/asymmetric_type.c | 2 ++
> > >  crypto/asymmetric_keys/restrict.c        | 9 ++++++---
> > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> > > --- a/crypto/asymmetric_keys/asymmetric_type.c
> > > +++ b/crypto/asymmetric_keys/asymmetric_type.c
> > > @@ -109,6 +109,8 @@ struct key *find_asymmetric_key(struct key *keyring,
> > >         if (id_0 && id_1) {
> > >                 const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> > >
> > > +               if (!kids)
> > > +                       goto reject;
> > >                 if (!kids->id[1]) {
> > >                         pr_debug("First ID matches, but second is missing\n");
> > >                         goto reject;
> > > diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > > --- a/crypto/asymmetric_keys/restrict.c
> > > +++ b/crypto/asymmetric_keys/restrict.c
> > > @@ -243,10 +243,14 @@ static int key_or_keyring_common(struct key *dest_keyring,
> > >                         if (IS_ERR(key))
> > >                                 key = NULL;
> > >                 } else if (trusted->type == &key_type_asymmetric) {
> > > +                       const struct asymmetric_key_ids *kids;
> > >                         const struct asymmetric_key_id **signer_ids;
> > >
> > > -                       signer_ids = (const struct asymmetric_key_id **)
> > > -                               asymmetric_key_ids(trusted)->id;
> > > +                       kids = asymmetric_key_ids(trusted);
> > > +                       if (!kids)
> > > +                               goto skip_trusted;
> > > +
> > > +                       signer_ids = (const struct asymmetric_key_id **)kids->id;
> > >
> > >                         /*
> > >                          * The auth_ids come from the candidate key (the
> > > @@ -290,6 +294,7 @@ static int key_or_keyring_common(struct key *dest_keyring,
> > >                 }
> > >         }
> > >
> > > +skip_trusted:
> > >         if (check_dest && !key) {
> > >                 /* See if the destination has a key that signed this one. */
> > >                 key = find_asymmetric_key(dest_keyring, sig->auth_ids[0],
> > > --
> > > 2.39.0
> > >
> >
> > Ignat
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-02  8:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29 18:16 [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference Weiming Shi
2026-05-01  5:37 ` Ignat Korchagin
2026-05-01  7:48   ` Weiming Shi
2026-05-02  8:40     ` Ignat Korchagin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox