visudo enhancement to edit-syntax-check arbitrary files

[Bob Proulx]

Proposal: Add an option to visudo to allow it to edit and thereby
syntax check arbitrary files.

The visudo command is great for many system configurations.  But it
does not work well for us.  Let me explain.  Here we try to use an
infrastructure where configuration management is done on files such as
/etc/sudoers.  The goal is to administer a network of machines as a
group and not a group of individual machines which are all uniquely
different.  The desired process is to make changes and check them into
revision control and then the distribution to the network of client
hosts is from revision control.  This way all changes can be reverted
in the case of problems.

This works well except that *if* no syntax checking is done on the
sudoers file before it is checked into revision control then it can be
distributed with broken syntax.  Unfortunately the visudo command only
edits the system live /etc/sudoers file.  And in order to use it we
need root capability and we need to change local system file first.
Neither of which were really what I wanted.  Even though I am root on
thousands of machines I try to operate as non-root when not needed.
And besides, if all of the changes in the revision control logs are
done as root it hides who was really making what changes.

Therefore I modified the visudo program to be able to edit and syntax
check any arbitrary file given on the command line.  This allows the
sudoers file to be edited and prepared offline prior to revision
control check-in and subsequent distribution.  It guards against
mistakes as the visudo program was designed to do.

In order to make this proposal concrete I am including my changes
against the 1.6.3p6 visudo.c file for review.  It would be most kind
of you to include this or similar capability in a future release of
sudo.  If response is favorable I can include man page and usage
string updates.  Discussion of /etc/sudoers configuration management
best practices is also appreciated.

Thanks
Bob


diff -ru ../sudo-1.6.3p6.original/visudo.c ./visudo.c
--- ../sudo-1.6.3p6.original/visudo.c	Sun Jun  4 20:39:01 2000
+++ ./visudo.c	Thu Dec  6 19:17:32 2001
@@ -140,12 +140,22 @@
     /*
      * Arg handling.
      */
-    while (--argc) {
-	if (!strcmp(argv[argc], "-V")) {
+    for (n = 1; n < argc; ++n) {
+	if (!strcmp(argv[n], "-V")) {
 	    (void) printf("visudo version %s\n", version);
 	    exit(0);
-	} else if (!strcmp(argv[argc], "-s")) {
+	} else if (!strcmp(argv[n], "-s")) {
 	    pedantic++;			/* strict mode */
+	} else if (!strncmp(argv[n], "-f", 2)) {
+	    if (argv[n][2] != 0) {
+	        sudoers = &argv[n][2];
+	    } else {
+	        ++n;
+		sudoers = argv[n];
+	    }
+	    stmp = malloc(strlen(sudoers) + sizeof ".tmp");
+	    strcpy(stmp,sudoers);
+	    strcat(stmp,".tmp");
 	} else {
 	    usage();
 	}
@@ -190,7 +200,7 @@
     /*
      * Open sudoers temp file.
      */
-    stmp_fd = open(stmp, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    stmp_fd = open(stmp, O_WRONLY | O_CREAT | O_TRUNC, SUDOERS_MODE + S_IWUSR);
     if (stmp_fd < 0) {
 	(void) fprintf(stderr, "%s: %s: %s\n", Argv[0], stmp, strerror(errno));
 	exit(1);
@@ -330,13 +340,13 @@
      * Change mode and ownership of temp file so when
      * we move it to sudoers things are kosher.
      */
-    if (chown(stmp, SUDOERS_UID, SUDOERS_GID)) {
+    if (geteuid() == 0 && chown(stmp, SUDOERS_UID, SUDOERS_GID)) {
 	(void) fprintf(stderr,
 	    "%s: Unable to set (uid, gid) of %s to (%d, %d): %s\n",
 	    Argv[0], stmp, SUDOERS_UID, SUDOERS_GID, strerror(errno));
 	Exit(-1);
     }
-    if (chmod(stmp, SUDOERS_MODE)) {
+    if (geteuid() == 0 && chmod(stmp, SUDOERS_MODE)) {
 	(void) fprintf(stderr,
 	    "%s: Unable to change mode of %s to %o: %s\n",
 	    Argv[0], stmp, SUDOERS_MODE, strerror(errno));