2023-10-11 17:12:06

by Denis Kenzior

[permalink] [raw]
Subject: Linux 6.5 broke iwd

Hi Herbert,

Looks like something in Linux 6.5 broke ell TLS unit tests (and thus likely
WPA-Enterprise support). I tried a git bisect and could narrow it down to a
general area. The last good commit was:

commit 6cb8815f41a966b217c0d9826c592254d72dcc31
Author: Herbert Xu <[email protected]>
Date: Thu Jun 15 18:28:48 2023 +0800

crypto: sig - Add interface for sign/verify

Split out the sign/verify functionality from the existing akcipher
interface. Most algorithms in akcipher either support encryption
and decryption, or signing and verify. Only one supports both.

As a signature algorithm may not support encryption at all, these
two should be spearated.

For now sig is simply a wrapper around akcipher as all algorithms
remain unchanged. This is a first step and allows users to start
allocating sig instead of akcipher.

Signed-off-by: Herbert Xu <[email protected]>

Narrowing down further didn't work due to:
/usr/bin/ld: crypto/asymmetric_keys/x509_public_key.o: in function
`x509_get_sig_params':
x509_public_key.c:(.text+0x363): undefined reference to `sm2_compute_z_digest'
collect2: error: ld returned 1 exit status

Regards,
-Denis


2023-10-12 00:38:42

by Herbert Xu

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

On Wed, Oct 11, 2023 at 12:11:57PM -0500, Denis Kenzior wrote:
> Hi Herbert,
>
> Looks like something in Linux 6.5 broke ell TLS unit tests (and thus likely
> WPA-Enterprise support). I tried a git bisect and could narrow it down to a
> general area. The last good commit was:
>
> commit 6cb8815f41a966b217c0d9826c592254d72dcc31
> Author: Herbert Xu <[email protected]>
> Date: Thu Jun 15 18:28:48 2023 +0800
>
> crypto: sig - Add interface for sign/verify
>
> Split out the sign/verify functionality from the existing akcipher
> interface. Most algorithms in akcipher either support encryption
> and decryption, or signing and verify. Only one supports both.
>
> As a signature algorithm may not support encryption at all, these
> two should be spearated.
>
> For now sig is simply a wrapper around akcipher as all algorithms
> remain unchanged. This is a first step and allows users to start
> allocating sig instead of akcipher.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> Narrowing down further didn't work due to:
> /usr/bin/ld: crypto/asymmetric_keys/x509_public_key.o: in function
> `x509_get_sig_params':
> x509_public_key.c:(.text+0x363): undefined reference to `sm2_compute_z_digest'
> collect2: error: ld returned 1 exit status

This looks like a Kconfig issue. Please send me your .config file.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-12 02:18:17

by Denis Kenzior

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

Hi Herbert,

>
> This looks like a Kconfig issue. Please send me your .config file.

Possible. I performed the bisect using a kernel configuration from the
following instructions:

https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/doc/test-runner.txt#n55

In my case, I was using the User Mode Linux arch and testing locally using iwd's
test runner.

# make ARCH=um olddefconfig
# make ARCH=um -j8

from iwd-git:
# tools/test-runner --kernel=<path to linux> --start=/bin/bash
<run the failing ell unit test. Namely ell/unit/test-key and ell/unit/test-tls>

