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