Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201217172101.381772-1-daniele.alessandrelli@linux.intel.com>
 <CY4PR1101MB2326ED0E6C23D1D868D53365E7D20@CY4PR1101MB2326.namprd11.prod.outlook.com>
 <20210104113148.GA20575@gondor.apana.org.au> <CY4PR1101MB23260DF5A317CA05BBA3C2F9E7D20@CY4PR1101MB2326.namprd11.prod.outlook.com>
 <CY4PR1101MB232696B49BA1A3441E8B335EE7A80@CY4PR1101MB2326.namprd11.prod.outlook.com>
 <CAMj1kXH9sHm_=dXS7646MbPQoQST9AepfHORSJgj0AxzWB4SvQ@mail.gmail.com>
 <CY4PR1101MB232656080E3F457EC345E7B2E7A40@CY4PR1101MB2326.namprd11.prod.outlook.com>
 <CAMj1kXF9yUVEdPeF6EUCSOdb44HdFuVPk6G2cKOAUAn-mVjCzw@mail.gmail.com> <7ae7890f52226e75bf9e368808d6377e8c5efc2d.camel@intel.com>
In-Reply-To: <7ae7890f52226e75bf9e368808d6377e8c5efc2d.camel@intel.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Thu, 21 Jan 2021 10:52:46 +0100
Message-ID: <CAMj1kXE8TnHvZrp2NQv9SJ4CfUOxy1sVXVusjrSWaiXOjRTQ5g@mail.gmail.com>
Subject: Re: [RFC PATCH 0/6] Keem Bay OCS ECC crypto driver
To:     "Alessandrelli, Daniele" <daniele.alessandrelli@intel.com>
Cc:     "Reshetova, Elena" <elena.reshetova@intel.com>,
        "mgross@linux.intel.com" <mgross@linux.intel.com>,
        "Khurana, Prabhjot" <prabhjot.khurana@intel.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 20 Jan 2021 at 20:00, Alessandrelli, Daniele
<daniele.alessandrelli@intel.com> wrote:
>
> Hi Ard,
>
> Thank you very much for your valuable feedback.
>
> On Mon, 2021-01-18 at 13:09 +0100, Ard Biesheuvel wrote:
> > This is rather unusual compared with how the crypto API is typically
> > used, but if this is really what you want to implement, you can do so
> > by:
> > - having a "ecdh" implementation that implements the entire range, and
> > uses a fallback for curves that it does not implement
> > - export the same implementation again as "ecdh" and with a known
> > driver name "ecdh-keembay-ocs", but with a slightly lower priority,
> > and in this case, return an error when the unimplemented curve is
> > requested.
> >
> > That way, you fully adhere to the API, by providing implementations of
> > all curves by default. And if a user requests "ecdh-keembay-ocs"
> > explicitly, it will not be able to use the P192 curve inadvertently.
>
> I tried to implement this, but it looks like the driver name is
> mandatory, so I specified one also for the first implementation.
>
> Basically I defined two 'struct kpp_alg' variables; both with cra_name
> = "ecdh", but with different 'cra_driver_name' (one with
> cra_driver_name = "ecdh-keembay-ocs-fallback" and the other one with
> cra_driver_name = "ecdh-keembay-ocs").
>
> Is this what you were suggesting?
>
> Anyway, that works (i.e., 'ecdh-keembay-ocs' returns an error when the
> unimplemented curve is requested; while 'ecdh-keembay-ocs' and 'ecdh'
> work fine with any curve), but I have to set the priority of 'ecdh-
> keembay-ocs' to something lower than the 'ecdh_generic' priority.
> Otherwise the implementation with fallback ends up using my "ecdh-
> keembay-ocs" as fallback (so it ends up using a fallback that still
> does not support the P-192 curve).
>
> Also, the implementation without fallback is still failing crypto self-
> tests (as expected I guess).
>
> Therefore, I tried with a slightly different solution. Still two
> implementations, but with different cra_names (one with cra_name =
> "ecdh" and the other one with cra_name = "ecdh-keembay"). This solution
> seems to be working, since, the "ecdh-keembay" is not tested by the
> self tests and is not picked up as fallback for "ecdh" (since, if I
> understand it correctly, it's like if I'm defining a new kind of kpp
> algorithm), but it's still picked when calling crypto_alloc_kpp("ecdh-
> keembay").
>
> Does this second solution looks okay to you? Or does it have some
> pitfall that I'm missing?
>

You should set the CRYPTO_ALG_NEED_FALLBACK flag on both
implementations, to ensure that neither of them are considered as
fallbacks themselves.