fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-02-03 20:40:34 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

core: introduce and use nm_utils_file_set_contents()

Browse source 
In some places we use g_file_set_contents() after a umask() to limit
the permissions of the created file. Unfortunately if the containing
directory has a default ACL the umask will be ignored and the new file
will have a mode equal to the default ACL (since g_file_set_contents()
opens the file with mode 0666).

Calling a chmod() after the file gets created is insecure (see commit
60b7ed3bdc) and so the only solution seems to be to reimplement
g_file_set_contents() and accept a mode as parameter.

We already had similar functions in the tree, consolidate them into a
new generic utility function.

https://bugzilla.gnome.org/show_bug.cgi?id=769702
This commit is contained in:
Beniamino Galvani 2016-08-26 17:07:14 +02:00
parent a4110d6d4d
commit 21358edc54
4 changed files with 118 additions and 131 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -3657,3 +3657,104 @@ nm_utils_get_reverse_dns_domains_ip6 (const struct in6_addr *ip, guint8 plen, GP
#undef N_SHIFT
}
/**
* Copied from GLib's g_file_set_contents() et al., but allows
* specifying a mode for the new file.
*/
gboolean
nm_utils_file_set_contents (const gchar *filename,
const gchar *contents,
gssize length,
mode_t mode,
GError **error)
{
gs_free char *tmp_name = NULL;
struct stat statbuf;
int errsv;
gssize s;
int fd;
g_return_val_if_fail (filename, FALSE);
g_return_val_if_fail (contents || !length, FALSE);
g_return_val_if_fail (!error || !*error, FALSE);
g_return_val_if_fail (length >= -1, FALSE);
tmp_name = g_strdup_printf ("%s.XXXXXX", filename);
fd = g_mkstemp_full (tmp_name, O_RDWR, mode);
if (fd < 0) {
errsv = errno;
g_set_error (error,
G_FILE_ERROR,
g_file_error_from_errno (errsv),
"failed to create file %s: %s",
tmp_name,
g_strerror (errsv));
return FALSE;
}
while (length > 0) {
s = write (fd, contents, length);
if (s < 0) {
errsv = errno;
if (errsv == EINTR)
continue;
close (fd);
unlink (tmp_name);
g_set_error (error,
G_FILE_ERROR,
g_file_error_from_errno (errsv),
"failed to write to file %s: %s",
tmp_name,
g_strerror (errsv));
return FALSE;
}
g_assert (s <= length);
contents += s;
length -= s;
}
/* If the final destination exists and is > 0 bytes, we want to sync the
* newly written file to ensure the data is on disk when we rename over
* the destination. Otherwise if we get a system crash we can lose both
* the new and the old file on some filesystems. (I.E. those that don't
* guarantee the data is written to the disk before the metadata.)
*/
if ( lstat (filename, &statbuf) == 0
&& statbuf.st_size > 0
&& fsync (fd) != 0) {
errsv = errno;
close (fd);
unlink (tmp_name);
g_set_error (error,
G_FILE_ERROR,
g_file_error_from_errno (errsv),
"failed to fsync %s: %s",
tmp_name,
g_strerror (errsv));
return FALSE;
}
close (fd);
if (rename (tmp_name, filename)) {
errsv = errno;
unlink (tmp_name);
g_set_error (error,
G_FILE_ERROR,
g_file_error_from_errno (errsv),
"failed to rename %s to %s: %s",
tmp_name,
filename,
g_strerror (errsv));
return FALSE;
}
return TRUE;
}

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

