Re: [PATCH] cvs security versus Checkin.prog and Update.prog

[Mike Sutton]

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