Here's my bisect log:
[denkenz@archdev linux]$ git bisect log
git bisect start
# status: waiting for both good and bad commits
# good: [6995e2de6891c724bfeb2db33d7b87775f913ad1] Linux 6.4
git bisect good 6995e2de6891c724bfeb2db33d7b87775f913ad1
# status: waiting for bad commit, 1 good commit known
# bad: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
git bisect bad 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
# good: [b775d6c5859affe00527cbe74263de05cfe6b9f9] Merge tag 'mips_6.5' of
git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
git bisect good b775d6c5859affe00527cbe74263de05cfe6b9f9
# bad: [56cbceab928d7ac3702de172ff8dcc1da2a6aaeb] Merge tag 'usb-6.5-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect bad 56cbceab928d7ac3702de172ff8dcc1da2a6aaeb
# good: [b30d7a77c53ec04a6d94683d7680ec406b7f3ac8] Merge tag
'perf-tools-for-v6.5-1-2023-06-28' of
git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next
git bisect good b30d7a77c53ec04a6d94683d7680ec406b7f3ac8
# bad: [dfab92f27c600fea3cadc6e2cb39f092024e1fef] Merge tag 'nfs-for-6.5-1' of
git://git.linux-nfs.org/projects/trondmy/linux-nfs
git bisect bad dfab92f27c600fea3cadc6e2cb39f092024e1fef
# good: [28968f384be3c064d66954aac4c534a5e76bf973] Merge tag 'pinctrl-v6.5-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
git bisect good 28968f384be3c064d66954aac4c534a5e76bf973
# bad: [5d95ff84e62be914b4a4dabfa814e4096b05b1b0] Merge tag 'v6.5-p1' of
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect bad 5d95ff84e62be914b4a4dabfa814e4096b05b1b0
# good: [d8c226ac1f748d0eac54ef869a4f41b26bc4f825] Merge branch
'pci/controller/endpoint'
git bisect good d8c226ac1f748d0eac54ef869a4f41b26bc4f825
# good: [ef492d080302913e85122a2d92efa2ca174930f8] crypto: caam - adjust RNG
timing to support more devices
git bisect good ef492d080302913e85122a2d92efa2ca174930f8
# good: [b25f62ccb490680a8cee755ac4528909395e0711] Merge tag 'vfio-v6.5-rc1' of
https://github.com/awilliam/linux-vfio
git bisect good b25f62ccb490680a8cee755ac4528909395e0711
# good: [6cb8815f41a966b217c0d9826c592254d72dcc31] crypto: sig - Add interface
for sign/verify

Regards,
-Denis

2023-10-12 02:27:07

by Herbert Xu

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

On Wed, Oct 11, 2023 at 09:18:04PM -0500, Denis Kenzior wrote:
>
> Possible. I performed the bisect using a kernel configuration from the
> following instructions:
>
> https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/doc/test-runner.txt#n55
>
> In my case, I was using the User Mode Linux arch and testing locally using
> iwd's test runner.
>
> # make ARCH=um olddefconfig
> # make ARCH=um -j8

I can't reproduce this with olddefconfig. It produces a config
where crypto/asymmetric_keys is disabled altogether.

Please send me your actual config file that triggers the build
failure.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-12 02:33:18

by Herbert Xu

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

On Thu, Oct 12, 2023 at 08:38:14AM +0800, Herbert Xu wrote:
>
> > Narrowing down further didn't work due to:
> > /usr/bin/ld: crypto/asymmetric_keys/x509_public_key.o: in function
> > `x509_get_sig_params':
> > x509_public_key.c:(.text+0x363): undefined reference to `sm2_compute_z_digest'
> > collect2: error: ld returned 1 exit status

I wonder if this is a bug in the bisection process. The reference
to sm2_compute_z_digest is protected by an IS_REACHABLE test on
SM2 which is presumably disabled in your configuration.

Can you please do a clean rebuild from the first buggy commit with
the same config file and see if it still triggers the build failure?

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-12 03:09:26

by Denis Kenzior

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

Hi Herbert,

>
> I wonder if this is a bug in the bisection process. The reference
> to sm2_compute_z_digest is protected by an IS_REACHABLE test on
> SM2 which is presumably disabled in your configuration.

So I just tried this again, here are the exact steps. I believe the sha1 below
was next in the bisect process.

[denkenz@archdev linux]$ git checkout 63ba4d67594ad05b2c899b5a3a8cc7581052dd13
HEAD is now at 63ba4d67594a KEYS: asymmetric: Use new crypto interface without
scatterlists
[denkenz@archdev linux]$ make ARCH=um x86_64_defconfig
#
# configuration written to .config
#
[denkenz@archdev linux]$ ARCH=um sh ~/iwd-master/tools/test_runner_kernel_config
[denkenz@archdev linux]$ make ARCH=um olddefconfig
#
# configuration written to .config
#
[denkenz@archdev linux]$ make clean
CLEAN crypto/asymmetric_keys
CLEAN drivers/base/firmware_loader/builtin
CLEAN init
CLEAN kernel
CLEAN lib
CLEAN .
CLEAN modules.builtin modules.builtin.modinfo .vmlinux.export.c
[denkenz@archdev linux]$ make ARCH=um -j16

...

/usr/bin/ld: crypto/asymmetric_keys/x509_public_key.o: in function
`x509_get_sig_params':
/home/denkenz/devel/linux/crypto/asymmetric_keys/x509_public_key.c:70:(.text+0x390):
undefined reference to `sm2_compute_z_digest'

Regards,
-Denis

2023-10-12 03:28:39

by Herbert Xu

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

On Wed, Oct 11, 2023 at 10:09:02PM -0500, Denis Kenzior wrote:
.
> [denkenz@archdev linux]$ git checkout 63ba4d67594ad05b2c899b5a3a8cc7581052dd13
> HEAD is now at 63ba4d67594a KEYS: asymmetric: Use new crypto interface
> without scatterlists

