Skip to content

Commit

Permalink
glib-aux: drop usage of malloc_usable_size() in nm_free_secret()
Browse files Browse the repository at this point in the history
The idea of nm_free_secret() is to clear the secrets from memory. That
surely is some layer of extra snake oil, because we tend to pass secrets
via D-Bus, where the memory gets passed down to (D-Bus) libraries which
have no idea to keep it private. Still...

But turns out, malloc_usable_size() might not actually be usable for
this. Read the discussion at [1].

Stop using malloc_usable_size(), which seems unfortunate.

There is probably no secret relevant data after the NUL byte anyway,
because we tend to create such strings once, and don't rewrite/truncate
them afterwards (which would leave secrets behind as garbage).

Note that systemd's erase_and_free() still uses malloc_usable_size()
([2]) but the macro foo to get that right is terrifying ([3]).

[1] systemd/systemd#22801 (comment)
[2] https://github.com/systemd/systemd/blob/11c0f0659ecd82572c2dc83f3b34493a36dcd954/src/basic/memory-util.h#L101
[3] systemd/systemd@7929e18

Fixes: d63cd26 ('shared: improve nm_free_secret() to clear entire memory buffer')
  • Loading branch information
thom311 committed Feb 8, 2023
1 parent df228a5 commit 8b66865
Showing 1 changed file with 1 addition and 15 deletions.
16 changes: 1 addition & 15 deletions src/libnm-glib-aux/nm-secret-utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,10 @@ nm_explicit_bzero(void *s, gsize n)
void
nm_free_secret(char *secret)
{
gsize len;

if (!secret)
return;

#if GLIB_CHECK_VERSION(2, 44, 0)
/* Here we mix malloc() and g_malloc() API. Usually we avoid this,
* however since glib 2.44.0 we are in fact guaranteed that g_malloc()/g_free()
* just wraps malloc()/free(), so this is actually fine.
*
* See https://gitlab.gnome.org/GNOME/glib/commit/3be6ed60aa58095691bd697344765e715a327fc1
*/
len = malloc_usable_size(secret);
#else
len = strlen(secret);
#endif

nm_explicit_bzero(secret, len);
nm_explicit_bzero(secret, strlen(secret));
g_free(secret);
}

Expand Down

0 comments on commit 8b66865

Please sign in to comment.