We wanted to avoid creating dozens of user accounts on our secure CVS
server for our developers.  This would normally be accomplished by
using the :pserver: mechanism with a CVS `passwd' that mapped them all
to a single user, like:

richard:ykNAcP.XiGXuk:cvs
edward:bKSofTdnNWXBo:cvs
george:IHFVPfPXn29Mw:cvs

But the plaintext password transmission of :pserver: was inadequate
for our needs, so :gserver: seemed our obvious choice - but then there
would be no way to map users to the singe `cvs' account (because the
:gserver: protocol (unlike :pserver:) does not know which repository
the user is going to use).  The CVS server would also need to run as
root, which was undesirable from the point of view of security.

Our solution was simple:

  (a) We changed the :gserver: method so that, instead of trying to
  setuid(2) to the user, it just compares his name to the `readers'
  and `writers' files when he tries to access a repository.  This
  means that the server can run as non-root and, since it will just
  keep running as its original user, we do not need to create a Unix
  account on the machine for each developer.

  (b) The current `readers' and `writers' scheme has an insecure
  default of permitting access, but most of our Kerberos users are not
  developers.  We therefore change (and in the process simplify) the
  meanings of these files: `writers' lists who can both read and
  write; `readers' lists who else can only read; and users listed in
  neither are denied access.

  (c) Since the /etc/krb5.keytab file is usually only readable by
  root, but our CVS server now runs as `cvs', we added a `-k keyfile'
  option to CVS with which an alternate keyfile can be specified.

A patch accomplishing the above is at the bottom of this mail.

After recompiling CVS (making sure its uses gssapi) and reinstalling,
create a user for CVS, which we called `cvs'; create a directory for
your repositories which can only be read and written by this user;
create a keytab file readable only by this user with Kerberos keys for
the principal cvs/HOSTNAME; and add the following to /etc/inetd.conf:

        cvspserver stream tcp nowait cvs /usr/local/etc/tcpd SCRIPT

where `cvspserver' corresponds to an appropriate port in /etc/services
and SCRIPT is a shell script that looks like:

        #!/bin/ksh
        cvs -k KEYTAB -f --allow-root=REPOSITORY_ROOT pserver

(We used a script to keep the command line in inetd.conf simple.)
Substitute the path to the keytab file you created for KEYTAB, and the
directory where your repositories will reside for REPOSITORY_ROOT.

Now to lock down your permissions: add `SystemAuth=no' to your
CVSROOT/config and create an empty CVSROOT/passwd file, which together
disable normal :pserver: access; then create a `readers' and `writers'
file (both are now required) listing the users to whom you wish to
grant access through :gserver:.

We welcome comments on our code and technique, and feedback about
whether others find this useful or - with further modification - might
find it useful in the future.

(Note that cvs-1.11.2 does not support :gserver: without an additional
patch - either use an earlier version, or the current version from the
development tree.)

diff -uNr cvs-orig/src/main.c cvs-1.11.2/src/main.c
--- cvs-orig/src/main.c	2002-03-19 14:15:45.000000000 -0500
+++ cvs-1.11.2/src/main.c	2002-06-24 18:07:37.557244000 -0400
@@ -21,6 +21,10 @@
 extern int gethostname ();
 #endif
 
+#ifdef HAVE_GSSAPI
+extern char *krb5_overridekeyname;
+#endif
+
 char *program_name;
 char *program_path;
 char *command_name;
@@ -263,6 +267,9 @@
     "    -a           Authenticate all net traffic.\n",
 #endif
     "    -s VAR=VAL   Set CVS user variable.\n",
+#ifdef HAVE_GSSAPI
+    "    -k keytab    Specify path to the Kerberos keytab.\n",
+#endif
     "(Specify the --help option for a list of other help options)\n",
     NULL
 };
@@ -402,12 +409,13 @@
     int tmpdir_update_env, cvs_update_env;
     int free_CVSroot = 0;
     int free_Editor = 0;
+    int free_Keytab = 0;
     int free_Tmpdir = 0;
 
     int help = 0;		/* Has the user asked for help?  This
 				   lets us support the `cvs -H cmd'
 				   convention to give help for cmd. */
-    static const char short_options[] = "+Qqrwtnlvb:T:e:d:Hfz:s:xa";
+    static const char short_options[] = "+Qqrwtnlvb:T:e:d:Hfz:s:xak:";
     static struct option long_options[] =
     {
         {"help", 0, NULL, 'H'},
@@ -620,6 +628,12 @@
                    We will issue an error later if stream
                    authentication is not supported.  */
 		break;
+	    case 'k':
+#ifdef HAVE_GSSAPI
+		krb5_overridekeyname = xstrdup (optarg);
+		free_Keytab = 1;
+#endif
+		break;
 	    case '?':
 	    default:
                 usage (usg);
@@ -1022,6 +1036,8 @@
 	free (CVSroot);
     if (free_Editor)
 	free (Editor);
+    if (free_Keytab)
+	free (krb5_overridekeyname);
     if (free_Tmpdir)
 	free (Tmpdir);
     root_allow_free ();
diff -uNr cvs-orig/src/server.c cvs-1.11.2/src/server.c
--- cvs-orig/src/server.c	2002-03-19 14:15:45.000000000 -0500
+++ cvs-1.11.2/src/server.c	2002-06-24 18:08:52.037306000 -0400
@@ -2429,176 +2429,109 @@
 }
 
 