No wonder I can't reproduce this. This is already fixed by

commit 3867caee497edf6ce6b6117aac1c0b87c0a2cb5f
Author: Herbert Xu <[email protected]>
Date: Sat Jun 24 13:19:56 2023 +0800

crypto: sm2 - Provide sm2_compute_z_digest when sm2 is disabled

It's just a bisection artifact, you need to skip the broken commit
when bisecting.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-12 14:19:57

by Denis Kenzior

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

Hi Herbert,

On 10/11/23 22:28, Herbert Xu wrote:
> On Wed, Oct 11, 2023 at 10:09:02PM -0500, Denis Kenzior wrote:
> .
>> [denkenz@archdev linux]$ git checkout 63ba4d67594ad05b2c899b5a3a8cc7581052dd13
>> HEAD is now at 63ba4d67594a KEYS: asymmetric: Use new crypto interface
>> without scatterlists
>
> No wonder I can't reproduce this. This is already fixed by
>
> commit 3867caee497edf6ce6b6117aac1c0b87c0a2cb5f
> Author: Herbert Xu <[email protected]>
> Date: Sat Jun 24 13:19:56 2023 +0800
>
> crypto: sm2 - Provide sm2_compute_z_digest when sm2 is disabled
>
> It's just a bisection artifact, you need to skip the broken commit
> when bisecting.
>

Unfortunately that commit causes the unit test to crash the kernel.
bash-5.1# uname -a
Linux (none) 6.4.0-rc1-00082-g3867caee497e #37 Thu Oct 12 09:11:21 CDT 2023
x86_64 GNU/Linux
bash-5.1# unit/test-key
TEST: unsupported
TEST: user key
TEST: Diffie-Hellman 1
TEST: Diffie-Hellman 2
TEST: Diffie-Hellman 3
TEST: simple keyring
TEST: trusted keyring
Kernel panic - not syncing: Kernel mode fault at addr 0x18, ip 0x601fac3a
CPU: 0 PID: 28 Comm: test-key Not tainted 6.4.0-rc1-00082-g3867caee497e #37
Stack:
6232a840 6232d400 00000000 2000000100
62345e00 00000000 718b7aa0 718b7aa0
00000000 712851d8 10000000400 00000000
Call Trace:
[<602115cd>] public_key_verify_signature+0x272/0x2ed
[<60210503>] ? asymmetric_key_id_same+0x0/0x3b
[<602131d9>] x509_check_for_self_signed+0x65/0xe3
[<600bd2ca>] ? kfree+0x0/0x4a
[<6021242f>] x509_cert_parse+0x205/0x245
[<60212de2>] x509_key_preparse+0x28/0x237
[<6006820d>] ? __down_read_common+0x92/0xc4
[<602102f1>] asymmetric_key_preparse+0x50/0x7f
[<601eaa68>] __key_create_or_update+0x1c1/0x4d4
[<601ec7a1>] ? key_ref_put+0x0/0x16
[<601ead90>] key_create_or_update+0x15/0x17
[<601eccc2>] sys_add_key+0x183/0x1d8
[<60025c4d>] handle_syscall+0x99/0xc7
[<60038646>] userspace+0x4d3/0x60f
[<60021ac2>] fork_handler+0x92/0x94
Aborted (core dumped)

I managed to narrow things down a bit further:

[denkenz@archdev linux]$ git bisect good
There are only 'skip'ped commits left to test.
The first bad commit could be any of:
501e197a02d4aef157f53ba3a0b9049c3e52fedc
afa9d00ee0fda2387ad598d0b106e96a7ed360ae
b335f258e8ddafec0e8ae2201ca78d29ed8f85eb
3867caee497edf6ce6b6117aac1c0b87c0a2cb5f
d744ae7477190967a3ddc289e2cd4ae59e8b1237
63ba4d67594ad05b2c899b5a3a8cc7581052dd13
767cfee8368f43c6d6c58cdf8c2d143a027fa55f
891ebfdfa3d08bf55ebec523c99bb68ac9c34cf7
e5221fa6a355112ddcc29dc82a94f7c3a1aacc0b
486bfb05913ac9969a3a71a4dc48f17f31cb162d
We cannot bisect more!

Four of the above are hwrng related, so not the culprits.

Maybe?

891ebfdfa3d08bf55ebec523c99bb68ac9c34cf7
crypto: sig - Fix verify call

Regards,
-Denis

