[gnutls-devel] GnuTLS | Add ecdh compute function gnutls_ecdh_compute_key (!1395)

[Read-only notification of GnuTLS library development activities]

Merge request https://gitlab.com/gnutls/gnutls/-/merge_requests/1395 was reviewed by Daiki Ueno

--
  
Daiki Ueno started a new discussion on lib/ecdh.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729481

> +  gnutls_ecc_curve_t curve_pub = GNUTLS_ECC_CURVE_INVALID, curve_priv = GNUTLS_ECC_CURVE_INVALID;
> +  unsigned int bits_pub = 0, bits_priv = 0;
> +  gnutls_datum_t priv_x = {NULL, 0}, priv_y = {NULL, 0}, priv_k = {NULL, 0}, pub_x = {NULL, 0}, pub_y = {NULL, 0};

Are those initializers really needed? Also, unlike nettle, we do use Linux kernel coding style with hard tabs :-)

--
  
Daiki Ueno started a new discussion on lib/ecdh.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729486

> +  gnutls_free(priv_k.data);
> +  gnutls_free(pub_x.data);
> +  gnutls_free(pub_y.data);

I suggest clearing the values with `zeroize_key` or `gnutls_memset` (at least for the private ones).

--
  
Daiki Ueno started a new discussion on lib/ecdh.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729489

> +/* Helper functions for ECC handling 
> + * based on public domain code by Tom St. Dennis.
> + */

Is this comment relevant?

--
  
Daiki Ueno started a new discussion on lib/ecdh.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729491

> +#include "errors.h"
> +
> +int gnutls_ecdh_compute_key(gnutls_privkey_t privkey, gnutls_pubkey_t pubkey, gnutls_datum_t *Z)

Would be nice to have a gtk-doc comment, explaining the usage of this function.

--
  
Daiki Ueno started a new discussion on lib/ecdh.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729495

> +  Z->size = 0;
> +  
> +  if (gnutls_privkey_get_pk_algorithm(privkey, &bits_priv) != GNUTLS_PK_ECDSA)

For FIPS, we deliberately don't support Curve25519/Curve448 (because they are not approved in FIPS140-2), but it might be worth adding support for them in the generic API.

--
  
Daiki Ueno started a new discussion on tests/ecdh-compute.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395#note_533729497

>   * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the GNU

I suppose this whitespace change is not intended?


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1395
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20210319/6ea33e1e/attachment-0001.html>