system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-10-15 12:34:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

core: extend nm_utils_*_get_contents() to zero temporary memory

Browse source 
When reading a file, we may allocate intermediate buffers (realloc()).
Also, reading might fail halfway through the process.

Add a new flag that makes sure that this memory is cleared. The
point is when reading secrets, that we don't accidentally leave
private sensitive material in memory.
This commit is contained in:
Thomas Haller 2018-08-30 13:56:05 +02:00
parent c5c0ffdfd0
commit 6b813b904f
5 changed files with 94 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
src/nm-core-utils.c
View file

@ -38,6 +38,7 @@
#include <net/ethernet.h>
#include "nm-utils/nm-random-utils.h"
#include "nm-utils/nm-secret-utils.h"
#include "nm-utils.h"
#include "nm-core-internal.h"
#include "nm-setting-connection.h"
@ -2586,6 +2587,32 @@ _get_contents_error (GError **error, int errsv, const char *format, ...)
return -errsv;
}
static char *
_mem_realloc (char *old, gboolean do_bzero_mem, gsize cur_len, gsize new_len)
{
char *new;
/* re-allocating to zero bytes is an odd case. We don't need it
* and it's not supported. */
nm_assert (new_len > 0);
/* regardless of success/failure, @old will always be freed/consumed. */
if (do_bzero_mem && cur_len > 0) {
new = g_try_malloc (new_len);
if (new)
memcpy (new, old, NM_MIN (cur_len, new_len));
nm_explicit_bzero (old, cur_len);
g_free (old);
} else {
new = g_try_realloc (old, new_len);
if (!new)
g_free (old);
}
return new;
}
/**
* nm_utils_fd_get_contents:
* @fd: open file descriptor to read. The fd will not be closed,
@ -2604,6 +2631,7 @@ _get_contents_error (GError **error, int errsv, const char *format, ...)
* If you set it to 1K, read will fail because fstat() claims the
* file is larger.
*
* @flags: %NMUtilsFileGetContentsFlags for reading the file.
* @contents: the output buffer with the file read. It is always
* NUL terminated. The buffer is at most @max_length long, including
* the NUL byte. That is, it reads only files up to a length of
@ -2621,6 +2649,7 @@ int
nm_utils_fd_get_contents (int fd,
gboolean close_fd,
gsize max_length,
NMUtilsFileGetContentsFlags flags,
char **contents,
gsize *length,
GError **error)
@ -2628,6 +2657,7 @@ nm_utils_fd_get_contents (int fd,
nm_auto_close int fd_keeper = close_fd ? fd : -1;
struct stat stat_buf;
gs_free char *str = NULL;
const bool do_bzero_mem = NM_FLAGS_HAS (flags, NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET);
g_return_val_if_fail (fd >= 0, -EINVAL);
g_return_val_if_fail (contents, -EINVAL);
@ -2654,17 +2684,16 @@ nm_utils_fd_get_contents (int fd,
return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu+1 bytes", n_stat);
n_read = nm_utils_fd_read_loop (fd, str, n_stat, TRUE);
if (n_read < 0)
if (n_read < 0) {
if (do_bzero_mem)
nm_explicit_bzero (str, n_stat);
return _get_contents_error (error, n_read, "error reading %zu bytes from file descriptor", n_stat);
}
str[n_read] = '\0';
if (n_read < n_stat) {
char *tmp;
tmp = g_try_realloc (str, n_read + 1);
if (!tmp)
if (!(str = _mem_realloc (str, do_bzero_mem, n_stat + 1, n_read + 1)))
return _get_contents_error (error, ENOMEM, "failure to reallocate buffer with %zu bytes", n_read + 1);
str = tmp;
}
NM_SET_OUT (length, n_read);
} else {
@ -2695,48 +2724,56 @@ nm_utils_fd_get_contents (int fd,
n_read = fread (buf, 1, sizeof (buf), f);
errsv = errno;
if (ferror (f))
if (ferror (f)) {
if (do_bzero_mem)
nm_explicit_bzero (buf, sizeof (buf));
return _get_contents_error (error, errsv, "error during fread");
}
if ( n_have > G_MAXSIZE - 1 - n_read
|| n_have + n_read + 1 > max_length) {
if (do_bzero_mem)
nm_explicit_bzero (buf, sizeof (buf));
return _get_contents_error (error, EMSGSIZE, "file stream too large (%zu+1 bytes with maximum %zu bytes)",
(n_have > G_MAXSIZE - 1 - n_read) ? G_MAXSIZE : n_have + n_read,
max_length);
}
if (n_have + n_read + 1 >= n_alloc) {
char *tmp;
gsize old_n_alloc = n_alloc;
if (str) {
if (n_alloc != 0) {
nm_assert (str);
if (n_alloc >= max_length / 2)
n_alloc = max_length;
else
n_alloc *= 2;
} else
} else {
nm_assert (!str);
n_alloc = NM_MIN (n_read + 1, sizeof (buf));
}
tmp = g_try_realloc (str, n_alloc);
if (!tmp)
if (!(str = _mem_realloc (str, do_bzero_mem, old_n_alloc, n_alloc))) {
if (do_bzero_mem)
nm_explicit_bzero (buf, sizeof (buf));
return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu bytes", n_alloc);
str = tmp;
}
}
memcpy (str + n_have, buf, n_read);
n_have += n_read;
}
if (do_bzero_mem)
nm_explicit_bzero (buf, sizeof (buf));
if (n_alloc == 0)
str = g_new0 (char, 1);
else {
str[n_have] = '\0';
if (n_have + 1 < n_alloc) {
char *tmp;
tmp = g_try_realloc (str, n_have + 1);
if (!tmp)
if (!(str = _mem_realloc (str, do_bzero_mem, n_alloc, n_have + 1)))
return _get_contents_error (error, ENOMEM, "failure to truncate buffer to %zu bytes", n_have + 1);
str = tmp;
}
}
@ -2753,6 +2790,7 @@ nm_utils_fd_get_contents (int fd,
* @filename: the filename to open. Possibly relative to @dirfd.
* @max_length: allocate at most @max_length bytes.
* WARNING: see nm_utils_fd_get_contents() hint about @max_length.
* @flags: %NMUtilsFileGetContentsFlags for reading the file.
* @contents: the output buffer with the file read. It is always
* NUL terminated. The buffer is at most @max_length long, including
* the NUL byte. That is, it reads only files up to a length of
@ -2770,6 +2808,7 @@ int
nm_utils_file_get_contents (int dirfd,
const char *filename,
gsize max_length,
NMUtilsFileGetContentsFlags flags,
char **contents,
gsize *length,
GError **error)
@ -2809,6 +2848,7 @@ nm_utils_file_get_contents (int dirfd,
return nm_utils_fd_get_contents (fd,
TRUE,
max_length,
flags,
contents,
length,
error);
@ -2942,6 +2982,7 @@ nm_utils_get_boot_id (void)
gs_free char *contents = NULL;
nm_utils_file_get_contents (-1, "/proc/sys/kernel/random/boot_id", 0,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&contents, NULL, NULL);
if (contents) {
g_strstrip (contents);

14
src/nm-core-utils.h
View file

@ -264,9 +264,22 @@ gboolean nm_utils_sysctl_ip_conf_is_path (int addr_family, const char *path, con
gboolean nm_utils_is_specific_hostname (const char *name);
/**
* NMUtilsFileGetContentsFlags:
* @NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE: no flag
* @NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET: if present, ensure that no
* data is left in memory. Essentially, it means to call explicity_bzero()
* to not leave key material on the heap (when reading secrets).
*/
typedef enum {
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE = 0,
NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET = (1 << 0),
} NMUtilsFileGetContentsFlags;
int nm_utils_fd_get_contents (int fd,
gboolean close_fd,
gsize max_length,
NMUtilsFileGetContentsFlags flags,
char **contents,
gsize *length,
GError **error);
@ -274,6 +287,7 @@ int nm_utils_fd_get_contents (int fd,
int nm_utils_file_get_contents (int dirfd,
const char *filename,
gsize max_length,
NMUtilsFileGetContentsFlags flags,
char **contents,
gsize *length,
GError **error);

12
src/platform/nm-linux-platform.c
View file

@ -853,7 +853,9 @@ _linktype_read_devtype (int dirfd)
nm_assert (dirfd >= 0);
if (nm_utils_file_get_contents (dirfd, "uevent", 1*1024*1024, &contents, NULL, NULL) < 0)
if (nm_utils_file_get_contents (dirfd, "uevent", 1*1024*1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&contents, NULL, NULL) < 0)
return NULL;
for (cont = contents; cont; cont = end) {
end = strpbrk (cont, "\r\n");
@ -3515,7 +3517,9 @@ _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, c
char *contents;
gs_free char *value_escaped = g_strescape (value, NULL);
if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, &contents, NULL, &error) < 0) {
if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&contents, NULL, &error) < 0) {
_LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", pathid, value_escaped, error->message);
g_clear_error (&error);
return;
@ -3732,7 +3736,9 @@ sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *pat
pathid = path;
}
if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, &contents, NULL, &error) < 0) {
if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&contents, NULL, &error) < 0) {
/* We assume FAILED means EOPNOTSUP */
if ( g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT)
|| g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NODEV)

