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-

Does this second solution looks okay to you? Or does it have some
pitfall that I'm missing?


Reply via email to