visudo: add -O and -P options to check/set owner and permissions.

This can be used in conjunction with the -c option to check that
the sudoers file ownership and permissions are correct.
Bug #1007

doc/visudo.man.in

@@ -21,7 +21,7 @@
 .\" Agency (DARPA) and Air Force Research Laboratory, Air Force
 .\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
 .\"
-.TH "VISUDO" "@mansectsu@" "September 17, 2021" "Sudo @PACKAGE_VERSION@" "System Manager's Manual"
+.TH "VISUDO" "@mansectsu@" "November 6, 2021" "Sudo @PACKAGE_VERSION@" "System Manager's Manual"
 .nh
 .if n .ad l
 .SH "NAME"
@@ -30,7 +30,7 @@
 .SH "SYNOPSIS"
 .HP 7n
 \fBvisudo\fR
-[\fB\-chqsV\fR]
+[\fB\-chOPqsV\fR]
 [[\fB\-f\fR]\ \fIsudoers\fR]
 .SH "DESCRIPTION"
 \fBvisudo\fR
@@ -183,7 +183,11 @@ If the path to the
 \fIsudoers\fR
 file was not specified,
 \fBvisudo\fR
-will also check the file owner and mode.
+will also check the file ownership and permissions (see the
+\fB\-O\fR
+and
+\fB\-P\fR
+options).
 A message will be printed to the standard output describing the status of
 \fIsudoers\fR
 unless the
@@ -209,6 +213,30 @@ option.
 \fB\-h\fR, \fB\--help\fR
 Display a short help message to the standard output and exit.
 .TP 12n
+\fB\-O\fR, \fB\--owner\fR
+Enforce the default ownership (user and group) of the
+\fIsudoers\fR
+file.
+In edit mode, the owner of the edited file will be set to the default.
+In check mode
+(\fB\-c\fR),
+an error will be reported if the owner is incorrect.
+This option is enabled by default if the
+\fIsudoers\fR
+file was not specified.
+.TP 12n
+\fB\-P\fR, \fB\--perms\fR
+Enforce the default permissions (mode) of the
+\fIsudoers\fR
+file.
+In edit mode, the permissions of the edited file will be set to the default.
+In check mode
+(\fB\-c\fR),
+an error will be reported if the file permissions are incorrect.
+This option is enabled by default if the
+\fIsudoers\fR
+file was not specified.
+.TP 12n
 \fB\-q\fR, \fB\--quiet\fR
 Enable
 \fIquiet\fR

doc/visudo.mdoc.in

@@ -20,7 +20,7 @@
 .\" Agency (DARPA) and Air Force Research Laboratory, Air Force
 .\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
 .\"
-.Dd September 17, 2021
+.Dd November 6, 2021
 .Dt VISUDO @mansectsu@
 .Os Sudo @PACKAGE_VERSION@
 .Sh NAME
@@ -28,7 +28,7 @@
 .Nd edit the sudoers file
 .Sh SYNOPSIS
 .Nm visudo
-.Op Fl chqsV
+.Op Fl chOPqsV
 .Op Bo Fl f Bc Ar sudoers
 .Sh DESCRIPTION
 .Nm
@@ -181,7 +181,11 @@ If the path to the
 .Em sudoers
 file was not specified,
 .Nm
-will also check the file owner and mode.
+will also check the file ownership and permissions (see the
+.Fl O
+and
+.Fl P
+options).
 A message will be printed to the standard output describing the status of
 .Em sudoers
 unless the
@@ -204,6 +208,28 @@ path can be specified without using the
 option.
 .It Fl h , -help
 Display a short help message to the standard output and exit.
+.It Fl O , -owner
+Enforce the default ownership (user and group) of the
+.Em sudoers
+file.
+In edit mode, the owner of the edited file will be set to the default.
+In check mode
+.Pq Fl c ,
+an error will be reported if the owner is incorrect.
+This option is enabled by default if the
+.Em sudoers
+file was not specified.
+.It Fl P , -perms
+Enforce the default permissions (mode) of the
+.Em sudoers
+file.
+In edit mode, the permissions of the edited file will be set to the default.
+In check mode
+.Pq Fl c ,
+an error will be reported if the file permissions are incorrect.
+This option is enabled by default if the
+.Em sudoers
+file was not specified.
 .It Fl q , -quiet
 Enable
 .Em quiet