14
src/platform/tests/test-link.c
View file

@ -2622,7 +2622,9 @@ test_sysctl_rename (void)
case 0: {
gs_free char *c = NULL;
if (nm_utils_file_get_contents (dirfd, "ifindex", 1*1024*1024, &c, NULL, NULL) < 0)
if (nm_utils_file_get_contents (dirfd, "ifindex", 1*1024*1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&c, NULL, NULL) < 0)
g_assert_not_reached();
g_assert_cmpint (ifindex[0], ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1));
break;
@ -2686,7 +2688,9 @@ test_sysctl_netns_switch (void)
{
gs_free char *c = NULL;
if (nm_utils_file_get_contents (dirfd, "ifindex", 0, &c, NULL, NULL) < 0)
if (nm_utils_file_get_contents (dirfd, "ifindex", 0,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&c, NULL, NULL) < 0)
g_assert_not_reached();
g_assert_cmpint (ifindex, ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1));
}
@ -2698,7 +2702,11 @@ test_sysctl_netns_switch (void)
{
gs_free char *c = NULL;
if (nm_utils_file_get_contents (-1, nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME), 0, &c, NULL, NULL) < 0)
if (nm_utils_file_get_contents (-1,
nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME),
0,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&c, NULL, NULL) < 0)
ifindex_tmp = -1;
else
ifindex_tmp = _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -2);

1
src/settings/plugins/ifcfg-rh/shvar.c
View file

@ -821,6 +821,7 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
if (nm_utils_fd_get_contents (closefd ? nm_steal_fd (&fd) : fd,
closefd,
10 * 1024 * 1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&arena,
NULL,
&local) < 0) {