[gnutls-devel] GnuTLS | [WIP] Consolidate FIPS .hmac files (!1562)

[Read-only notification of GnuTLS library development activities]

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

--
  
Daiki Ueno started a new discussion on configure.ac: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986046

>  AC_MSG_RESULT($gmp_so)
>  AC_DEFINE_UNQUOTED([GMP_LIBRARY_SONAME], ["$gmp_so"], [The soname of gmp library])
> +AC_SUBST([gmp_path], [`ldconfig -p | grep $gmp_so | tr ' ' '\n' | grep / | head -n1`])

Using `ldconfig` is handy but I'm not sure if it is portable. What about dynamically determining those paths with `dladdr` (`dli_fname`) as in `get_library_path`?

--
  
Daiki Ueno started a new discussion on lib/fipshmac.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986066

>  #include <gnutls/gnutls.h>
>  #include <gnutls/crypto.h>
> +#include <libgen.h>

`<libgen.h>` has portability [problems](https://www.gnu.org/software/gnulib/manual/html_node/libgen_002eh.html); better use alternatives provided by Gnulib.

You could pull in the `basename-lgpl` module through bootstrap.conf, and use the `last_component` function.

--
  
Daiki Ueno started a new discussion on lib/fips.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986072

> -		return;
> +	data.size = strlen(value);
> +	if (hex_data_size(data.size) != HMAC_SIZE) {

Isn't hex-encoded data twice longer than the original bytes?

--
  
Daiki Ueno started a new discussion on lib/fips.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986075

> +	if (hex_data_size(data.size) != HMAC_SIZE) {
> +		_gnutls_debug_log("Invalid size of hmac data\n");
> +		return -1;

Use meaningful error code.

--
  
Daiki Ueno started a new discussion on lib/fips.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986078

> +		return gnutls_assert_val(-1);
> +
> +	if (memcmp(hmac, new_hmac, HMAC_SIZE) != 0) {

Not your problem nor security concern, but I'd use `gnutls_memcmp` for verifying HMACs.

--
  
Daiki Ueno started a new discussion on lib/fips.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562#note_892986080

> +	if (ini_parse_file(stream, handler, p) < 0) {
> +		_gnutls_debug_log("Could not parse hmac file for MAC testing\n");
> +		return -1;

`stream` is leaking here.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1562
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/20220329/957a72e3/attachment-0001.html>