[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Savannah-hackers-public] Re: 2 security concerns: remote init, and disa
From: |
Derek Price |
Subject: |
[Savannah-hackers-public] 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;
}