@ -428,4 +428,10 @@ const char *nm_utils_dnsmasq_status_to_string (int status, char *dest, gsize siz
void nm_utils_get_reverse_dns_domains_ip4 (guint32 ip, guint8 plen, GPtrArray *domains);
void nm_utils_get_reverse_dns_domains_ip6 (const struct in6_addr *ip, guint8 plen, GPtrArray *domains);
gboolean nm_utils_file_set_contents (const gchar *filename,
const gchar *contents,
gssize length,
mode_t mode,
GError **error);
#endif /* __NM_CORE_UTILS_H__ */

65
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
View file

@ -146,62 +146,6 @@ error:
svSetValue (ifcfg, key, value, FALSE);
}
static gboolean
write_secret_file (const char *path,
const char *data,
gsize len,
GError **error)
{
char *tmppath;
int fd = -1, written;
gboolean success = FALSE;
mode_t saved_umask;
tmppath = g_malloc0 (strlen (path) + 10);
memcpy (tmppath, path, strlen (path));
strcat (tmppath, ".XXXXXX");
/* Only readable by root */
saved_umask = umask (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
errno = 0;
fd = mkstemp (tmppath);
if (fd < 0) {
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not create temporary file for '%s': %d",
path, errno);
goto out;
}
errno = 0;
written = write (fd, data, len);
if (written != len) {
close (fd);
unlink (tmppath);
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not write temporary file for '%s': %d",
path, errno);
goto out;
}
close (fd);
/* Try to rename */
errno = 0;
if (rename (tmppath, path)) {
unlink (tmppath);
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not rename temporary file to '%s': %d",
path, errno);
goto out;
}
success = TRUE;
out:
umask (saved_umask);
g_free (tmppath);
return success;
}
typedef struct ObjectType {
const char *setting_key;
NMSetting8021xCKScheme (*scheme_func)(NMSetting8021x *setting);
@ -356,10 +300,11 @@ write_object (NMSetting8021x *s_8021x,
* can use paths from now on instead of pushing around the certificate
* data itself.
*/
success = write_secret_file (new_file,
(const char *) g_bytes_get_data (blob, NULL),
g_bytes_get_size (blob),
&write_error);
success = nm_utils_file_set_contents (new_file,
(const char *) g_bytes_get_data (blob, NULL),
g_bytes_get_size (blob),
0600,
&write_error);
if (success) {
svSetValue (ifcfg, objtype->ifcfg_key, new_file, FALSE);
g_free (new_file);

77
src/settings/plugins/keyfile/nms-keyfile-writer.c
View file

@ -39,65 +39,6 @@ typedef struct {
const char *keyfile_dir;
} WriteInfo;
/*****************************************************************************/
static gboolean
write_cert_key_file (const char *path,
const guint8 *data,
gsize data_len,
GError **error)
{
char *tmppath;
int fd = -1, written;
gboolean success = FALSE;
mode_t saved_umask;
tmppath = g_malloc0 (strlen (path) + 10);
g_assert (tmppath);
memcpy (tmppath, path, strlen (path));
strcat (tmppath, ".XXXXXX");
/* Only readable by root */
saved_umask = umask (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
errno = 0;
fd = mkstemp (tmppath);
if (fd < 0) {
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not create temporary file for '%s': %d",
path, errno);
goto out;
}
errno = 0;
written = write (fd, data, data_len);
if (written != data_len) {
close (fd);
unlink (tmppath);
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not write temporary file for '%s': %d",
path, errno);
goto out;
}
close (fd);
/* Try to rename */
errno = 0;
if (rename (tmppath, path) == 0)
success = TRUE;
else {
unlink (tmppath);
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Could not rename temporary file to '%s': %d",
path, errno);
}
out:
umask (saved_umask);
g_free (tmppath);
return success;
}
static void
cert_writer (NMConnection *connection,
GKeyFile *file,
@ -182,7 +123,8 @@ cert_writer (NMConnection *connection,
new_path = g_strdup_printf ("%s/%s-%s.%s", info->keyfile_dir, nm_connection_get_uuid (connection),
cert_data->suffix, ext);
success = write_cert_key_file (new_path, blob_data, blob_len, &local);
success = nm_utils_file_set_contents (new_path, (const gchar *) blob_data,
blob_len, 0600, &local);
if (success) {
/* Write the path value to the keyfile.
* We know, that basename(new_path) starts with a UUID, hence no conflict with "data:;base64," */
@ -239,8 +181,6 @@ _internal_write_connection (NMConnection *connection,
WriteInfo info = { 0 };
GError *local_err = NULL;
int errsv;
gboolean success = FALSE;
mode_t saved_umask;
g_return_val_if_fail (!out_path || !*out_path, FALSE);
g_return_val_if_fail (keyfile_dir && keyfile_dir[0] == '/', FALSE);
@ -324,15 +264,13 @@ _internal_write_connection (NMConnection *connection,
if (existing_path != NULL && strcmp (path, existing_path) != 0)
unlink (existing_path);
saved_umask = umask (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
g_file_set_contents (path, data, len, &local_err);
nm_utils_file_set_contents (path, data, len, 0600, &local_err);
if (local_err) {
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"error writing to file '%s': %s",
path, local_err->message);
g_error_free (local_err);
goto out;
return FALSE;
}
if (chown (path, owner_uid, owner_grp) < 0) {
@ -341,7 +279,7 @@ _internal_write_connection (NMConnection *connection,
"error chowning '%s': %s (%d)",
path, g_strerror (errsv), errsv);
unlink (path);
goto out;
return FALSE;
}
if (out_path && g_strcmp0 (existing_path, path)) {
@ -349,10 +287,7 @@ _internal_write_connection (NMConnection *connection,
path = NULL;
}
success = TRUE;
out:
umask (saved_umask);
return success;
return TRUE;
}
gboolean