bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 2 security concerns: remote init, and disabling CVSROOT/passwd


From: Derek Price
Subject: Re: 2 security concerns: remote init, and disabling CVSROOT/passwd
Date: Mon, 07 May 2007 11:03:47 -0400
User-agent: Thunderbird 1.5.0.10 (Windows/20070221)

Excuse the cross-post. Development discussions are more appropriately sent to bug-cvs. Please delete address@hidden from any replies.


Sylvain Beucler wrote:
[summary of remote `cvs init' exploit]

Currently the command is disabled for remote access, using a
quick'n'dirty patch ("if (server_active) exit(EXIT_FAILURE)").

What would you recommend? Are there legitimate use for remote 'init'?
I wouldn't like users creating their private repositories at Savannah
either.

No, there really aren't any legitimate uses for `cvs init' via remote access. Anyone who is creating a new CVS repository or upgrading a CVS server to use a new CVS executable presumably has local access to the machine anyhow.

I've recommended something like your `hack' to customers in the past, but I've never actually installed the change into CVS itself until a few minutes ago (in stable - the merge to 1.12.x is still running through the regression suite). It should be incorporated into the 1.11.23 & 1.12.14 releases.


Since we have numerous repositories, we hit command line limits for
pserver --allow-root (2700 * 2 * 20 / 1024 = 105KB, not counting
aliases). Besides, it is not really easy to change the 'pserver'
command line in xinetd each time a new project is created.

To overcome this, we used Vincent Caron's patch
(https://mail.gna.org/public/savane-dev/2005-08/msg00042.html).

For starters, I've heard of an --allow-root-file patch which allows all the roots to be specified in a file with only the file name being specified on the command line. The only reference I found to it in a quick google search was in a savane-dev archive, however, and there were some broken links so I couldn't figure out how to get the attachment:

https://mail.gna.org/public/savane-dev/2002-04/msg00373.html

[elide well known pserver abuses]

Currently there's a hard-coded patch at Savannah which prevent parsing
CVSROOT/passwd for pserver; the root-owned pserver is also ran behind
the firewall, as it's only used internaly, and only for web
repositories (the public pserver is ran as user nobody). Of course
that's a pretty brute way to handle the situation.

[snip]

- Permit the CVS administrator to disable CVSROOT/passwd
  authentication with a pserver command-line switch (a cracker might
  still switch to an unsecure PAM scheme, but that's less
  straightforward).

- Or more generaly, specify the allowed authentication scheme in the
  pserver command line (this would be easier to secure) - overriding
  CVSROOT/config.

Could you send me the patch you mention? I should be able to adapt it to a command-line switch pretty easily.

[1]
http://web.archive.org/web/20040327105943/http://ccvs.cvshome.org/issues/show_bug.cgi?id=41
Unfortunately the old cvshome issue tracker appears to be down, and I
can't grab the patch anymore. Does somebody have it?

I pulled out and attached both patches from that issue from a backup of the old Issuezilla.

I don't know if you still want the --allow-root-regexp patch merged into 1.12.x, but I found some discussion in the archives and it sounds like we were waiting on documentation and test cases for the change.

Regards,

Derek
--
Derek R. Price
CVS Solutions Architect
Get CVS support at Ximbiot <http://ximbiot.com>!
v: +1 248.835.1260
f: +1 248.835.1263
<mailto:address@hidden>
Index: NEWS
===================================================================
RCS file: /cvs/ccvs/NEWS,v
retrieving revision 1.97
diff -c -r1.97 NEWS
*** NEWS        13 Sep 2001 19:08:15 -0000      1.97
--- NEWS        15 Oct 2001 20:28:13 -0000
***************
*** 8,13 ****
--- 8,17 ----
  should only really affect developers.  See the section of the INSTALL file
  about using the autotools if you are compiling CVS yourself.
  
+ * A new command line option, --allow-root-regexp, was added.  It
+ allows to specify a list of regular expressions for the repositories
+ locations, in addition to the list of exact paths.
+ 
  Changes from 1.11.1 to 1.11.1p1:
  
  * Read only access was broken - now fixed.
Index: config.h.in
===================================================================
RCS file: /cvs/ccvs/config.h.in,v
retrieving revision 1.55
diff -c -r1.55 config.h.in
*** config.h.in 17 Jul 2001 09:17:31 -0000      1.55
--- config.h.in 15 Oct 2001 20:28:13 -0000
***************
*** 1,4 ****
! /* config.h.in.  Generated automatically from configure.in by autoheader.  */
  
  /* Define if on AIX 3.
     System headers sometimes define this.
--- 1,4 ----
! /* config.h.in.  Generated automatically from configure.in by autoheader 
2.13.  */
  
  /* Define if on AIX 3.
     System headers sometimes define this.
Index: doc/cvs.texinfo
===================================================================
RCS file: /cvs/ccvs/doc/cvs.texinfo,v
retrieving revision 1.533
diff -c -r1.533 cvs.texinfo
*** doc/cvs.texinfo     4 Oct 2001 14:45:16 -0000       1.533
--- doc/cvs.texinfo     15 Oct 2001 20:28:43 -0000
***************
*** 2317,2322 ****
--- 2317,2326 ----
  different @sc{cvsroot} directory will not be allowed to
  connect.  If there is more than one @sc{cvsroot}
  directory which you want to allow, repeat the option.
+ If there is a whole class of @sc{cvsroot}
+ directories which you want to allow, you can also specify a
+ regular expression with the @samp{--allow-root-regexp}
+ option.  This option is repeatable, too.
  (Unfortunately, many versions of @code{inetd} have very small
  limits on the number of arguments and/or the total length
  of the command.  The usual solution to this problem is
***************
*** 7775,7780 ****
--- 7779,7788 ----
  Specify legal @sc{cvsroot} directory.  See
  @ref{Password authentication server}.
  
+ @item address@hidden
+ Specify legal @sc{cvsroot} directories.  See
+ @ref{Password authentication server}.
+ 
  @cindex Authentication, stream
  @cindex Stream authentication
  @item -a
***************
*** 10434,10439 ****
--- 10442,10453 ----
  in @sc{cvs} 1.9 and older).  See @ref{Password
  authentication server}.
  
+ @item address@hidden
+ Specify a regular expression to be matched by legal
+ @sc{cvsroot} directories (server only) (not in @sc{cvs}
+ 1.11.1 and older).  See @ref{Password authentication
+ server}.
+ 
  @item -a
  Authenticate all communication (client only) (not in @sc{cvs}
  1.9 and older).  See @ref{Global options}.
***************
*** 13306,13312 ****
  pserver server which chooses not to provide a
  specific reason for denying authorization.  Check that
  the username and password specified are correct and
! that the @code{CVSROOT} specified is allowed by @samp{--allow-root}
  in @file{inetd.conf}.  See @ref{Password authenticated}.
  
  @item cvs @var{command}: conflict: removed @var{file} was modified by second 
party
--- 13320,13327 ----
  pserver server which chooses not to provide a
  specific reason for denying authorization.  Check that
  the username and password specified are correct and
! that the @code{CVSROOT} specified is allowed by
! @samp{--allow-root} or @samp{--allow-root-regexp}
  in @file{inetd.conf}.  See @ref{Password authenticated}.
  
  @item cvs @var{command}: conflict: removed @var{file} was modified by second 
party
Index: doc/stamp-vti
===================================================================
RCS file: /cvs/ccvs/doc/stamp-vti,v
retrieving revision 1.5
diff -c -r1.5 stamp-vti
*** doc/stamp-vti       6 Sep 2001 23:12:27 -0000       1.5
--- doc/stamp-vti       15 Oct 2001 20:28:43 -0000
***************
*** 1,4 ****
! @set UPDATED 6 September 2001
! @set UPDATED-MONTH September 2001
  @set EDITION 1.11.1.1
  @set VERSION 1.11.1.1
--- 1,4 ----
! @set UPDATED 14 October 2001
! @set UPDATED-MONTH October 2001
  @set EDITION 1.11.1.1
  @set VERSION 1.11.1.1
Index: doc/version.texi
===================================================================
RCS file: /cvs/ccvs/doc/version.texi,v
retrieving revision 1.6
diff -c -r1.6 version.texi
*** doc/version.texi    6 Sep 2001 23:12:27 -0000       1.6
--- doc/version.texi    15 Oct 2001 20:28:43 -0000
***************
*** 1,4 ****
! @set UPDATED 6 September 2001
! @set UPDATED-MONTH September 2001
  @set EDITION 1.11.1.1
  @set VERSION 1.11.1.1
--- 1,4 ----
! @set UPDATED 14 October 2001
! @set UPDATED-MONTH October 2001
  @set EDITION 1.11.1.1
  @set VERSION 1.11.1.1
Index: src/ChangeLog
===================================================================
RCS file: /cvs/ccvs/src/ChangeLog,v
retrieving revision 1.2215
diff -c -r1.2215 ChangeLog
*** src/ChangeLog       2 Oct 2001 18:07:05 -0000       1.2215
--- src/ChangeLog       15 Oct 2001 20:28:56 -0000
***************
*** 1,3 ****
--- 1,17 ----
+ 2001-10-14  Roland Mas  <address@hidden>
+ 
+       * root.c: Added new functions root_allow_regexp_add and
+       root_allow_regewp_ok, new variables root_allow_regexp_count,
+       root_allow_regexp_vector and root_allow_regexp_size.
+ 
+       * server.c (pserver_authenticate_connection): Use new
+       root_allow_regexp function.
+ 
+       * main.c (main): Use new root_allow_regexp_add function, declare
+       new --allow-root-regexp option parameter.
+ 
+       * NEWS: Documented these changes.
+ 
  2001-10-02  Larry Jones  <address@hidden>
  
        * rcs.c (RCS_fully_parse): Add revision number to more error messages.
Index: src/cvs.h
===================================================================
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.225
diff -c -r1.225 cvs.h
*** src/cvs.h   24 Aug 2001 17:47:02 -0000      1.225
--- src/cvs.h   15 Oct 2001 20:28:58 -0000
***************
*** 454,461 ****
--- 454,463 ----
  cvsroot_t *local_cvsroot PROTO((char *dir));
  void Create_Root PROTO((char *dir, char *rootdir));
  void root_allow_add PROTO ((char *));
+ void root_allow_regexp_add PROTO ((char *));
  void root_allow_free PROTO ((void));
  int root_allow_ok PROTO ((char *));
+ int root_allow_regexp_ok PROTO ((char *));
  
  char *gca PROTO((const char *rev1, const char *rev2));
  extern void check_numeric PROTO ((const char *, int, char **));
Index: src/main.c
===================================================================
RCS file: /cvs/ccvs/src/main.c,v
retrieving revision 1.168
diff -c -r1.168 main.c
*** src/main.c  4 Sep 2001 22:43:23 -0000       1.168
--- src/main.c  15 Oct 2001 20:29:01 -0000
***************
*** 414,419 ****
--- 414,420 ----
        {"help-synonyms", 0, NULL, 2},
        {"help-options", 0, NULL, 4},
        {"allow-root", required_argument, NULL, 3},
+       {"allow-root-regexp", required_argument, NULL, 5},
          {0, 0, 0, 0}
      };
      /* `getopt_long' stores the option index here, but right now we
***************
*** 517,522 ****
--- 518,527 ----
            case 3:
                /* --allow-root */
                root_allow_add (optarg);
+               break;
+           case 5:
+               /* --allow-root-regexp */
+               root_allow_regexp_add (optarg);
                break;
            case 'Q':
                really_quiet = 1;
Index: src/root.c
===================================================================
RCS file: /cvs/ccvs/src/root.c,v
retrieving revision 1.55
diff -c -r1.55 root.c
*** src/root.c  5 Jul 2001 19:11:39 -0000       1.55
--- src/root.c  15 Oct 2001 20:29:02 -0000
***************
*** 178,183 ****
--- 178,187 ----
  static char **root_allow_vector;
  static int root_allow_size;
  
+ static int root_allow_regexp_count;
+ static char **root_allow_regexp_vector;
+ static int root_allow_regexp_size;
+ 
  void
  root_allow_add (arg)
      char *arg;
***************
*** 230,240 ****
--- 234,299 ----
  }
  
  void
+ root_allow_regexp_add (arg)
+     char *arg;
+ {
+     char *p;
+ 
+     if (root_allow_regexp_size <= root_allow_regexp_count)
+     {
+       if (root_allow_regexp_size == 0)
+       {
+           root_allow_regexp_size = 1;
+           root_allow_regexp_vector =
+               (char **) malloc (root_allow_regexp_size * sizeof (char *));
+       }
+       else
+       {
+           root_allow_regexp_size *= 2;
+           root_allow_regexp_vector =
+               (char **) realloc (root_allow_regexp_vector,
+                                  root_allow_regexp_size * sizeof (char *));
+       }
+ 
+       if (root_allow_regexp_vector == NULL)
+       {
+       no_memory:
+           /* Strictly speaking, we're not supposed to output anything
+              now.  But we're about to exit(), give it a try.  */
+           printf ("E Fatal server error, aborting.\n\
+ error ENOMEM Virtual memory exhausted.\n");
+ 
+           /* I'm doing this manually rather than via error_exit ()
+              because I'm not sure whether we want to call server_cleanup.
+              Needs more investigation....  */
+ 
+ #ifdef SYSTEM_CLEANUP
+           /* Hook for OS-specific behavior, for example socket
+              subsystems on NT and OS2 or dealing with windows
+              and arguments on Mac.  */
+           SYSTEM_CLEANUP ();
+ #endif
+ 
+           exit (EXIT_FAILURE);
+       }
+     }
+     p = malloc (strlen (arg) + 1);
+     if (p == NULL)
+       goto no_memory;
+     strcpy (p, arg);
+     root_allow_regexp_vector[root_allow_regexp_count++] = p;
+ }
+ 
+ void
  root_allow_free ()
  {
      if (root_allow_vector != NULL)
        free_names (&root_allow_count, root_allow_vector);
      root_allow_size = 0;
+ 
+     if (root_allow_regexp_vector != NULL)
+       free_names (&root_allow_regexp_count, root_allow_regexp_vector);
+     root_allow_regexp_size = 0;
  }
  
  int
***************
*** 243,249 ****
  {
      int i;
  
!     if (root_allow_count == 0)
      {
        /* Probably someone upgraded from CVS before 1.9.10 to 1.9.10
           or later without reading the documentation about
--- 302,308 ----
  {
      int i;
  
!     if (root_allow_count == 0 && root_allow_regexp_count == 0)
      {
        /* Probably someone upgraded from CVS before 1.9.10 to 1.9.10
           or later without reading the documentation about
***************
*** 260,271 ****
      }
  
      for (i = 0; i < root_allow_count; ++i)
!       if (strcmp (root_allow_vector[i], arg) == 0)
!           return 1;
      return 0;
  }
  
  
  
  /* This global variable holds the global -d option.  It is NULL if -d
     was not used, which means that we must get the CVSroot information
--- 319,348 ----
      }
  
      for (i = 0; i < root_allow_count; ++i)
!       if (strcmp (root_allow_vector[i], arg) == 0)
!           return 1;
      return 0;
  }
  
+ int
+ root_allow_regexp_ok (arg)
+     char *arg;
+ {
+     int i, status;
+     regex_t   re;
  
+     for (i = 0; i < root_allow_regexp_count; ++i) {
+        if (regcomp(&re, root_allow_regexp_vector[i],
+                  REG_EXTENDED|REG_NOSUB) != 0) {
+           return 0;     /* report error */
+        }
+        status = regexec(&re, arg, (size_t) 0, NULL, 0);
+        regfree(&re);
+        if (status == 0)
+         return 1;
+     }
+     return 0;
+ }
  
  /* This global variable holds the global -d option.  It is NULL if -d
     was not used, which means that we must get the CVSroot information
Index: src/server.c
===================================================================
RCS file: /cvs/ccvs/src/server.c,v
retrieving revision 1.267
diff -c -r1.267 server.c
*** src/server.c        4 Sep 2001 18:07:18 -0000       1.267
--- src/server.c        15 Oct 2001 20:29:12 -0000
***************
*** 5776,5782 ****
      {
        error (1, 0, "bad auth protocol end: %s", tmp);
      }
!     if (!root_allow_ok (repository))
      {
        printf ("error 0 %s: no such repository\n", repository);
  #ifdef HAVE_SYSLOG_H
--- 5776,5782 ----
      {
        error (1, 0, "bad auth protocol end: %s", tmp);
      }
!     if (!root_allow_ok (repository) && !root_allow_regexp_ok (repository))
      {
        printf ("error 0 %s: no such repository\n", repository);
  #ifdef HAVE_SYSLOG_H
--- ./root.c    2002-12-06 14:10:28.000000000 -0500
+++ ../../../cvs-1.11.5/src/root.c      2003-02-21 21:48:43.000000000 -0500
@@ -249,9 +249,23 @@
        error_exit ();
     }
 
-    for (i = 0; i < root_allow_count; ++i)
-       if (strcmp (root_allow_vector[i], arg) == 0)
-           return 1;
+    for (i = 0; i < root_allow_count; ++i) {
+       /* If the --allow-root entry ends in "**" (e.g., /var/cvs/**)
+        * then allow any repository that is a suffix of that path. */
+       int allow_suffix_paths = 0; /* assume NO */
+        int root_allow_path_length = strlen(root_allow_vector[i]);
+        if (root_allow_path_length > 2) /* ensure a check for "**" is even 
possible */
+           allow_suffix_paths = 
(root_allow_vector[i][root_allow_path_length-2] == '*') &&
+                                
(root_allow_vector[i][root_allow_path_length-1] == '*');
+       if (allow_suffix_paths) {
+           /* allow any repository that is a suffix of the path */
+           if (strncmp (root_allow_vector[i], arg, root_allow_path_length-2) 
== 0)
+               return 1;
+       } else {
+           if (strcmp (root_allow_vector[i], arg) == 0)
+               return 1;
+       }
+    }
     return 0;
 }
 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]