Re: [PATCH] crypto: ecdsa - Reject non-DER X9.62 signatures

[Florian Bauckholt <florian.bauckholt@cispa.de>]

Hi Stefan,

thanks for testing.

On 11/13/26 00:38, Stefan Berger wrote:
> On 11/12/26 12:06 PM, Stefan Berger wrote:
>> I have a user-space test suite here:
>> https://github.com/stefanberger/eckey-testing
>>
>> It tries to load openssl-generated self-signed certs into the kernel
>> and this fails now:
>>
>> $ ./test-ecc-kernel-keys.sh

I can reproduce this with your script. It fails without the
KEYCTL_PKEY_VERIFY patch and passes with it.

>>> This is a behaviour change for every user of the x962 template (X.509
> 
> I would say a 'potential but unlikely behaviour change' because if you
> signed with a tool (e.g., OpenSSL) that created properly (DER-)formatted
> signatures in X.509 certificates, then those signatures still verify
> with your patch applied. I would assume that most people signed with
> such a tool. In your patch you seem to add more tests for proper DER
> formatting that all existing x.509 certs and signatures should meet, right?
>

Yes, this only changes behaviour for non-conforming signatures. Anything
generated by OpenSSL behaves the same. I'll change the wording to be
explicit about this for v2.

The existing testmgr vectors covered only valid DER encodings and this
patch adds new ones for the BER edge cases.

>>> certificates, module and IMA signatures, KEYCTL_PKEY_VERIFY): signatures
>>> that are valid ECDSA signatures but are not DER-encoded used to verify
>>> and are now rejected with -EINVAL. The p1363 template is not affected.
> 
> What breaks the signature verification test in my test suite currently
> was pointed out by AI. For signature verifications using the
> KEYCTL_PKEY_VERIFY interface, it currently would pass along an input
> size of 72 bytes for a secp256r1 signature, which is the max. size, but
> this does not always correspond to the application provided signature
> size, which may for example be just 71 bytes.
> -               params->out_len = info.max_sig_size;
> +               params->in2_len = uparams.in2_len;

Yes, sorry, maybe I should have mentioned this in the commit message.
The notes below the --- link to this exact fix:

https://lore.kernel.org/r/20261111-keyctl-pkey-fixes-v1-1-934ebaa267a7@cispa.de

The AI review in the thread also ran the patch without the keyctl fix.

I'll move this dependency to the commit message for v2, or wait until
the keyctl fix is in mainline.

Thanks!
Florian