plugins/sudoers/visudo.c

@@ -86,9 +86,9 @@ TAILQ_HEAD(sudoersfile_list, sudoersfile);
 static void quit(int);
 static int whatnow(void);
 static char *get_editor(int *editor_argc, char ***editor_argv);
-static bool check_syntax(const char *, bool, bool, bool);
+static bool check_syntax(const char *, bool, bool, bool, bool);
 static bool edit_sudoers(struct sudoersfile *, char *, int, char **, int);
-static bool install_sudoers(struct sudoersfile *, bool);
+static bool install_sudoers(struct sudoersfile *, bool, bool);
 static int print_unused(struct sudoers_parse_tree *, struct alias *, void *);
 static bool reparse_sudoers(char *, int, char **, bool, bool);
 static int run_command(char *, char **);
@@ -107,12 +107,14 @@ struct sudo_user sudo_user;
 struct passwd *list_pw;
 static struct sudoersfile_list sudoerslist = TAILQ_HEAD_INITIALIZER(sudoerslist);
 static bool checkonly;
-static const char short_opts[] =  "cf:hqsVx:";
+static const char short_opts[] =  "cf:hOPqsVx:";
 static struct option long_opts[] = {
     { "check",		no_argument,		NULL,	'c' },
     { "export",		required_argument,	NULL,	'x' },
     { "file",		required_argument,	NULL,	'f' },
     { "help",		no_argument,		NULL,	'h' },
+    { "owner",		no_argument,		NULL,	'O' },
+    { "perms",		no_argument,		NULL,	'P' },
     { "quiet",		no_argument,		NULL,	'q' },
     { "strict",		no_argument,		NULL,	's' },
     { "version",	no_argument,		NULL,	'V' },
@@ -128,7 +130,7 @@ main(int argc, char *argv[])
     char *editor, **editor_argv;
     const char *export_path = NULL;
     int ch, oldlocale, editor_argc, exitcode = 0;
-    bool quiet, strict, fflag;
+    bool use_perms, use_owner, quiet, strict, fflag;
     debug_decl(main, SUDOERS_DEBUG_MAIN);
 
 #if defined(SUDO_DEVEL) && defined(__OpenBSD__)
@@ -168,7 +170,7 @@ main(int argc, char *argv[])
     /*
      * Arg handling.
      */
-    checkonly = fflag = quiet = strict = false;
+    fflag = quiet = strict = use_owner = use_perms = false;
     while ((ch = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) {
 	switch (ch) {
 	    case 'V':
@@ -187,6 +189,12 @@ main(int argc, char *argv[])
 	    case 'h':
 		help();
 		break;
+	    case 'O':
+		use_owner = true;	/* check/set owner */
+		break;
+	    case 'P':
+		use_perms = true;	/* check/set perms */
+		break;
 	    case 's':
 		strict = true;		/* strict mode */
 		break;
@@ -218,6 +226,12 @@ main(int argc, char *argv[])
 	usage(1);
     }
 
+    /* Check/set owner and mode for installed sudoers file. */
+    if (!fflag) {
+	use_owner = true;
+	use_perms = true;
+    }
+
     if (export_path != NULL) {
 	/* Backward compatibility for the time being. */
 	sudo_warnx("%s",
@@ -247,7 +261,8 @@ main(int argc, char *argv[])
 	sudo_fatalx("%s", U_("unable to initialize sudoers default values"));
 
     if (checkonly) {
-	exitcode = check_syntax(sudoers_file, quiet, strict, fflag) ? 0 : 1;
+	exitcode = check_syntax(sudoers_file, quiet, strict, use_owner,
+	    use_perms) ? 0 : 1;
 	goto done;
     }
 
@@ -287,7 +302,10 @@ main(int argc, char *argv[])
      */
     if (reparse_sudoers(editor, editor_argc, editor_argv, strict, quiet)) {
 	TAILQ_FOREACH(sp, &sudoerslist, entries) {
-	    (void) install_sudoers(sp, fflag);
+	    if (!install_sudoers(sp, use_owner, use_perms)) {
+		sudo_warnx(U_("contents of edit session left in %s"), sp->tpath);
+		exitcode = 1;
+	    }
 	}
     }
     free(editor);
@@ -496,7 +514,7 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
 	/*
 	 * Check for zero length sudoers file.
 	 */
-	if (stat(sp->tpath, &sb) < 0) {
+	if (stat(sp->tpath, &sb) == -1) {
 	    sudo_warnx(U_("unable to stat temporary file (%s), %s unchanged"),
 		sp->tpath, sp->path);
 	    goto done;
@@ -676,7 +694,7 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv,
  * move it into place.  Returns true on success, else false.
  */
 static bool
-install_sudoers(struct sudoersfile *sp, bool oldperms)
+install_sudoers(struct sudoersfile *sp, bool set_owner, bool set_perms)
 {
     struct stat sb;
     bool ret = false;
@@ -690,19 +708,22 @@ install_sudoers(struct sudoersfile *sp, bool oldperms)
 	 * No changes but fix owner/mode if needed.
 	 */
 	(void) unlink(sp->tpath);
-	if (!oldperms && fstat(sp->fd, &sb) != -1) {
+	if (fstat(sp->fd, &sb) == 0) {
-	    if (sb.st_uid != sudoers_uid || sb.st_gid != sudoers_gid) {
+	    if (set_owner) {
-		if (chown(sp->path, sudoers_uid, sudoers_gid) != 0) {
+		if (sb.st_uid != sudoers_uid || sb.st_gid != sudoers_gid) {
-		    sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
+		    if (chown(sp->path, sudoers_uid, sudoers_gid) != 0) {
-			"%s: unable to chown %d:%d %s", __func__,
+			sudo_warn(U_("unable to set (uid, gid) of %s to (%u, %u)"),
-			(int)sudoers_uid, (int)sudoers_gid, sp->path);
+			    sp->path, (unsigned int)sudoers_uid,
+			    (unsigned int)sudoers_gid);
+		    }
 		}
 	    }
-	    if ((sb.st_mode & ACCESSPERMS) != sudoers_mode) {
+	    if (set_perms) {
-		if (chmod(sp->path, sudoers_mode) != 0) {
+		if ((sb.st_mode & ACCESSPERMS) != sudoers_mode) {
-		    sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
+		    if (chmod(sp->path, sudoers_mode) != 0) {
-			"%s: unable to chmod 0%o %s", __func__,
+			sudo_warn(U_("unable to change mode of %s to 0%o"),
-			(int)sudoers_mode, sp->path);
+			    sp->path, (unsigned int)sudoers_mode);
+		    }
 		}
 	    }
 	}
@@ -714,30 +735,35 @@ install_sudoers(struct sudoersfile *sp, bool oldperms)
      * Change mode and ownership of temp file so when
      * we move it to sp->path things are kosher.
      */
-    if (oldperms) {
+    if (!set_owner || !set_perms) {
-	/* Use perms of the existing file.  */
+	/* Preserve owner/perms of the existing file.  */
 	if (fstat(sp->fd, &sb) == -1)
 	    sudo_fatal(U_("unable to stat %s"), sp->path);
-	if (chown(sp->tpath, sb.st_uid, sb.st_gid) != 0) {
+    }
-	    sudo_warn(U_("unable to set (uid, gid) of %s to (%u, %u)"),
+    if (set_owner) {
-		sp->tpath, (unsigned int)sb.st_uid, (unsigned int)sb.st_gid);
-	}
-	if (chmod(sp->tpath, sb.st_mode & ACCESSPERMS) != 0) {
-	    sudo_warn(U_("unable to change mode of %s to 0%o"), sp->tpath,
-		(unsigned int)(sb.st_mode & ACCESSPERMS));
-	}
-    } else {
 	if (chown(sp->tpath, sudoers_uid, sudoers_gid) != 0) {
 	    sudo_warn(U_("unable to set (uid, gid) of %s to (%u, %u)"),
 		sp->tpath, (unsigned int)sudoers_uid,
 		(unsigned int)sudoers_gid);
 	    goto done;
 	}
+    } else {
+	if (chown(sp->tpath, sb.st_uid, sb.st_gid) != 0) {
+	    sudo_warn(U_("unable to set (uid, gid) of %s to (%u, %u)"),
+		sp->tpath, (unsigned int)sb.st_uid, (unsigned int)sb.st_gid);
+	}
+    }
+    if (set_perms) {
 	if (chmod(sp->tpath, sudoers_mode) != 0) {
 	    sudo_warn(U_("unable to change mode of %s to 0%o"), sp->tpath,
 		(unsigned int)sudoers_mode);
 	    goto done;
 	}
+    } else {
+	if (chmod(sp->tpath, sb.st_mode & ACCESSPERMS) != 0) {
+	    sudo_warn(U_("unable to change mode of %s to 0%o"), sp->tpath,
+		(unsigned int)(sb.st_mode & ACCESSPERMS));
+	}
     }
 
     /*
@@ -872,26 +898,32 @@ run_command(char *path, char **argv)
 }
 
 static bool
-check_owner(const char *path, bool quiet)
+check_file(const char *path, bool quiet, bool check_owner, bool check_perms)
 {
     struct stat sb;
     bool ok = true;
-    debug_decl(check_owner, SUDOERS_DEBUG_UTIL);
+    debug_decl(check_file, SUDOERS_DEBUG_UTIL);
 
     if (stat(path, &sb) == 0) {
-	if (sb.st_uid != sudoers_uid || sb.st_gid != sudoers_gid) {
+	if (check_owner) {
-	    ok = false;
+	    if (sb.st_uid != sudoers_uid || sb.st_gid != sudoers_gid) {
-	    if (!quiet) {
+		ok = false;
-		fprintf(stderr,
+		if (!quiet) {
-		    _("%s: wrong owner (uid, gid) should be (%u, %u)\n"),
+		    fprintf(stderr,
-		    path, (unsigned int)sudoers_uid, (unsigned int)sudoers_gid);
+			_("%s: wrong owner (uid, gid) should be (%u, %u)\n"),
+			path, (unsigned int)sudoers_uid,
+			(unsigned int)sudoers_gid);
 		}
+	    }
 	}
-	if ((sb.st_mode & ALLPERMS) != sudoers_mode) {
+	if (check_perms) {
-	    ok = false;
+	    if ((sb.st_mode & ALLPERMS) != sudoers_mode) {
-	    if (!quiet) {
+		ok = false;
-		fprintf(stderr, _("%s: bad permissions, should be mode 0%o\n"),
+		if (!quiet) {
-		    path, (unsigned int)sudoers_mode);
+		    fprintf(stderr,
+			_("%s: bad permissions, should be mode 0%o\n"),
+			path, (unsigned int)sudoers_mode);
+		}
 	    }
 	}
     }
@@ -899,7 +931,8 @@ check_owner(const char *path, bool quiet)
 }
 
 static bool
-check_syntax(const char *file, bool quiet, bool strict, bool oldperms)
+check_syntax(const char *file, bool quiet, bool strict, bool check_owner,
+    bool check_perms)
 {
     bool ok = false;
     int oldlocale;
@@ -937,14 +970,14 @@ check_syntax(const char *file, bool quiet, bool strict, bool oldperms)
 	struct sudoersfile *sp;
 
 	/* Parsed OK, check mode and owner. */
-	if (oldperms || check_owner(file, quiet)) {
+	if (check_file(file, quiet, check_owner, check_perms)) {
 	    if (!quiet)
 		(void) printf(_("%s: parsed OK\n"), file);
 	} else {
 	    ok = false;
 	}
 	TAILQ_FOREACH(sp, &sudoerslist, entries) {
-	    if (oldperms || check_owner(sp->path, quiet)) {
+	    if (check_file(sp->path, quiet, check_owner, check_perms)) {
 		if (!quiet)
 		    (void) printf(_("%s: parsed OK\n"), sp->path);
 	    } else {