[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] cvs security versus Checkin.prog and Update.prog
From: |
Mike Sutton |
Subject: |
Re: [PATCH] cvs security versus Checkin.prog and Update.prog |
Date: |
Thu, 27 Mar 2003 08:19:19 -0500 |
User-agent: |
Mutt/1.4i |
See like a reasonable approach to me.
On 03/26/03 19:14:18, Mark D. Baushke wrote:
> Hi Folks,
>
> I was just revisiting the thread about the CVS/Checkin.prog and
> CVS/Update.prog for security. The two relevant threads seem to be:
>
> http://www.mail-archive.com/bug-cvs@gnu.org/msg00384.html
> and
> http://mail.gnu.org/archive/html/bug-cvs/2003-03/msg00107.html
>
> I have not really finished writing updates for the documentation of this
> proposed patch yet, but I thought I would float the idea to see what
> folks think of it.
>
> This patch the choice to be up to a given repository manager with the
> default being to be more secure.
>
> Comments?
>
> Thanks,
>
> Index: doc/cvs.texinfo
> ===================================================================
> RCS file: /cvs/ccvs/doc/cvs.texinfo,v
> retrieving revision 1.564
> diff -u -p -r1.564 cvs.texinfo
> +++ doc/cvs.texinfo 27 Mar 2003 04:12:20 -0000
> @@ -13570,6 +13570,20 @@ As an example, to restrict users not in
> group to using @code{cvs admin} to change the default keyword
> substitution mode, lock revisions, unlock revisions, and
> replace the log message, use @samp{UserAdminOptions=klum}.
> +
> +@cindex ServerProgs, in CVSROOT/config
> +@item ServerProgs=@var{value}
> +Control if the server will be allowed to run
> +@code{CVS/Checkin.prog} or @code{CVS/Update.prog} for
> +the user.
> +
> +When @samp{ServerProgs=allowed} or
> +@samp{ServerProgs=yes}, then the server will allow the
> +client request to run the scripts for the user.
> +
> +When @samp{ServerProgs=forbidden} (the default) or
> +@samp{ServerProgs=no}, then the server will try to be
> +more secure and not agree to the scripts for the user.
> @end table
>
> @c ---------------------------------------------------------------------
> Index: src/cvs.h
> ===================================================================
> RCS file: /cvs/ccvs/src/cvs.h,v
> retrieving revision 1.248
> diff -u -p -r1.248 cvs.h
> +++ src/cvs.h 27 Mar 2003 04:12:20 -0000
> @@ -588,6 +588,16 @@ extern char *lock_dir;
>
> /* AllowedAdminOptions setting from CVSROOT/config. */
> extern char *UserAdminOptions;
> +
> +/* AllowServerProgs setting from CVSROOT/config. */
> +/*
> + * Is the client allowed to ask the server to execute either
> + * Update-prog or Checkin-prog?
> + */
> +#define SERVERPROG_FORBIDDEN 0
> +#define SERVERPROG_PERMITTED 1
> +#define SERVERPROG_DEFAULT SERVERPROG_FORBIDDEN
> +extern int ServerProgs;
> Index: src/mkmodules.c
> ===================================================================
> RCS file: /cvs/ccvs/src/mkmodules.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 mkmodules.c
> +++ src/mkmodules.c 27 Mar 2003 04:12:20 -0000
> @@ -317,6 +317,11 @@ static const char *const config_contents
> "# The following string would enable all `cvs admin' commands for all\n",
> "# users:\n",
> "#UserAdminOptions=aAbceIklLmnNostuU\n",
> + "#\n",
> + "# Set `ServerProgs' to `forbidden' (the default) to disable the
> CVS/Checkin.prog\n",
> + "# and CVS/Update.prog processing. Set it to `allowed' (the previous
> CVS behavior)\n",
> + "# to allow them to be executed. Warning: This option has security
> implications\n",
> + "# as it may allow for arbitrary commands to be run on the server by a
> committer.\n",
> NULL
> };
>
> Index: src/parseinfo.c
> ===================================================================
> RCS file: /cvs/ccvs/src/parseinfo.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 parseinfo.c
> +++ src/parseinfo.c 27 Mar 2003 04:12:20 -0000
> @@ -397,6 +397,17 @@ warning: this CVS does not support Prese
> UserAdminOptions = xmalloc(strlen(p) + 1);
> strcpy(UserAdminOptions, p);
> }
> + else if (strcmp (line, "ServerProgs") == 0)
> + {
> + if (strcmp (p, "no") == 0 ||
> + strcmp (p, "forbidden") == 0 ||
> + strcmp (p, "never") == 0)
> + ServerProgs = SERVERPROG_FORBIDDEN;
> + else if (strcmp (p, "yes") == 0 ||
> + strcmp (p, "permitted") == 0 ||
> + strcmp (p, "allowed") == 0)
> + ServerProgs = SERVERPROG_PERMITTED;
> + }
> else
> {
> /* We may be dealing with a keyword which was added in a
> Index: src/sanity.sh
> ===================================================================
> RCS file: /cvs/ccvs/src/sanity.sh,v
> retrieving revision 1.784
> diff -u -p -r1.784 sanity.sh
> +++ src/sanity.sh 27 Mar 2003 04:12:20 -0000
> @@ -10272,6 +10272,21 @@ args: realmodule"
> dotest modules5-9 "test -d realmodule && test -f realmodule/a" ""
> dotest_fail modules5-10 "test -f realmodule/b" ""
> if $remote; then
> + dotest_fail modules5-serverprogs-1 "${testcvs} -q co realmodule" \
> +"Flag -i in modules administratively disabled"
> + cd ..
> + dotest modules5-serverprogs-2 "${testcvs} -Q co CVSROOT" ''
> + cd CVSROOT
> + echo ServerProgs=allowed >> config
> + dotest modules5-serverprogs-3 "${testcvs} -Q ci -mallow config" \
> +"Checking in config;
> +${CVSROOT_DIRNAME}/CVSROOT/config,v <-- config
> +new revision: 1\.2; previous revision: 1\.1
> +done
> +${PROG} [a-z]*: Rebuilding administrative file database"
> + cd ..
> + rm -r CVSROOT
> + cd 1
> dotest modules5-11 "${testcvs} -q co realmodule" \
> "checkout script invoked in ${TMPDIR}/cvs-serv[0-9a-z]*
> args: realmodule"
> Index: src/server.c
> ===================================================================
> RCS file: /cvs/ccvs/src/server.c,v
> retrieving revision 1.290
> diff -u -p -r1.290 server.c
> +++ src/server.c 27 Mar 2003 04:12:20 -0000
> @@ -4669,11 +4669,21 @@ warning: this client does not support -i
> buf_send_counted (protocol);
> }
>
> +int ServerProgs = SERVERPROG_FORBIDDEN;
> static void
> serve_checkin_prog (arg)
> char *arg;
> {
> FILE *f;
> +
> + if (ServerProgs == SERVERPROG_FORBIDDEN)
> + {
> + if (alloc_pending (80))
> + sprintf (pending_error_text, "\
> +E Flag -i in modules administratively disabled");
> + return;
> + }
> +
> f = CVS_FOPEN (CVSADM_CIPROG, "w+");
> if (f == NULL)
> {
> @@ -4717,6 +4727,14 @@ serve_update_prog (arg)
> if (alloc_pending (80))
> sprintf (pending_error_text, "\
> E Flag -u in modules not allowed in readonly mode");
> + return;
> + }
> +
> + if (ServerProgs == SERVERPROG_FORBIDDEN)
> + {
> + if (alloc_pending (80))
> + sprintf (pending_error_text, "\
> +E Flag -u in modules administratively disabled");
> return;
> }
>
>
>
> _______________________________________________
> Bug-cvs mailing list
> Bug-cvs@gnu.org
> http://mail.gnu.org/mailman/listinfo/bug-cvs
--
Mike Sutton
SAIC
Division 397
(937) 431-2273 FAX ext. 2297
suttonm@saic.com