Hi Aurélien,
I have submitted PR#3245 in order to add Edwards curves support to Mbed TLS, with the eventual goal to add support for the EdDSA algorithm. There are still a few things to fix that require discussion.
Thank you very much for your contribution, and for opening this discussion!
Let's start the discussion with the first one. Adding a new curve type requires to add a new entry to the mbedtls_ecp_curve_type enum. The curve type used by a group is returned by the mbedtls_ecp_get_type function. It currently uses the coordinates type of the base point to determine the curve type. Montgomery curves are lacking the y coordinate, and the Short Weierstrass curves use the three x, y and z coordinates.
The Edwards arithmetic implementation in this PR uses the projective coordinates. As such it also uses the x, y and z coordinates and this gives no way to differentiate a Short Weierstrass from an Edwards curve.
Indeed, the current implementation of `ecp_curve_type()` is a bit of a hack and needs changing now that you're adding Edwards curves.
I have currently implemented that by checking if the curves are the Ed25519 or Ed448 ones using the group id [1]. I am not sure it's very clean and it won't scale if more curves are added later. Another alternative would be to add another entry to mbedtls_ecp_group to hold the curve type.
What do you think is the best option? Any other idea?
Honestly I think both options are fine. In the medium term (in 3.0 or 3.x) I'd like to entirely rework the `ecp_group` structure, which currently potentially wastes memory by having multiple instances for the same curve around (for example, when verifying a chain of N certificates all signed with the same curve, we'll have N instances of the `ecp_group` structure for that curve in RAM, which is quite wasteful). So I think in the meantime we don't need to sweat it too much.
I have a slight preference for the option you currently implemented (checking the group ID), for two reasons:
1. Lack of scaling is not an issue, as it's unlikely a large number of Edwards curves are going to be added soon (in particular, not before the rework of ecp_group), since these last few years the tendency seems to be to focus on a small number of trusted curves [1].
2. Adding a new member to `mbedtls_ecp_group` would be an ABI break, and while we don't have a strict policy of ABI stability, it tends to cause pain to packagers if we change the ABI too often, so it's probably better to avoid it if we can.
Thanks, Manuel.
[1] For example, compare https://tools.ietf.org/html/rfc4492#section-5.1.1 to https://tools.ietf.org/html/rfc8422#section-5.1.1