2015-06-26 Miloslav Trmač <mitr@redhat.com>
* modules/files.c (open_and_copy_file): Replace and heavily modify ...
(lu_files_create_backup): ... this.
(lock_file_handle_existing, lock_file_create, lock_file_remove)
(struct editing, editing_open, replace_file_or_symlink)
(editing_close): New functions.
(generic_lookup, generic_is_locked, lu_files_enumerate)
(lu_files_users_enumerate_by_group, lu_files_groups_enumerate_by_user)
(lu_files_enumerate_full): Remove locking on read-only operations.
(generic_add, generic_mod, generic_del, generic_lock)
(generic_setpass): Use struct editing instead of dealing with locking,
backups, SELinux individually.
* lib/user_private.h (lu_util_lock_obtain, lu_util_lock_free): Mark
as deprecated.
* lib/util.c (lu_util_field_write): Fail on an incomplete write().
2015-06-25 Miloslav Trmač <mitr@redhat.com>
* modules/files.c (format_generic, generic_setpass): Refuse to write
field values which contain \n.
* tests/files_test.py (Tests.testUserAdd9, Tests.testUserMod8)
(tests.testUserSetpass5, tests.testGroupAdd6, tests.testGroupMod7)
(tests.testGroupSetpass4): New tests.
diff -up libuser-0.60/lib/user_private.h.CVE-2015-3246 libuser-0.60/lib/user_private.h
--- libuser-0.60/lib/user_private.h.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200
+++ libuser-0.60/lib/user_private.h 2015-07-08 15:15:14.060544103 +0200
@@ -330,9 +330,11 @@ typedef char lu_security_context_t; /* "
((void)(PATH), (void)(MODE), (void)(ERROR), TRUE)
#endif
-/* Lock a file. */
+#ifndef LU_DISABLE_DEPRECATED
+/* Lock a file. Deprecated. */
gpointer lu_util_lock_obtain(int fd, struct lu_error **error);
void lu_util_lock_free(gpointer lock);
+#endif
/* Manipulate a colon-delimited flat text file. */
char *lu_util_line_get_matching1(int fd, const char *firstpart,
diff -up libuser-0.60/lib/util.c.CVE-2015-3246 libuser-0.60/lib/util.c
--- libuser-0.60/lib/util.c.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200
+++ libuser-0.60/lib/util.c 2015-07-08 15:15:14.060544103 +0200
@@ -632,7 +632,7 @@ lu_util_field_write(int fd, const char *
goto err;
}
len = strlen(buf);
- if (write(fd, buf, len) == -1) {
+ if (write(fd, buf, len) != len) {
lu_error_new(error, lu_error_write, NULL);
ret = FALSE;
goto err;
diff -up libuser-0.60/modules/files.c.CVE-2015-3246 libuser-0.60/modules/files.c
--- libuser-0.60/modules/files.c.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200
+++ libuser-0.60/modules/files.c 2015-07-08 15:16:41.014981429 +0200
@@ -25,6 +25,7 @@
#include <fcntl.h>
#include <fnmatch.h>
#include <limits.h>
+#include <shadow.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -101,82 +102,79 @@ module_filename(struct lu_module *module
return g_strconcat(dir, file_suffix, NULL);
}
-/* Create a backup copy of "filename" named "filename-". */
-static gboolean
-lu_files_create_backup(const char *filename,
- struct lu_error **error)
+/* Copy contents of INPUT_FILENAME to OUTPUT_FILENAME, exclusively creating it
+ * if EXCLUSIVE.
+ * Return the file descriptor for OUTPUT_FILENAME, open for reading and writing,
+ * or -1 on error.
+ * Note that this does no locking and assumes the directories hosting the files
+ * are not being manipulated by an attacker. */
+static int
+open_and_copy_file(const char *input_filename, const char *output_filename,
+ gboolean exclusive, struct lu_error **error)
{
int ifd, ofd;
- gpointer ilock, olock;
- char *backupname;
- struct stat ist, ost;
- off_t offset;
- gboolean res = FALSE;
+ struct stat st;
+ int res = -1;
+ int flags;
- g_assert(filename != NULL);
- g_assert(strlen(filename) > 0);
+ g_assert(input_filename != NULL);
+ g_assert(strlen(input_filename) > 0);
+ g_assert(output_filename != NULL);
+ g_assert(strlen(output_filename) > 0);
- /* Open the original file. */
- ifd = open(filename, O_RDONLY);
+ /* Open the input file. */
+ ifd = open(input_filename, O_RDONLY);
if (ifd == -1) {
lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
+ _("couldn't open `%s': %s"), input_filename,
strerror(errno));
goto err;
}
- /* Lock the input file. */
- if ((ilock = lu_util_lock_obtain(ifd, error)) == NULL)
- goto err_ifd;
-
/* Read the input file's size. */
- if (fstat(ifd, &ist) == -1) {
+ if (fstat(ifd, &st) == -1) {
lu_error_new(error, lu_error_stat,
- _("couldn't stat `%s': %s"), filename,
+ _("couldn't stat `%s': %s"), input_filename,
strerror(errno));
- goto err_ilock;
+ goto err_ifd;
}
- /* Generate the backup file's name and open it, creating it if it
- * doesn't already exist. */
- backupname = g_strconcat(filename, "-", NULL);
- ofd = open(backupname, O_WRONLY | O_CREAT, ist.st_mode);
+ /* We only need O_WRONLY, but the caller needs RDWR if ofd will be
+ * used as e->new_fd. */
+ flags = O_RDWR | O_CREAT;
+ if (exclusive) {
+ /* This ensures that if there is a concurrent writer which is
+ * not doing locking for some reason, we will not truncate their
+ * temporary file. Still, the other writer may truncate our
+ * file, and ultimately the rename() committing the changes will
+ * lose one or the other set of changes. */
+ (void)unlink(output_filename);
+ flags |= O_EXCL;
+ } else
+ flags |= O_TRUNC;
+ /* Start with absolutely restrictive permissions to make sure nobody
+ * can get a file descriptor for this file until we are done resetting
+ * ownership. */
+ ofd = open(output_filename, flags, 0);
if (ofd == -1) {
lu_error_new(error, lu_error_open,
- _("error creating `%s': %s"), backupname,
+ _("error creating `%s': %s"), output_filename,
strerror(errno));
- goto err_backupname;
- }
-
- /* If we can't read its type, or it's not a normal file, bail. */
- if (fstat(ofd, &ost) == -1) {
- lu_error_new(error, lu_error_stat, _("couldn't stat `%s': %s"),
- backupname, strerror(errno));
- goto err_ofd;
- }
- if (!S_ISREG(ost.st_mode)) {
- lu_error_new(error, lu_error_open,
- _("backup file `%s' exists and is not a regular file"),
- backupname);
- goto err_ofd;
+ goto err_ifd;
}
- /* Now lock the output file. */
- if ((olock = lu_util_lock_obtain(ofd, error)) == NULL)
- goto err_ofd;
-
/* Set the permissions on the new file to match the old one. */
- if (fchown(ofd, ist.st_uid, ist.st_gid) == -1 && errno != EPERM) {
+ if (fchown(ofd, st.st_uid, st.st_gid) == -1 && errno != EPERM) {
lu_error_new(error, lu_error_generic,
- _("Error changing owner of `%s': %s"), backupname,
- strerror(errno));
- goto err_olock;
+ _("Error changing owner of `%s': %s"),
+ output_filename, strerror(errno));
+ goto err_ofd;
}
- if (fchmod(ofd, ist.st_mode) == -1) {
+ if (fchmod(ofd, st.st_mode) == -1) {
lu_error_new(error, lu_error_generic,
- _("Error changing mode of `%s': %s"), backupname,
- strerror(errno));
- goto err_olock;
+ _("Error changing mode of `%s': %s"),
+ output_filename, strerror(errno));
+ goto err_ofd;
}
/* Copy the data, block by block. */
@@ -190,9 +188,9 @@ lu_files_create_backup(const char *filen
if (errno == EINTR)
continue;
lu_error_new(error, lu_error_read,
- _("Error reading `%s': %s"), filename,
- strerror(errno));
- goto err_olock;
+ _("Error reading `%s': %s"),
+ input_filename, strerror(errno));
+ goto err_ofd;
}
if (left == 0)
break;
@@ -206,55 +204,297 @@ lu_files_create_backup(const char *filen
continue;
lu_error_new(error, lu_error_write,
_("Error writing `%s': %s"),
- backupname, strerror(errno));
- goto err_olock;
+ output_filename, strerror(errno));
+ goto err_ofd;
}
p += out;
left -= out;
}
}
- /* Flush data to disk, and truncate at the current offset. This is
- * necessary if the file existed before we opened it. */
- fsync(ofd);
- offset = lseek(ofd, 0, SEEK_CUR);
- if (offset == -1 || ftruncate(ofd, offset) == -1) {
- lu_error_new(error, lu_error_generic,
- _("Error writing `%s': %s"), backupname,
- strerror(errno));
- goto err_olock;
- }
-
- /* Re-read data about the output file. */
- if (fstat(ofd, &ost) == -1) {
- lu_error_new(error, lu_error_stat,
- _("couldn't stat `%s': %s"), backupname,
- strerror(errno));
- goto err_olock;
- }
-
- /* Complain if the files are somehow not the same. */
- if (ist.st_size != ost.st_size) {
- lu_error_new(error, lu_error_generic,
- _("backup file size mismatch"));
- goto err_olock;
+ /* Flush data to disk. */
+ if (fsync(ofd) != 0 || lseek(ofd, 0, SEEK_SET) == -1) {
+ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"),
+ output_filename, strerror(errno));
+ goto err_ofd;
}
- res = TRUE;
+ res = ofd;
+ goto err_ifd; /* Do not close ofd */
- err_olock:
- lu_util_lock_free(olock);
err_ofd:
close(ofd);
- err_backupname:
- g_free(backupname);
- err_ilock:
- lu_util_lock_free(ilock);
err_ifd:
close(ifd);
err:
return res;
}
+/* Deal with an existing LOCK_FILENAME.
+ * Return TRUE if the caller should try again. */
+static gboolean
+lock_file_handle_existing(const char *lock_filename, struct lu_error **error)
+{
+ gchar *lock_contents;
+ GError *gerror;
+ gboolean ret = FALSE;
+ uintmax_t pid;
+ char *p;
+
+ gerror = NULL;
+ if (g_file_get_contents(lock_filename, &lock_contents, NULL, &gerror)
+ == FALSE) {
+ lu_error_new(error, lu_error_read,
+ _("couldn't read from `%s': %s"), lock_filename,
+ gerror->message);
+ g_error_free(gerror);
+ goto err;
+ }
+ errno = 0;
+ pid = strtoumax(lock_contents, &p, 10);
+ if (errno != 0 || *p != 0 || p == lock_contents || (pid_t)pid != pid) {
+ lu_error_new(error, lu_error_lock,
+ _("Invalid contents of lock `%s'"), lock_filename);
+ goto err_lock_contents;
+ }
+ if (kill(pid, 0) == 0 || errno != ESRCH) {
+ lu_error_new(error, lu_error_lock,
+ _("The lock %s is held by process %ju"),
+ lock_filename, pid);
+ goto err_lock_contents;
+ }
+ /* This is unfixably racy, but that should matter only if a genuine
+ * lock owner crashes. */
+ if (unlink(lock_filename) != 0) {
+ lu_error_new(error, lu_error_lock,
+ _("Error removing stale lock `%s': %s"), lock_filename,
+ strerror(errno));
+ goto err_lock_contents;
+ }
+ ret = TRUE;
+ /* Fall through */
+
+err_lock_contents:
+ g_free(lock_contents);
+err:
+ return ret;
+}
+
+/* Create a lock file for FILENAME. */
+static gboolean
+lock_file_create(const char *filename, struct lu_error **error)
+{
+ char *lock_filename, *tmp_filename;
+ char pid_string[sizeof (pid_t) * CHAR_BIT + 1];
+ int fd;
+ gboolean ret = FALSE;
+
+ lock_filename = g_strconcat(filename, ".lock", NULL);
+ tmp_filename = g_strdup_printf("%s.lock.XXXXXX", filename);
+
+ fd = mkstemp(tmp_filename);
+ if (fd == -1) {
+ lu_error_new(error, lu_error_open,
+ _("error opening temporary file for `%s': %s"),
+ lock_filename, strerror(errno));
+ goto err_tmp_filename;
+ }
+ if (snprintf(pid_string, sizeof(pid_string), "%ju", (uintmax_t)getpid())
+ >= sizeof(pid_string))
+ g_assert_not_reached();
+ if (write(fd, pid_string, strlen(pid_string)) != strlen(pid_string)) {
+ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"),
+ tmp_filename, strerror(errno));
+ close(fd);
+ goto err_tmp_file;
+ }
+ close(fd);
+
+ if (link(tmp_filename, lock_filename) != 0) {
+ if (errno == EEXIST) {
+ if (lock_file_handle_existing(lock_filename, error)
+ == FALSE)
+ goto err_tmp_file;
+ if (link(tmp_filename, lock_filename) == 0)
+ goto got_link;
+ }
+ lu_error_new(error, lu_error_lock,
+ _("Cannot obtain lock `%s': %s"), lock_filename,
+ strerror(errno));
+ goto err_tmp_file;
+ }
+got_link:
+ ret = TRUE;
+ /* Fall through */
+
+err_tmp_file:
+ (void)unlink(tmp_filename);
+err_tmp_filename:
+ g_free(tmp_filename);
+ g_free(lock_filename);
+ return ret;
+}
+
+/* Remove the lock file for FILENAME. */
+static void
+lock_file_remove(const char *filename)
+{
+ char *lock_file;
+
+ lock_file = g_strconcat(filename, ".lock", NULL);
+ (void)unlink(lock_file);
+ g_free(lock_file);
+}
+
+/* State related to a file currently open for editing. */
+struct editing {
+ char *filename;
+ lu_security_context_t fscreate;
+ char *new_filename;
+ int new_fd;
+};
+
+/* Open and lock FILE_SUFFIX in MODULE for editing.
+ * Return editing state, or NULL on error. */
+static struct editing *
+editing_open(struct lu_module *module, const char *file_suffix,
+ struct lu_error **error)
+{
+ struct editing *e;
+ char *backup_name;
+ int fd;
+
+ e = g_malloc0(sizeof (*e));
+ e->filename = module_filename(module, file_suffix);
+ /* Make sure this all works if e->filename is a symbolic link, at least
+ * as long as it points to the same file system. */
+
+ if (geteuid() == 0) {
+ if (lckpwdf() != 0) {
+ lu_error_new(error, lu_error_lock,
+ _("error locking file: %s"),
+ strerror(errno));
+ goto err_filename;
+ }
+ }
+ if (lock_file_create(e->filename, error) == FALSE)
+ goto err_lckpwdf;
+
+ if (!lu_util_fscreate_save(&e->fscreate, error))
+ goto err_locked;
+ if (!lu_util_fscreate_from_file(e->filename, error))
+ goto err_fscreate;
+
+ backup_name = g_strconcat(e->filename, "-", NULL);
+ fd = open_and_copy_file(e->filename, backup_name, FALSE, error);
+ g_free (backup_name);
+ close(fd);
+ if (fd == -1)
+ goto err_fscreate;
+
+ e->new_filename = g_strconcat(e->filename, "+", NULL);
+ e->new_fd = open_and_copy_file(e->filename, e->new_filename, TRUE,
+ error);
+ if (e->new_fd == -1)
+ goto err_new_filename;
+
+ return e;
+
+err_new_filename:
+ g_free(e->new_filename);
+err_fscreate:
+ lu_util_fscreate_restore(e->fscreate);
+
+err_locked:
+ (void)lock_file_remove(e->filename);
+err_lckpwdf:
+ if (geteuid() == 0)
+ (void)ulckpwdf();
+
+err_filename:
+ g_free(e->filename);
+ g_free(e);
+ return NULL;
+}
+
+
+/* Replace DESTINATION with SOURCE, even if DESTINATION is a symbolic link. */
+static gboolean
+replace_file_or_symlink(const char *source, const char *destination,
+ struct lu_error **error)
+{
+ struct stat st;
+ char *tmp;
+ gboolean ret = FALSE;
+
+ tmp = NULL;
+ if (lstat(destination, &st) == 0 && S_ISLNK(st.st_mode)) {
+ tmp = realpath(destination, NULL);
+ if (tmp == NULL) {
+ lu_error_new(error, lu_error_generic,
+ _("Error resolving `%s': %s"), destination,
+ strerror(errno));
+ goto err;
+ }
+ destination = tmp;
+ }
+ if (rename(source, destination) != 0) {
+ lu_error_new(error, lu_error_write,
+ _("Error replacing `%s': %s"), destination,
+ strerror(errno));
+ goto err;
+ }
+ ret = TRUE;
+ /* Fall through */
+
+err:
+ free(tmp);
+ return ret;
+}
+
+/* Finish editing E, commit edits if COMMIT.
+ * Return true only if RET_INPUT and everything went OK; suggested usage is
+ * ret = editing_close(e, commit, ret, error); */
+static gboolean
+editing_close(struct editing *e, gboolean commit, gboolean ret_input,
+ struct lu_error **error)
+{
+ gboolean ret = FALSE;
+ gboolean unlink_new_filename = TRUE;
+
+ g_assert(e != NULL);
+
+ if (commit && fsync(e->new_fd) != 0) {
+ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"),
+ e->new_filename, strerror(errno));
+ close(e->new_fd);
+ goto err;
+ }
+ close(e->new_fd);
+
+ if (commit) {
+ if (replace_file_or_symlink(e->new_filename, e->filename,
+ error) == FALSE)
+ goto err;
+ unlink_new_filename = FALSE;
+ }
+ ret = ret_input;
+
+err:
+ if (unlink_new_filename)
+ (void)unlink(e->new_filename);
+ g_free(e->new_filename);
+ lu_util_fscreate_restore(e->fscreate);
+
+ (void)lock_file_remove(e->filename);
+ if (geteuid() == 0)
+ (void)ulckpwdf();
+
+ g_free(e->filename);
+ g_free(e);
+ return ret;
+}
+
+
/* Read a line from the file, no matter how long it is, and return it as a
* newly-allocated string, with the terminator intact. */
static char *
@@ -435,7 +675,6 @@ generic_lookup(struct lu_module *module,
{
gboolean ret;
int fd = -1;
- gpointer lock;
char *line, *filename;
g_assert(module != NULL);
@@ -446,7 +685,7 @@ generic_lookup(struct lu_module *module,
filename = module_filename(module, file_suffix);
- /* Open the file and lock it. */
+ /* Open the file. */
fd = open(filename, O_RDONLY);
if (fd == -1) {
lu_error_new(error, lu_error_open,
@@ -457,15 +696,9 @@ generic_lookup(struct lu_module *module,
}
g_free(filename);
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- return FALSE;
- }
-
/* Search for the entry in this file. */
line = lu_util_line_get_matchingx(fd, name, field, error);
if (line == NULL) {
- lu_util_lock_free(lock);
close(fd);
return FALSE;
}
@@ -473,7 +706,6 @@ generic_lookup(struct lu_module *module,
/* If we found data, parse it and then free the data. */
ret = parser(line, ent);
g_free(line);
- lu_util_lock_free(lock);
close(fd);
return ret;
@@ -666,6 +898,13 @@ format_generic(struct lu_ent *ent, const
char *field;
field = format_field(ent, formats + i);
+ if (strchr(field, '\n') != NULL) {
+ lu_error_new(error, lu_error_invalid_attribute_value,
+ _("%s value `%s': `\\n' not allowed"),
+ formats[i].attribute, field);
+ g_free(field);
+ goto err;
+ }
if (i != format_count - 1 && strchr(field, ':') != NULL) {
lu_error_new(error, lu_error_invalid_attribute_value,
_("%s value `%s': `:' not allowed"),
@@ -729,11 +968,9 @@ generic_add(struct lu_module *module, co
const struct format_specifier *formats, size_t format_count,
struct lu_ent *ent, struct lu_error **error)
{
- lu_security_context_t fscreate;
- char *line, *filename, *contents;
- int fd;
+ struct editing *e;
+ char *line, *contents;
ssize_t r;
- gpointer lock;
struct stat st;
off_t offset;
gboolean ret = FALSE;
@@ -743,50 +980,30 @@ generic_add(struct lu_module *module, co
g_assert(format_count > 0);
g_assert(ent != NULL);
- filename = module_filename(module, file_suffix);
-
line = format_generic(ent, formats, format_count, error);
if (line == NULL)
- goto err_filename;
+ goto err;
- if (!lu_util_fscreate_save(&fscreate, error))
+ e = editing_open(module, file_suffix, error);
+ if (e == NULL)
goto err_line;
- if (!lu_util_fscreate_from_file(filename, error))
- goto err_fscreate;
-
- /* Create a backup copy of the file we're about to modify. */
- if (lu_files_create_backup(filename, error) == FALSE)
- goto err_fscreate;
-
- /* Open the file. */
- fd = open(filename, O_RDWR);
- if (fd == -1) {
- lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
- strerror(errno));
- goto err_fscreate;
- }
-
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
/* Read the file's size. */
- if (fstat(fd, &st) == -1) {
+ if (fstat(e->new_fd, &st) == -1) {
lu_error_new(error, lu_error_stat,
- _("couldn't stat `%s': %s"), filename,
+ _("couldn't stat `%s': %s"), e->new_filename,
strerror(errno));
- goto err_lock;
+ goto err_editing;
}
/* Read the entire file in. There's some room for improvement here,
* but at least we still have the lock, so it's not going to get
* funky on us. */
contents = g_malloc0(st.st_size + 1);
- if (read(fd, contents, st.st_size) != st.st_size) {
+ if (read(e->new_fd, contents, st.st_size) != st.st_size) {
lu_error_new(error, lu_error_read,
_("couldn't read from `%s': %s"),
- filename, strerror(errno));
+ e->new_filename, strerror(errno));
goto err_contents;
}
@@ -798,54 +1015,43 @@ generic_add(struct lu_module *module, co
goto err_contents;
}
/* Hooray, we can add this entry at the end of the file. */
- offset = lseek(fd, 0, SEEK_END);
+ offset = lseek(e->new_fd, 0, SEEK_END);
if (offset == -1) {
lu_error_new(error, lu_error_write,
_("couldn't write to `%s': %s"),
- filename, strerror(errno));
+ e->new_filename, strerror(errno));
goto err_contents;
}
/* If the last byte in the file isn't a newline, add one, and silently
* curse people who use text editors (which shall remain unnamed) which
* allow saving of the file without a final line terminator. */
if ((st.st_size > 0) && (contents[st.st_size - 1] != '\n')) {
- if (write(fd, "\n", 1) != 1) {
+ if (write(e->new_fd, "\n", 1) != 1) {
lu_error_new(error, lu_error_write,
_("couldn't write to `%s': %s"),
- filename,
- strerror(errno));
+ e->new_filename, strerror(errno));
goto err_contents;
}
}
/* Attempt to write the entire line to the end. */
- r = write(fd, line, strlen(line));
+ r = write(e->new_fd, line, strlen(line));
if ((size_t)r != strlen(line)) {
/* Oh, come on! */
lu_error_new(error, lu_error_write,
- _("couldn't write to `%s': %s"),
- filename,
+ _("couldn't write to `%s': %s"), e->new_filename,
strerror(errno));
- /* Truncate off whatever we actually managed to write and
- * give up. */
- (void)ftruncate(fd, offset);
goto err_contents;
}
- /* Hey, it succeeded. */
ret = TRUE;
/* Fall through */
err_contents:
g_free(contents);
-err_lock:
- lu_util_lock_free(lock);
-err_fd:
- close(fd);
-err_fscreate:
- lu_util_fscreate_restore(fscreate);
+err_editing:
+ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */
err_line:
g_free(line);
-err_filename:
- g_free(filename);
+err:
return ret;
}
@@ -938,11 +1144,9 @@ generic_mod(struct lu_module *module, co
const struct format_specifier *formats, size_t format_count,
struct lu_ent *ent, struct lu_error **error)
{
- lu_security_context_t fscreate;
- char *filename, *new_line, *contents, *line, *rest;
+ struct editing *e;
+ char *new_line, *contents, *line, *rest;
char *current_name, *fragment;
- int fd;
- gpointer lock;
const char *name_attribute;
gboolean ret = FALSE;
struct stat st;
@@ -971,43 +1175,24 @@ generic_mod(struct lu_module *module, co
return FALSE;
}
- filename = module_filename(module, file_suffix);
-
new_line = format_generic(ent, formats, format_count, error);
if (new_line == NULL)
- goto err_filename;
+ goto err_current_name;
- if (!lu_util_fscreate_save(&fscreate, error))
+ e = editing_open(module, file_suffix, error);
+ if (e == NULL)
goto err_new_line;
- if (!lu_util_fscreate_from_file(filename, error))
- goto err_fscreate;
- /* Create a backup file. */
- if (lu_files_create_backup(filename, error) == FALSE)
- goto err_fscreate;
- /* Open the file to be modified. */
- fd = open(filename, O_RDWR);
- if (fd == -1) {
- lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
- strerror(errno));
- goto err_fscreate;
- }
-
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
-
- if (fstat(fd, &st) == -1) {
+ if (fstat(e->new_fd, &st) == -1) {
lu_error_new(error, lu_error_stat, _("couldn't stat `%s': %s"),
- filename, strerror(errno));
- goto err_lock;
+ e->new_filename, strerror(errno));
+ goto err_editing;
}
contents = g_malloc(st.st_size + 1 + strlen(new_line));
- if (read(fd, contents, st.st_size) != st.st_size) {
+ if (read(e->new_fd, contents, st.st_size) != st.st_size) {
lu_error_new(error, lu_error_read,
- _("couldn't read from `%s': %s"), filename,
+ _("couldn't read from `%s': %s"), e->new_filename,
strerror(errno));
goto err_contents;
}
@@ -1045,16 +1230,16 @@ generic_mod(struct lu_module *module, co
memmove(line + strlen(new_line), rest,
contents + st.st_size + 1 - rest);
memcpy(line, new_line, strlen(new_line));
- if (lseek(fd, line - contents, SEEK_SET) == -1) {
+ if (lseek(e->new_fd, line - contents, SEEK_SET) == -1) {
lu_error_new(error, lu_error_write, NULL);
goto err_contents;
}
len = strlen(line);
- if ((size_t)write(fd, line, len) != len) {
+ if ((size_t)write(e->new_fd, line, len) != len) {
lu_error_new(error, lu_error_write, NULL);
goto err_contents;
}
- if (ftruncate(fd, (line - contents) + len) != 0) {
+ if (ftruncate(e->new_fd, (line - contents) + len) != 0) {
lu_error_new(error, lu_error_write, NULL);
goto err_contents;
}
@@ -1063,16 +1248,11 @@ generic_mod(struct lu_module *module, co
err_contents:
g_free(contents);
-err_lock:
- lu_util_lock_free(lock);
-err_fd:
- close(fd);
-err_fscreate:
- lu_util_fscreate_restore(fscreate);
+err_editing:
+ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */
err_new_line:
g_free(new_line);
-err_filename:
- g_free(filename);
+err_current_name:
g_free(current_name);
return ret;
}
@@ -1118,16 +1298,14 @@ static gboolean
generic_del(struct lu_module *module, const char *file_suffix,
struct lu_ent *ent, struct lu_error **error)
{
- lu_security_context_t fscreate;
+ struct editing *e;
char *name;
- char *contents, *filename;
+ char *contents;
char *fragment2;
struct stat st;
size_t len;
- int fd;
- gboolean ret = FALSE;
+ gboolean commit = FALSE, ret = FALSE;
gboolean found;
- gpointer lock;
/* Get the entity's current name. */
if (ent->type == lu_user)
@@ -1141,42 +1319,23 @@ generic_del(struct lu_module *module, co
g_assert(module != NULL);
g_assert(ent != NULL);
- filename = module_filename(module, file_suffix);
-
- if (!lu_util_fscreate_save(&fscreate, error))
- goto err_filename;
- if (!lu_util_fscreate_from_file(filename, error))
- goto err_fscreate;
- /* Create a backup of that file. */
- if (lu_files_create_backup(filename, error) == FALSE)
- goto err_fscreate;
-
- /* Open the file to be modified. */
- fd = open(filename, O_RDWR);
- if (fd == -1) {
- lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
- strerror(errno));
- goto err_fscreate;
- }
-
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
+ e = editing_open(module, file_suffix, error);
+ if (e == NULL)
+ goto err_name;
/* Determine the file's size. */
- if (fstat(fd, &st) == -1) {
+ if (fstat(e->new_fd, &st) == -1) {
lu_error_new(error, lu_error_stat,
- _("couldn't stat `%s': %s"), filename,
+ _("couldn't stat `%s': %s"), e->new_filename,
strerror(errno));
- goto err_lock;
+ goto err_editing;
}
/* Allocate space to hold the file and read it all in. */
contents = g_malloc(st.st_size + 1);
- if (read(fd, contents, st.st_size) != st.st_size) {
+ if (read(e->new_fd, contents, st.st_size) != st.st_size) {
lu_error_new(error, lu_error_read,
- _("couldn't read from `%s': %s"), filename,
+ _("couldn't read from `%s': %s"), e->new_filename,
strerror(errno));
goto err_contents;
}
@@ -1229,41 +1388,38 @@ generic_del(struct lu_module *module, co
/* Otherwise we need to write the new data to the file. Jump back to
* the beginning of the file. */
- if (lseek(fd, 0, SEEK_SET) == -1) {
+ if (lseek(e->new_fd, 0, SEEK_SET) == -1) {
lu_error_new(error, lu_error_write,
- _("couldn't write to `%s': %s"), filename,
+ _("couldn't write to `%s': %s"), e->new_filename,
strerror(errno));
goto err_contents;
}
/* Write the new contents out. */
- if ((size_t)write(fd, contents, len) != len) {
+ if ((size_t)write(e->new_fd, contents, len) != len) {
lu_error_new(error, lu_error_write,
- _("couldn't write to `%s': %s"), filename,
+ _("couldn't write to `%s': %s"), e->new_filename,
strerror(errno));
goto err_contents;
}
/* Truncate the file to the new (certainly shorter) length. */
- if (ftruncate(fd, len) == -1) {
+ if (ftruncate(e->new_fd, len) == -1) {
lu_error_new(error, lu_error_generic,
- _("couldn't write to `%s': %s"), filename,
+ _("couldn't write to `%s': %s"), e->new_filename,
strerror(errno));
goto err_contents;
}
+ commit = TRUE;
ret = TRUE;
/* Fall through */
err_contents:
g_free(contents);
- err_lock:
- lu_util_lock_free(lock);
- err_fd:
- close(fd);
-err_fscreate:
- lu_util_fscreate_restore(fscreate);
- err_filename:
- g_free(filename);
+err_editing:
+ /* Commit/rollback happens here. */
+ ret = editing_close(e, commit, ret, error);
+err_name:
g_free(name);
return ret;
}
@@ -1344,12 +1500,9 @@ static gboolean
generic_lock(struct lu_module *module, const char *file_suffix, int field,
struct lu_ent *ent, enum lock_op op, struct lu_error **error)
{
- lu_security_context_t fscreate;
- char *filename;
+ struct editing *e;
char *value, *new_value, *name;
- int fd;
- gpointer lock;
- gboolean ret = FALSE;
+ gboolean commit = FALSE, ret = FALSE;
/* Get the name which keys the entries of interest in the file. */
g_assert((ent->type == lu_user) || (ent->type == lu_group));
@@ -1362,33 +1515,14 @@ generic_lock(struct lu_module *module, c
g_assert(module != NULL);
g_assert(ent != NULL);
- filename = module_filename(module, file_suffix);
-
- if (!lu_util_fscreate_save(&fscreate, error))
- goto err_filename;
- if (!lu_util_fscreate_from_file(filename, error))
- goto err_fscreate;
- /* Create a backup of the file. */
- if (lu_files_create_backup(filename, error) == FALSE)
- goto err_fscreate;
-
- /* Open the file. */
- fd = open(filename, O_RDWR);
- if (fd == -1) {
- lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
- strerror(errno));
- goto err_fscreate;
- }
-
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
+ e = editing_open(module, file_suffix, error);
+ if (e == NULL)
+ goto err_name;
/* Read the old value from the file. */
- value = lu_util_field_read(fd, name, field, error);
+ value = lu_util_field_read(e->new_fd, name, field, error);
if (value == NULL)
- goto err_lock;
+ goto err_editing;
/* Check that we actually care about this. If there's a non-empty,
* not locked string in there, but it's too short to be a hash, then
@@ -1396,27 +1530,27 @@ generic_lock(struct lu_module *module, c
if (LU_CRYPT_INVALID(value)) {
g_free(value);
ret = TRUE;
- goto err_lock;
+ goto err_editing;
}
/* Generate a new value for the file. */
new_value = lock_process(value, op, ent, error);
g_free(value);
if (new_value == NULL)
- goto err_lock;
+ goto err_editing;
/* Make the change. */
- ret = lu_util_field_write(fd, name, field, new_value, error);
+ if (lu_util_field_write(e->new_fd, name, field, new_value, error)
+ == FALSE)
+ goto err_editing;
+ commit = TRUE;
+ ret = TRUE;
/* Fall through */
-err_lock:
- lu_util_lock_free(lock);
- err_fd:
- close(fd);
-err_fscreate:
- lu_util_fscreate_restore(fscreate);
- err_filename:
- g_free(filename);
+err_editing:
+ /* Commit/rollback happens here. */
+ ret = editing_close(e, commit, ret, error);
+err_name:
g_free(name);
return ret;
}
@@ -1429,7 +1563,6 @@ generic_is_locked(struct lu_module *modu
char *filename;
char *value, *name;
int fd;
- gpointer lock;
gboolean ret = FALSE;
/* Get the name of this account. */
@@ -1454,22 +1587,16 @@ generic_is_locked(struct lu_module *modu
goto err_filename;
}
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
-
/* Read the value. */
value = lu_util_field_read(fd, name, field, error);
if (value == NULL)
- goto err_lock;
+ goto err_fd;
/* It all comes down to this. */
ret = value[0] == '!';
g_free(value);
/* Fall through */
-err_lock:
- lu_util_lock_free(lock);
err_fd:
close(fd);
err_filename:
@@ -1624,10 +1751,8 @@ generic_setpass(struct lu_module *module
struct lu_ent *ent, const char *password, gboolean is_shadow,
struct lu_error **error)
{
- lu_security_context_t fscreate;
- char *filename, *value, *name;
- int fd;
- gpointer lock;
+ struct editing *e;
+ char *value, *name;
gboolean ret = FALSE;
/* Get the name of this account. */
@@ -1641,34 +1766,14 @@ generic_setpass(struct lu_module *module
g_assert(module != NULL);
g_assert(ent != NULL);
- filename = module_filename(module, file_suffix);
-
- if (!lu_util_fscreate_save(&fscreate, error))
- goto err_filename;
- if (!lu_util_fscreate_from_file(filename, error))
- goto err_fscreate;
-
- /* Create a backup of the file. */
- if (lu_files_create_backup(filename, error) == FALSE)
- goto err_filename;
-
- /* Open the file. */
- fd = open(filename, O_RDWR);
- if (fd == -1) {
- lu_error_new(error, lu_error_open,
- _("couldn't open `%s': %s"), filename,
- strerror(errno));
- goto err_fscreate;
- }
-
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL)
- goto err_fd;
+ e = editing_open(module, file_suffix, error);
+ if (e == NULL)
+ goto err_name;
/* Read the current contents of the field. */
- value = lu_util_field_read(fd, name, field, error);
+ value = lu_util_field_read(e->new_fd, name, field, error);
if (value == NULL)
- goto err_lock;
+ goto err_editing;
/* pam_unix uses shadow passwords only if pw_passwd is "x"
(or ##${username}). Make sure to preserve the shadow marker
@@ -1693,9 +1798,9 @@ generic_setpass(struct lu_module *module
else if (g_ascii_strncasecmp(password, LU_CRYPTED, strlen(LU_CRYPTED))
== 0) {
password = password + strlen(LU_CRYPTED);
- if (strchr(password, ':') != NULL) {
+ if (strpbrk(password, ":\n") != NULL) {
lu_error_new(error, lu_error_invalid_attribute_value,
- _("`:' not allowed in encrypted "
+ _("`:' and `\\n' not allowed in encrypted "
"password"));
goto err_value;
}
@@ -1713,19 +1818,14 @@ generic_setpass(struct lu_module *module
}
/* Now write our changes to the file. */
- ret = lu_util_field_write(fd, name, field, password, error);
+ ret = lu_util_field_write(e->new_fd, name, field, password, error);
/* Fall through */
- err_value:
+err_value:
g_free(value);
-err_lock:
- lu_util_lock_free(lock);
- err_fd:
- close(fd);
-err_fscreate:
- lu_util_fscreate_restore(fscreate);
- err_filename:
- g_free(filename);
+err_editing:
+ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */
+err_name:
g_free(name);
return ret;
}
@@ -1802,7 +1902,6 @@ lu_files_enumerate(struct lu_module *mod
const char *pattern, struct lu_error **error)
{
int fd;
- gpointer lock;
GValueArray *ret;
GValue value;
char *buf;
@@ -1824,20 +1923,12 @@ lu_files_enumerate(struct lu_module *mod
return NULL;
}
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- g_free(filename);
- return NULL;
- }
-
/* Wrap the file for stdio operations. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), filename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
g_free(filename);
return NULL;
@@ -1873,7 +1964,6 @@ lu_files_enumerate(struct lu_module *mod
/* Clean up. */
g_value_unset(&value);
- lu_util_lock_free(lock);
fclose(fp);
g_free(filename);
@@ -1902,7 +1992,6 @@ lu_files_users_enumerate_by_group(struct
struct lu_error **error)
{
int fd;
- gpointer lock;
GValueArray *ret;
GValue value;
char *buf, grp[CHUNK_SIZE];
@@ -1927,21 +2016,12 @@ lu_files_users_enumerate_by_group(struct
return NULL;
}
- /* Lock the passwd file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- g_free(pwdfilename);
- g_free(grpfilename);
- return NULL;
- }
-
/* Wrap the descriptor in a stdio FILE. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), pwdfilename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
g_free(pwdfilename);
g_free(grpfilename);
@@ -2000,7 +2080,6 @@ lu_files_users_enumerate_by_group(struct
}
/* Close the file. */
g_value_unset(&value);
- lu_util_lock_free(lock);
fclose(fp);
/* Open the group file. */
@@ -2015,22 +2094,12 @@ lu_files_users_enumerate_by_group(struct
return NULL;
}
- /* Lock the group file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- g_free(pwdfilename);
- g_free(grpfilename);
- g_value_array_free(ret);
- return NULL;
- }
-
/* Wrap the group file in an stdio file. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), grpfilename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
g_free(pwdfilename);
g_free(grpfilename);
@@ -2085,7 +2154,6 @@ lu_files_users_enumerate_by_group(struct
}
/* Clean up. */
- lu_util_lock_free(lock);
fclose(fp);
g_free(pwdfilename);
@@ -2102,7 +2170,6 @@ lu_files_groups_enumerate_by_user(struct
struct lu_error **error)
{
int fd;
- gpointer lock;
GValueArray *ret;
GValue value;
char *buf;
@@ -2126,19 +2193,12 @@ lu_files_groups_enumerate_by_user(struct
goto err_pwdfilename;
}
- /* Lock it. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- goto err_pwdfilename;
- }
-
/* Open it so that we can use stdio. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), pwdfilename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
goto err_pwdfilename;
}
@@ -2186,7 +2246,6 @@ lu_files_groups_enumerate_by_user(struct
}
g_free(buf);
}
- lu_util_lock_free(lock);
fclose(fp);
/* Open the groups file. */
@@ -2198,19 +2257,12 @@ lu_files_groups_enumerate_by_user(struct
goto err_key;
}
- /* Lock it. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- goto err_key;
- }
-
/* Open it so that we can use stdio. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), grpfilename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
goto err_key;
}
@@ -2267,7 +2319,6 @@ lu_files_groups_enumerate_by_user(struct
g_free(key);
g_value_unset(&value);
- lu_util_lock_free(lock);
fclose(fp);
g_free(pwdfilename);
g_free(grpfilename);
@@ -2291,7 +2342,6 @@ lu_files_enumerate_full(struct lu_module
struct lu_error **error)
{
int fd;
- gpointer lock;
GPtrArray *ret = NULL;
char *buf;
char *key, *filename;
@@ -2311,19 +2361,12 @@ lu_files_enumerate_full(struct lu_module
goto err_filename;
}
- /* Lock the file. */
- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) {
- close(fd);
- goto err_filename;
- }
-
/* Wrap the file up in stdio. */
fp = fdopen(fd, "r");
if (fp == NULL) {
lu_error_new(error, lu_error_open,
_("couldn't open `%s': %s"), filename,
strerror(errno));
- lu_util_lock_free(lock);
close(fd);
goto err_filename;
}
@@ -2358,7 +2401,6 @@ lu_files_enumerate_full(struct lu_module
g_free(key);
}
- lu_util_lock_free(lock);
fclose(fp);
err_filename:
diff -up libuser-0.60/tests/files_test.py.CVE-2015-3246 libuser-0.60/tests/files_test.py
--- libuser-0.60/tests/files_test.py.CVE-2015-3246 2013-10-12 23:56:08.000000000 +0200
+++ libuser-0.60/tests/files_test.py 2015-07-08 15:15:14.061544074 +0200
@@ -262,6 +262,21 @@ class Tests(unittest.TestCase):
e = self.a.initUser('user6_8')
self.assertRaises(RuntimeError, self.a.addUser, e, False, False)
+ def testUserAdd9(self):
+ # '\n' in field values
+ # libuser.USERPASSWORD not tested because lu_shadow_user_add_prep()
+ # always replaces it by 'x'. libuser.UIDNUMBER not tested because
+ # ent_has_name_and_id() interprets the value as a number.
+ for field in (libuser.USERNAME, libuser.GIDNUMBER, libuser.GECOS,
+ libuser.HOMEDIRECTORY, libuser.LOGINSHELL,
+ libuser.SHADOWPASSWORD, libuser.SHADOWLASTCHANGE,
+ libuser.SHADOWMIN, libuser.SHADOWMAX,
+ libuser.SHADOWWARNING, libuser.SHADOWINACTIVE,
+ libuser.SHADOWEXPIRE, libuser.SHADOWFLAG):
+ e = self.a.initUser('user_6_9' + field)
+ e[field] = str(e[field][0]) + '\nx'
+ self.assertRaises(RuntimeError, self.a.addUser, e, False, False)
+
def testUserMod1(self):
# A minimal case
e = self.a.initUser('user7_1')
@@ -421,6 +436,26 @@ class Tests(unittest.TestCase):
e[libuser.USERNAME] = 'user7_7'
self.assertRaises(RuntimeError, self.a.modifyUser, e, False)
+ def testUserMod8(self):
+ # '\n' in field values
+ # libuser.USERPASSWORD not tested because lu_shadow_user_add_prep()
+ # always replaces it by 'x'. libuser.UIDNUMBER not tested because
+ # ent_has_name_and_id() interprets the value as a number.
+ for field in (libuser.USERNAME, libuser.USERPASSWORD, libuser.GIDNUMBER,
+ libuser.GECOS, libuser.HOMEDIRECTORY, libuser.LOGINSHELL,
+ libuser.SHADOWPASSWORD, libuser.SHADOWLASTCHANGE,
+ libuser.SHADOWMIN, libuser.SHADOWMAX,
+ libuser.SHADOWWARNING, libuser.SHADOWINACTIVE,
+ libuser.SHADOWEXPIRE, libuser.SHADOWFLAG):
+ e = self.a.initUser('user7_9' + field)
+ self.a.addUser(e, False, False)
+ del e
+ e = self.a.lookupUserByName('user7_9' + field)
+ self.assertIsNotNone(e)
+ self.assertNotIn('\n', str(e[field][0]))
+ e[field] = str(e[field][0]) + '\nx'
+ self.assertRaises(RuntimeError, self.a.modifyUser, e, False)
+
def testUserDel(self):
e = self.a.initUser('user8_1')
self.a.addUser(e, False, False)
@@ -619,6 +654,22 @@ class Tests(unittest.TestCase):
crypted = crypt.crypt('a:b', e[libuser.SHADOWPASSWORD][0])
self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted])
+ def testUserSetpass5(self):
+ # '\n' in field value
+ e = self.a.initUser('user12_5')
+ self.a.addUser(e, False, False)
+ del e
+ e = self.a.lookupUserByName('user12_5')
+ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0])
+ self.assertRaises(SystemError, self.a.setpassUser, e, 'a\nb', True)
+ self.a.setpassUser(e, 'a\nb', False)
+ del e
+ e = self.a.lookupUserByName('user12_5')
+ self.assertEqual(e[libuser.SHADOWPASSWORD][0][:3], '$1$')
+ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0])
+ crypted = crypt.crypt('a\nb', e[libuser.SHADOWPASSWORD][0])
+ self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted])
+
def testUserRemovepass(self):
e = self.a.initUser('user13_1')
e[libuser.SHADOWPASSWORD] = '03dgZm5nZvqOc'
@@ -884,6 +935,20 @@ class Tests(unittest.TestCase):
e = self.a.initGroup('group21_5')
self.assertRaises(RuntimeError, self.a.addGroup, e)
+ def testGroupAdd6(self):
+ # '\n' in field values
+ # libuser.GROUPPASSWORD not tested because lu_shadow_group_add_prep()
+ # always replaces it by 'x'. libuser.GIDNUMBER not tested because
+ # ent_has_name_and_id() interprets the value as a number.
+ for field in (libuser.GROUPNAME, libuser.SHADOWPASSWORD,
+ libuser.MEMBERNAME, libuser.ADMINISTRATORNAME):
+ e = self.a.initGroup('group_21_6' + field)
+ if e.has_key(field):
+ e[field] = str(e[field][0]) + '\nx'
+ else:
+ e[field] = field + '\nx'
+ self.assertRaises(RuntimeError, self.a.addGroup, e)
+
def testGroupMod1(self):
# A minimal case
e = self.a.initGroup('group22_1')
@@ -997,6 +1062,25 @@ class Tests(unittest.TestCase):
e[libuser.GROUPNAME] = 'group22_6'
self.assertRaises(RuntimeError, self.a.modifyGroup, e)
+ def testGroupMod7(self):
+ # '\n' in field values
+ # libuser.GIDNUMBER not tested because ent_has_name_and_id() interprets
+ # the value as a number.
+ for field in (libuser.GROUPNAME, libuser.GROUPPASSWORD,
+ libuser.SHADOWPASSWORD, libuser.MEMBERNAME,
+ libuser.ADMINISTRATORNAME):
+ e = self.a.initGroup('group22_8' + field)
+ self.a.addGroup(e)
+ del e
+ e = self.a.lookupGroupByName('group22_8' + field)
+ self.assertIsNotNone(e)
+ if e.has_key(field):
+ self.assertNotIn('\n', str(e[field][0]))
+ e[field] = str(e[field][0]) + '\nx'
+ else:
+ e[field] = field + '\nx'
+ self.assertRaises(RuntimeError, self.a.modifyGroup, e)
+
def testGroupDel(self):
e = self.a.initGroup('group23_1')
self.a.addGroup(e)
@@ -1190,6 +1274,22 @@ class Tests(unittest.TestCase):
crypted = crypt.crypt('a:b', e[libuser.SHADOWPASSWORD][0])
self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted])
+ def testGroupSetpass4(self):
+ # '\n' in field value
+ e = self.a.initGroup('group27_5')
+ self.a.addGroup(e)
+ del e
+ e = self.a.lookupGroupByName('group27_5')
+ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0])
+ self.assertRaises(SystemError, self.a.setpassGroup, e, 'a\nb', True)
+ self.a.setpassGroup(e, 'a\nb', False)
+ del e
+ e = self.a.lookupGroupByName('group27_5')
+ self.assertEqual(e[libuser.SHADOWPASSWORD][0][:3], '$1$')
+ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0])
+ crypted = crypt.crypt('a\nb', e[libuser.SHADOWPASSWORD][0])
+ self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted])
+
def testGroupRemovepass(self):
e = self.a.initGroup('group28_1')
e[libuser.SHADOWPASSWORD] = '07Js7N.eEhbgs'