This edition of my unprivileged server patch is the first one which
might actually work, marking a sort of milestone in our progress with
it here. It should be applied to the stock 1.11.2 distribution.
This patch enhances security for sites using the :gserver: method.
Particularly important to us are that: the server can now run as a
user (we made a `cvs' account for it here) instead of being root; CVS
users need only exist in the Kerberos database rather than being given
accounts on the CVS machine; and the `readers' and `writers' files not
only have far more sensible and obvious meanings, but feature a secure
default - they deny access to users listed in neither file (our campus
Kerberos database has something like thirty thousand entries, most of
whom are not involved in our project).
The specific enhancements included in this patch:
Users need only present a valid Kerberos ticket, they need not have
an account on the machine where the server is running.
The LOGNAME, USER, and CVS_USER environment variables are set to
the user whom Kerberos authenticates, so that scripts (commitinfo
scripts in particular) can correctly determine user identity.
Access rules: users listed in `writers' can both read and write;
users listed in `readers' can read; other users are denied access.
The error message denying access now states correctly whether you
are being denied "read" or "write" access depending on the command.
The server supports a `-k keytab' option to specify an alternate
Kerberos keytab file (our system one is readable by root only).
Since all email sent through `loginfo' now comes from the server
account, not from the user doing the commit, we added a line to the
log message stating which user is committing.
When we set up a server we: create a `cvs' account; create a Kerberos
principal for the service which we place in its own ~/keytab file;
initialize its repository (such as ~/repository); and arrange for
inetd to run:
cvs -k /.../keytab -f --allow-root=/.../repository pserver
as the user `cvs' when connections arrive.
Comments, contributions, and corrections welcome as always.
Thanks to Mark Visser <[EMAIL PROTECTED]> for helping us test.
diff -ur cvs-orig/src/logmsg.c cvs-1.11.2/src/logmsg.c
--- cvs-orig/src/logmsg.c 2001-09-14 13:12:10.000000000 -0400
+++ cvs-1.11.2/src/logmsg.c 2002-09-19 16:30:01.134153000 -0400
@@ -928,9 +928,10 @@
fprintf (pipefp, "%s\n\n", cp);
free (cp);
}
+ if (CVS_Username) (void) fprintf (pipefp, "By user %s\n", CVS_Username);
setup_tmpfile (pipefp, "", changes);
- (void) fprintf (pipefp, "Log Message:\n%s\n", message);
+ (void) fprintf (pipefp, "\nLog Message:\n\n%s\n", message);
if (logfp != (FILE *) 0)
{
(void) fprintf (pipefp, "Status:\n");
diff -ur 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 -ur 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-09-17 12:35:06.706721000 -0400
@@ -104,10 +104,9 @@
# include <shadow.h>
# endif
-/* The cvs username sent by the client, which might or might not be
- the same as the system username the server eventually switches to
- run as. CVS_Username gets set iff password authentication is
- successful. */
+/* The cvs username sent by the client, which may not be the same as
+ the username under which the server runs. CVS_Username gets set
+ iff authentication is successful. */
char *CVS_Username = NULL;
/* Used to check that same repos is transmitted in pserver auth and in
@@ -2429,176 +2428,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
+ * unset CVS_Username means that system 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 */
}
@@ -2660,8 +2592,12 @@
buf_output0 (buf_to_net, program_name);
buf_output0 (buf_to_net, " [server aborted]: \"");
buf_output0 (buf_to_net, cmd_name);
- buf_output0 (buf_to_net, "\" requires write access to the repository\n\
-error \n");
+ buf_output0 (buf_to_net, "\" requires ");
+ if (lookup_command_attribute (cmd_name) & CVS_CMD_MODIFIES_REPOSITORY)
+ buf_output0 (buf_to_net, "write");
+ else
+ buf_output0 (buf_to_net, "read");
+ buf_output0 (buf_to_net, " access to the repository\nerror \n");
goto free_args_and_return;
}
@@ -5279,6 +5215,31 @@
static void switch_to_user PROTO((const char *));
static void
+set_user_env (username)
+const char *username;
+{
+#if HAVE_PUTENV
+ /* Set LOGNAME, USER and CVS_USER in the environment, in case they
+ are already set to something else. */
+ char *env;
+
+ env = xmalloc (sizeof "LOGNAME=" + strlen (username));
+ (void) sprintf (env, "LOGNAME=%s", username);
+ (void) putenv (env);
+
+ env = xmalloc (sizeof "USER=" + strlen (username));
+ (void) sprintf (env, "USER=%s", username);
+ (void) putenv (env);
+
+#ifdef AUTH_SERVER_SUPPORT
+ env = xmalloc (sizeof "CVS_USER=" + strlen (CVS_Username));
+ (void) sprintf (env, "CVS_USER=%s", CVS_Username);
+ (void) putenv (env);
+#endif
+#endif /* HAVE_PUTENV */
+}
+
+static void
switch_to_user (username)
const char *username;
{
@@ -5366,27 +5327,7 @@
CVS_Username = xstrdup (username);
#endif
-#if HAVE_PUTENV
- /* Set LOGNAME, USER and CVS_USER in the environment, in case they
- are already set to something else. */
- {
- char *env;
-
- env = xmalloc (sizeof "LOGNAME=" + strlen (username));
- (void) sprintf (env, "LOGNAME=%s", username);
- (void) putenv (env);
-
- env = xmalloc (sizeof "USER=" + strlen (username));
- (void) sprintf (env, "USER=%s", username);
- (void) putenv (env);
-
-#ifdef AUTH_SERVER_SUPPORT
- env = xmalloc (sizeof "CVS_USER=" + strlen (CVS_Username));
- (void) sprintf (env, "CVS_USER=%s", CVS_Username);
- (void) putenv (env);
-#endif
- }
-#endif /* HAVE_PUTENV */
+ set_user_env(username);
}
#endif
@@ -6058,8 +5999,7 @@
if (gss_display_name (&stat_min, client_name, &desc,
&mechid) != GSS_S_COMPLETE
|| krb5_parse_name (kc, ((gss_buffer_t) &desc)->value, &p) != 0
- || krb5_aname_to_localname (kc, p, sizeof buf, buf) != 0
- || krb5_kuserok (kc, p, buf) != TRUE)
+ || krb5_aname_to_localname (kc, p, sizeof buf, buf) != 0)
{
error (1, 0, "access denied");
}
@@ -6079,7 +6019,9 @@
error (1, errno, "fwrite failed");
}
- switch_to_user (buf);
+ CVS_Username = xmalloc (strlen (buf) + 1);
+ strcpy (CVS_Username, buf);
+ set_user_env(buf); /* set LOGNAME, USER, CVS_USER */
printf ("I LOVE YOU\n");
fflush (stdout);
--
Brandon Craig Rhodes http://www.rhodesmill.org/brandon
Georgia Tech [EMAIL PROTECTED]
