linux-kernel - Re: [PATCH v9 3/7] crypto: move curve_id of ECDH from the key to algorithm name

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:   Wed, 24 Feb 2021 10:15:40 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <yumeng18@...wei.com>, <herbert@...dor.apana.org.au>,
        <davem@...emloft.net>, <marcel@...tmann.org>,
        <johan.hedberg@...il.com>, <luiz.dentz@...il.com>
CC:     <linux-crypto@...r.kernel.org>, <xuzaibo@...wei.com>,
        <wangzhou1@...ilicon.com>, <linux-kernel@...r.kernel.org>,
        <Nicolas.Ferre@...rochip.com>
Subject: Re: [PATCH v9 3/7] crypto: move curve_id of ECDH from the key to
 algorithm name

On 2/24/21 3:29 AM, yumeng wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 在 2021/2/23 18:44, Tudor.Ambarus@...rochip.com 写道:
>> Hi,
>>
>> On 2/23/21 9:10 AM, Meng Yu wrote:
>>> --- a/drivers/crypto/atmel-ecc.c
>>> +++ b/drivers/crypto/atmel-ecc.c
>>> @@ -104,7 +104,7 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
>>>                  return -EINVAL;
>>>          }
>>>
>>> -       ctx->n_sz = atmel_ecdh_supported_curve(params.curve_id);
>>> +       ctx->n_sz = atmel_ecdh_supported_curve(ctx->curve_id);
>>>          if (!ctx->n_sz || params.key_size) {
>>>                  /* fallback to ecdh software implementation */
>>>                  ctx->do_fallback = true;
>>
>> Now that you moved the curve id info into the alg name, and it is
>> no longer dynamically discovered when decoding the key, does it
>> still make sense to keep the curve id, the key size checks, and
>> the fallback to the software implementation?
>> I think we can keep the curve id, the key size check if 'atmel-ecc' may
> support other curves in the future, and if you're sure P256 is the only
> curve 'atmel-ecc' uses, and it will be changed, we can delete it.
> 
> And fallback to ecdh software implementation is needed when
> params.key_size is zero.
> 

I've read the code again, now I remember. The fallback is needed indeed,
but for other reason. ecdh-nist-p256 will be handled in the crypto IP
only when its user provides a zero length private key: we will use a
pre-provisioned private key that can't be read outside of the device.
The fallback was needed for ecdh-nist-p256 when the user provides a
non-zero private key, or for other curve IDs.

Since the atecc508 and atecc608 (for which there isn't support in kernel
as of now) both support just NIST Standard P256 Elliptic Curve, and the
curve id is now unique per alg, the ctx->curve_id handling does no longer
make sense. So please remove the ctx->curve_id handling. ctx->n_sz can be
removed too and use instead directly ATMEL_ECC_NIST_P256_N_SIZE, similar
to how ATMEL_ECC_PUBKEY_SIZE is used.