2023-10-12 14:28:34

by Herbert Xu

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

On Thu, Oct 12, 2023 at 09:19:42AM -0500, Denis Kenzior wrote:
>
> Unfortunately that commit causes the unit test to crash the kernel.
> bash-5.1# uname -a
> Linux (none) 6.4.0-rc1-00082-g3867caee497e #37 Thu Oct 12 09:11:21 CDT 2023
> x86_64 GNU/Linux
> bash-5.1# unit/test-key
> TEST: unsupported
> TEST: user key
> TEST: Diffie-Hellman 1
> TEST: Diffie-Hellman 2
> TEST: Diffie-Hellman 3
> TEST: simple keyring
> TEST: trusted keyring
> Kernel panic - not syncing: Kernel mode fault at addr 0x18, ip 0x601fac3a
> CPU: 0 PID: 28 Comm: test-key Not tainted 6.4.0-rc1-00082-g3867caee497e #37
> Stack:
> 6232a840 6232d400 00000000 2000000100
> 62345e00 00000000 718b7aa0 718b7aa0
> 00000000 712851d8 10000000400 00000000

OK, we need to take a step back. The commits that you're testing
are all known to be buggy.

So please tell me what exactly is the problem with the latest kernel
and then we can work it back from there.

If you have a reproducer that works on the latest kernel that would
be great.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-12 15:09:03

by Denis Kenzior

[permalink] [raw]
Subject: Re: Linux 6.5 broke iwd

Hi Herbert,

> OK, we need to take a step back. The commits that you're testing
> are all known to be buggy.
>
> So please tell me what exactly is the problem with the latest kernel
> and then we can work it back from there.

Looks like something took out the ability to run sign/verify without a hash on
asymmetric keys.

>
> If you have a reproducer that works on the latest kernel that would
> be great.
>

Easiest is to run the ell unit test:

[denkenz@archdev tmp]$ git clone git://git.kernel.org/pub/scm/libs/ell/ell.git
[denkenz@archdev tmp]$ cd ell
[denkenz@archdev ell]$ ./bootstrap-configure
[denkenz@archdev ell]$ make -j16
[denkenz@archdev ell]$ unit/test-key
TEST: unsupported
TEST: user key
TEST: Diffie-Hellman 1
TEST: Diffie-Hellman 2
TEST: Diffie-Hellman 3
TEST: simple keyring
TEST: trusted keyring
TEST: trust chain
TEST: key crypto
test-key: unit/test-key.c:624: test_key_crypto: Assertion `len ==
sizeof(ciphertext)' failed.

Regards,
-Denis

2023-10-16 08:36:24

by Herbert Xu

[permalink] [raw]
Subject: [PATCH] KEYS: asymmetric: Fix sign/verify on pkcs1pad without a hash

On Thu, Oct 12, 2023 at 10:08:46AM -0500, Denis Kenzior wrote:
>
> Looks like something took out the ability to run sign/verify without a hash
> on asymmetric keys.

Indeed this is what it was. Please try this patch. Thanks!

---8<---
The new sign/verify code broke the case of pkcs1pad without a
hash algorithm. Fix it by setting issig correctly for this case.

Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
Cc: [email protected] # v6.5
Reported-by: Denis Kenzior <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index abeecb8329b3..2f9181c4cd59 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -81,14 +81,13 @@ software_key_determine_akcipher(const struct public_key *pkey,
* RSA signatures usually use EMSA-PKCS1-1_5 [RFC3447 sec 8.2].
*/
if (strcmp(encoding, "pkcs1") == 0) {
+ *sig = op == kernel_pkey_sign ||
+ op == kernel_pkey_verify;
if (!hash_algo) {
- *sig = false;
n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
"pkcs1pad(%s)",
pkey->pkey_algo);
} else {
- *sig = op == kernel_pkey_sign ||
- op == kernel_pkey_verify;
n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
"pkcs1pad(%s,%s)",
pkey->pkey_algo, hash_algo);
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-16 19:37:18

by Denis Kenzior

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix sign/verify on pkcs1pad without a hash

Hi Herbert,

On 10/16/23 03:35, Herbert Xu wrote:
> On Thu, Oct 12, 2023 at 10:08:46AM -0500, Denis Kenzior wrote:
>>
>> Looks like something took out the ability to run sign/verify without a hash
>> on asymmetric keys.
>
> Indeed this is what it was. Please try this patch. Thanks!
>

I can confirm that this fix does make all unit tests pass again. Feel free to add:

Tested-by: Denis Kenzior <[email protected]>

Regards,
-Denis