+/* Determine whether the user is listed in a file of usernames.
+ */
+static int
+check_user_in_file_p (fname)
+    char *fname;
+{
+    FILE *fp;
+    char *linebuf = NULL;
+    size_t linebuf_len = 0;
+    int num_red = 0;
+
+    fp = fopen (fname, "r");
+    if (fp == NULL)
+    {
+	error (0, errno, "cannot open %s", fname);
+	return 0;
+    }
+
+    while ((num_red = getline (&linebuf, &linebuf_len, fp)) > 0)
+    {
+	/* Chop newline by hand, for strcmp()'s sake. */
+	if (linebuf[num_red - 1] == '\n')
+	    linebuf[num_red - 1] = '\0';
+	if (strcmp (linebuf, CVS_Username) == 0)
+	    break;		/* success - we will return 1 */
+    }
+
+    if (num_red < 0 && !feof (fp))
+	error (0, errno, "error while reading %s", fname);
+    if (linebuf)
+	free(linebuf);
+    if (fclose (fp) < 0)
+	error (0, errno, "cannot close %s", fname);
+    return num_red > 0 ? 1 : 0;
+}
 
 /* If command is legal, return 1.
- * Else if command is illegal and croak_on_illegal is set, then die.
- * Else just return 0 to indicate that command is illegal.
+ * Else return 0 to indicate that command is illegal.
  */
 static int
 check_command_legal_p (cmd_name)
     char *cmd_name;
 {
-    /* Right now, only pserver notices illegal commands -- namely,
-     * write attempts by a read-only user.  Therefore, if CVS_Username
-     * is not set, this just returns 1, because CVS_Username unset
-     * means pserver is not active.
-     */
 #ifdef AUTH_SERVER_SUPPORT
+    char *fname;
+    size_t flen;
+    int found_it = 0;
+
+    /* If CVS_Username is not set, this just returns 1, because an
+     * CVS_Username unset means file permissions are being used as the
+     * permissions mechanism.
+     */
+    
     if (CVS_Username == NULL)
         return 1;
-
+    
+    /* If the user is listed in the writers file, then any command is
+     * allowed. */
+    
+    flen = strlen (current_parsed_root->directory)
+	    + strlen (CVSROOTADM)
+	    + strlen (CVSROOTADM_WRITERS)
+	    + 3;
+    
+    fname = xmalloc (flen);
+    (void) sprintf (fname, "%s/%s/%s", current_parsed_root->directory,
+		    CVSROOTADM, CVSROOTADM_WRITERS);
+    found_it = check_user_in_file_p (fname);
+    free(fname);
+    
+    if (found_it)
+	return 1;
+    
+    /* If the operation *requires* write access, the writers file was
+     * the only chance for permission. */
+    
     if (lookup_command_attribute (cmd_name) & CVS_CMD_MODIFIES_REPOSITORY)
-    {
-        /* This command has the potential to modify the repository, so
-         * we check if the user have permission to do that.
-         *
-         * (Only relevant for remote users -- local users can do
-         * whatever normal Unix file permissions allow them to do.)
-         *
-         * The decision method:
-         *
-         *    If $CVSROOT/CVSADMROOT_READERS exists and user is listed
-         *    in it, then read-only access for user.
-         *
-         *    Or if $CVSROOT/CVSADMROOT_WRITERS exists and user NOT
-         *    listed in it, then also read-only access for user.
-         *
-         *    Else read-write access for user.
-         */
+	return 0;
+    
+    /* If the command does not require write access, we can check the
+     * readers file. */
+    
+    flen = strlen (current_parsed_root->directory)
+	    + strlen (CVSROOTADM)
+	    + strlen (CVSROOTADM_READERS)
+	    + 3;
+    
+    fname = xmalloc (flen);
+    (void) sprintf (fname, "%s/%s/%s", current_parsed_root->directory,
+		    CVSROOTADM, CVSROOTADM_READERS);
+
+    found_it = check_user_in_file_p (fname);
+    free(fname);
+    
+    if (found_it)
+	return 1;
 
-         char *linebuf = NULL;
-         int num_red = 0;
-         size_t linebuf_len = 0;
-         char *fname;
-         size_t flen;
-         FILE *fp;
-         int found_it = 0;
-
-         /* else */
-         flen = strlen (current_parsed_root->directory)
-                + strlen (CVSROOTADM)
-                + strlen (CVSROOTADM_READERS)
-                + 3;
-
-         fname = xmalloc (flen);
-         (void) sprintf (fname, "%s/%s/%s", current_parsed_root->directory,
-			CVSROOTADM, CVSROOTADM_READERS);
-
-         fp = fopen (fname, "r");
-
-         if (fp == NULL)
-	 {
-	     if (!existence_error (errno))
-	     {
-		 /* Need to deny access, so that attackers can't fool
-		    us with some sort of denial of service attack.  */
-		 error (0, errno, "cannot open %s", fname);
-		 free (fname);
-		 return 0;
-	     }
-	 }
-         else  /* successfully opened readers file */
-         {
-             while ((num_red = getline (&linebuf, &linebuf_len, fp)) >= 0)
-             {
-                 /* Hmmm, is it worth importing my own readline
-                    library into CVS?  It takes care of chopping
-                    leading and trailing whitespace, "#" comments, and
-                    newlines automatically when so requested.  Would
-                    save some code here...  -kff */
-
-                 /* Chop newline by hand, for strcmp()'s sake. */
-                 if (linebuf[num_red - 1] == '\n')
-                     linebuf[num_red - 1] = '\0';
-
-                 if (strcmp (linebuf, CVS_Username) == 0)
-                     goto handle_illegal;
-             }
-	     if (num_red < 0 && !feof (fp))
-		 error (0, errno, "cannot read %s", fname);
-
-             /* If not listed specifically as a reader, then this user
-                has write access by default unless writers are also
-                specified in a file . */
-	     if (fclose (fp) < 0)
-		 error (0, errno, "cannot close %s", fname);
-         }
-	 free (fname);
-
-	 /* Now check the writers file.  */
-
-         flen = strlen (current_parsed_root->directory)
-                + strlen (CVSROOTADM)
-                + strlen (CVSROOTADM_WRITERS)
-                + 3;
-
-         fname = xmalloc (flen);
-         (void) sprintf (fname, "%s/%s/%s", current_parsed_root->directory,
-			CVSROOTADM, CVSROOTADM_WRITERS);
-
-         fp = fopen (fname, "r");
-
-         if (fp == NULL)
-         {
-	     if (linebuf)
-	         free (linebuf);
-	     if (existence_error (errno))
-	     {
-		 /* Writers file does not exist, so everyone is a writer,
-		    by default.  */
-		 free (fname);
-		 return 1;
-	     }
-	     else
-	     {
-		 /* Need to deny access, so that attackers can't fool
-		    us with some sort of denial of service attack.  */
-		 error (0, errno, "cannot read %s", fname);
-		 free (fname);
-		 return 0;
-	     }
-         }
-
-         found_it = 0;
-         while ((num_red = getline (&linebuf, &linebuf_len, fp)) >= 0)
-         {
-             /* Chop newline by hand, for strcmp()'s sake. */
-             if (linebuf[num_red - 1] == '\n')
-                 linebuf[num_red - 1] = '\0';
-
-             if (strcmp (linebuf, CVS_Username) == 0)
-             {
-                 found_it = 1;
-                 break;
-             }
-         }
-	 if (num_red < 0 && !feof (fp))
-	     error (0, errno, "cannot read %s", fname);
-
-         if (found_it)
-         {
-             if (fclose (fp) < 0)
-		 error (0, errno, "cannot close %s", fname);
-             if (linebuf)
-                 free (linebuf);
-	     free (fname);
-             return 1;
-         }
-         else   /* writers file exists, but this user not listed in it */
-         {
-         handle_illegal:
-             if (fclose (fp) < 0)
-		 error (0, errno, "cannot close %s", fname);
-             if (linebuf)
-                 free (linebuf);
-	     free (fname);
-	     return 0;
-         }
-    }
-#endif /* AUTH_SERVER_SUPPORT */
+    /* If the user is listed in neither file, we forbid.*/
+    return 0;
 
-    /* If ever reach end of this function, command must be legal. */
+#else /* AUTH_SERVER_SUPPORT */
     return 1;
+#endif /* AUTH_SERVER_SUPPORT */
 }
 
 
@@ -6079,7 +6012,8 @@
 	    error (1, errno, "fwrite failed");
     }
 
-    switch_to_user (buf);
+    CVS_Username = xmalloc (strlen (buf) + 1);
+    strcpy (CVS_Username, buf);
 
     printf ("I LOVE YOU\n");
     fflush (stdout);

-- 
Brandon Craig Rhodes                         http://www.rhodesmill.org/brandon
Georgia Tech                                            [EMAIL PROTECTED]

Reply via email to