linux-kernel - Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 21 Mar 2024 12:36:36 -0400
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Jarkko Sakkinen <jarkko@...nel.org>,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
        Lukas Wunner <lukas@...ner.de>
Cc: Stefan Berger <stefanb@...ux.vnet.ibm.com>, keyrings@...r.kernel.org,
        linux-crypto@...r.kernel.org, herbert@...dor.apana.org.au,
        davem@...emloft.net, linux-kernel@...r.kernel.org,
        saulo.alessandre@....jus.br, bbhushan2@...vell.com
Subject: Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

On 3/21/24 12:19, Jarkko Sakkinen wrote:
> On Thu Mar 21, 2024 at 6:17 PM EET, Jarkko Sakkinen wrote:
>> On Wed Mar 20, 2024 at 4:41 PM EET, Konstantin Ryabitsev wrote:
>>> On Wed, Mar 20, 2024 at 06:40:33AM +0100, Lukas Wunner wrote:
>>>> If Herbert applies patches with "b4 am --apply-cover-trailers" or
>>>> "b4 shazam --apply-cover-trailers" (I don't know if he does),
>>>> it is completely irrelevant whether Stefan strips my Tested-by from
>>>> individual patches:  It will automatically be re-added when the
>>>> series gets applied.
>>>
>>> Applying trailers sent to the cover letter is now the default behaviour in
>>> 0.13, so this flag is no longer required (it does nothing).
>>>
>>> -K
>>
>> The whole policy of how to put tested-by in my experience is subsystem
>> dependent.
>>
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>
>> Official documentation only speaks about patches so perhaps it should
>> then be refined for the series.
>>
>> I'm hearing about this option in b4 for the first time in my life.
> 
> It is also pretty relevant to know when you read the commit log e.g.
> when bisecting what was *actually* tested.
> 
> If you put tested-by to whole series it probably means that you've
> tested the uapi and are getting the expected results. Thus in this
> case it would 13/13 that is *actually* tested.

Btw, we have 2 entry points into the code and those are uapi and testmgr.

So if I was to exercise the uapi with a NIST P521 key then are you 
saying that none of the other code was exercised and therefore wasn't 
tested? How would YOU suggest to test individual patches then?

What the docs at the link above say is this:

A Tested-by: tag indicates that the patch has been successfully tested 
(in some environment) by the person named. This tag informs maintainers 
that some testing has been performed, provides a means to locate testers 
for future patches, and ensures credit for the testers.

Note: 'some testing' 'in some environment'. We probably can reasonably 
assume that not only 13/13 is necessary but also several of the other 
patches are necessary to support this new curve and were exercised with 
either UAPI and probably also testmgr.

> 
> Putting tested-by to every possible patch only degrades the quality
> of the commit log.

I would still be interested how one would test individual patches in a 
series so they are worthy of a Tested-by tag.

> 
> I don't see how this is "irrelevant".
> 
> BR, Jarkko