Re: history and val-tags locks.

2005-05-18 Thread Mark D. Baushke
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Derek Price [EMAIL PROTECTED] writes:

 Derek Price wrote:
 
 Derek Price wrote:
 
 I see your point. What about `cvs server'? I
 can see both setups being useful... an admin
 who allowed users access to the CVS repository
 would probably prefer not to allow the config
 file to be specified whereas an admin who
 restriced the command that SSH users could run
 to a particular shell script that provided the
 -c option wouldn't mind... perhaps it should
 be a compile time option, with the default to
 disallow it.
 
 
 On further consideration, if we are going to
 consider a configurable config path with other
 CVS modes a security risk, then using it with
 pserver has to be considered a security risk
 too. There is nothing stopping a creative user
 with shell access to a machine from using
 pserver mode to access their repository.
 
 I might argue that any administrator worried
 that much about security should be disabling
 shell access to the machine anyhow, which would
 deal with any insecurity resulting from a
 configurable config path, but I don't feel so
 strongly about it that I wouldn't happily
 install it as a compile-time option that
 defaults to off.
 
 
 I've implemented this as an option to server 
 pserver. Installing as a global option would
 have create problems in multiroot mode anyhow.

True.

 Preliminary patch against 1.11.x attached. The
 final version will go into feature - I'm not
 advocating putting this in stable, but this is
 what I have now and I thought I would request a
 review. 

okay.

 This patch also finally disables the
 sourcing of the ~/.cvsrc file for the server
 commands as an added protection against a user
 setting the path to the config file.
 
 2005-05-17  Derek Price  [EMAIL PROTECTED]
 
 * configure.in: Add --enable-config-override.
 * main.c (main): Don't source .cvsrc in server mode.
 Remove obsolete comment.
 * parseinfo.c (ConfigPath): New global.
 (parse_config): Open ConfigPath when provided.
 * server.c (server): Parse -c option.
 * sanity.sh (server_usage): New static global.
 (sever): Add tests of ConfigPath and .cvsrc.
   
 
 I've been thinking about this more, and I'm
 starting to feel that as an option to
 server/pserver/etc, this really isn't so
 insecure. In general, an admin will be able to
 and probably does restrict the arguments to the
 server  pserver commands, and a user with shell
 access to the server could run a hacked CVS
 against a repo or even alter a repo directly
 anyhow, so the argument about security is mostly
 moot.

For pserver, the administrator has full control over
the command line.

For server, if the administrator is using a
restricted shell for users, they may or may not be
able to restrict command-line arguments (it
depends on how the restricted shell is
implemented).

 The only exception would be where the admin only
 used a setuid CVS executable to restrict repo
 access to a specific CVS executable. I'm not
 sure how common this is however, as it also
 disables the ability to use UNIX uids  gids for
 finer control over read  write access.

I have used (am using) a set-gid cvs and it does
not disable the use of UNIX uids at all. Yes, it
does inhibit gids as the entire repository uses
the same gid ('cvs'), but the cvs_acls deals with
permissions for commit and anyone with a login to
the special-purpose CVS server has already been
granted read permissions for checkout in any case.

I will grant you that this is not necessarily a
normal situation, but I make note of it as setuid
is not the only configuration possiblity here.

For the patch, if you want the
- --enable-config-override to be a configure and
compile-time option, I don't have any strong
objections to it as long as it defaults to
the more paranoid configuration. So, the current
enable_config_override=yes is not one that I
believe should be the default.

As an alternative, I would not have a problem with
allowing cvs to be built with a list of
directories that could be searched for a valid
config file. So, a --enable-config-prefix=/etc/cvs
might mean that /etc/cvs/$CVSROOT/config would looked
at before $CVSROOT/config ... doing that means that
the administrator would still have complete control
over the configurations and would not need to rely
on the user to make the right choice.

The downside is that src/sanity.sh tests for this
case would be more painful.

Your change to src/main.c looks fine.

While I would favor another mechanism for config
override, the code you have written for
src/parseinfo.c does appear to work as you suggest

As far as your src/sanity.sh tests, I believe you
should use the -c CONFIG-FILE to determine if cvs
has been configured with this option or not before
you fail the tests (granted STABLE does not have
that available to sanity.sh)

As far as src/server.c, the code looks reasonable
for what you are attempting.

-- Mark
-BEGIN PGP SIGNATURE-

Re: history and val-tags locks.

2005-05-18 Thread Derek Price
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Mark D. Baushke wrote:

 For pserver, the administrator has full control over
 the command line.

 For server, if the administrator is using a
 restricted shell for users, they may or may not be
 able to restrict command-line arguments (it
 depends on how the restricted shell is
 implemented).


For server, OpenSSH, at the very least, allows the command line to be
restricted.  Are we really worried about the security of the command
line if the user is still using a vulnerable tool like RSH to access
their server?

 I have used (am using) a set-gid cvs and it does
 not disable the use of UNIX uids at all. Yes, it
 does inhibit gids as the entire repository uses
 the same gid ('cvs'), but the cvs_acls deals with
 permissions for commit and anyone with a login to
 the special-purpose CVS server has already been
 granted read permissions for checkout in any case.

 I will grant you that this is not necessarily a
 normal situation, but I make note of it as setuid
 is not the only configuration possiblity here.


Well, no, but for a secure setup, aren't we still recommending that the
admin rely on a transport such as SSH and filesystem permissions or ACLs
for access restriction in the repository?  The stated position of the
CVS developers has been that users should rely on systems that get more
thorough security audits than CVS does to provide the secure portions of
their setups.  This means using SSH as a transport, restricting the
command line, and relying on system permissions/ACLs to restrict access
to the various portions of a repository.

setuid and setgid CVS executables both disable the fine-grained access
restrictions that would otherwise be possible, limiting you, basically,
to one group per repository/server.

 As an alternative, I would not have a problem with
 allowing cvs to be built with a list of
 directories that could be searched for a valid
 config file. So, a --enable-config-prefix=/etc/cvs
 might mean that /etc/cvs/$CVSROOT/config would looked
 at before $CVSROOT/config ... doing that means that
 the administrator would still have complete control
 over the configurations and would not need to rely
 on the user to make the right choice.


This compromise does sound reasonable.  Would you then consider a
default value of `--enable-config-prefix=/etc' as reasonable here?  i.e.
the ability to override the config would default to `on', with access to
config files restricted to /etc and subdirs.

 The downside is that src/sanity.sh tests for this
 case would be more painful.


Well, we could test that the restricted paths were disabled, at least.

 As far as your src/sanity.sh tests, I believe you
 should use the -c CONFIG-FILE to determine if cvs
 has been configured with this option or not before
 you fail the tests (granted STABLE does not have
 that available to sanity.sh)


If it defaults to on, with config-prefix used as you suggest, would you
still consider this last important?

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCi525LD1OTBfyMaQRAjERAKDuDciHLatseYmpN/PlQABI4Kcy7gCghAs8
KpNygtYAx/ZTx2+JWGjNaQI=
=G/xc
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-05-18 Thread Mark D. Baushke
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Derek Price [EMAIL PROTECTED] writes:

 Mark D. Baushke wrote:
 
  For pserver, the administrator has full control over
  the command line.
 
  For server, if the administrator is using a
  restricted shell for users, they may or may not be
  able to restrict command-line arguments (it
  depends on how the restricted shell is
  implemented).
 
 For server, OpenSSH, at the very least, allows
 the command line to be restricted. Are we really
 worried about the security of the command line
 if the user is still using a vulnerable tool
 like RSH to access their server?

Well, I would like to try not to impose
implementation decisions on administrators if
possible. Just because the
$HOME/.ssh/authorized_keys file is able to specify
a command= parameter for OpenSSH does not mean that
is the only version of secure transport that exists
(cf gssapi).

  I have used (am using) a set-gid cvs and it
  does not disable the use of UNIX uids at all.
  Yes, it does inhibit gids as the entire
  repository uses the same gid ('cvs'), but the
  cvs_acls deals with permissions for commit and
  anyone with a login to the special-purpose CVS
  server has already been granted read
  permissions for checkout in any case.
 
  I will grant you that this is not necessarily a
  normal situation, but I make note of it as setuid
  is not the only configuration possiblity here.
 
 
 Well, no, but for a secure setup, aren't we
 still recommending that the admin rely on a
 transport such as SSH and filesystem permissions
 or ACLs for access restriction in the
 repository?

Yes.

 The stated position of the CVS developers has
 been that users should rely on systems that get
 more thorough security audits than CVS does to
 provide the secure portions of their setups.

Yes.

 This means using SSH as a transport, restricting
 the command line, and relying on system
 permissions/ACLs to restrict access to the
 various portions of a repository.

Sure, but is would be better to not make for
fragile configurations that are accidentally
broken by administrators who do not understand
the subtle holes that might be available.

 setuid and setgid CVS executables both disable
 the fine-grained access restrictions that would
 otherwise be possible, limiting you, basically,
 to one group per repository/server.

Actually, one rational for going to a set-gid CVS
executable was that there were so many groups in
use on the system that it exceeded the Solaris
maximum number at the time. Letting everyone play
with a set-gid executable gave them an extra group
without burning thru the maximum number allowed by
the OS.

Fine grained access can go to far in some cases
and it is best to allow the administrators as much
flexibility as possible.

  As an alternative, I would not have a problem with
  allowing cvs to be built with a list of
  directories that could be searched for a valid
  config file. So, a --enable-config-prefix=/etc/cvs
  might mean that /etc/cvs/$CVSROOT/config would looked
  at before $CVSROOT/config ... doing that means that
  the administrator would still have complete control
  over the configurations and would not need to rely
  on the user to make the right choice.
 
 
 This compromise does sound reasonable. Would you
 then consider a default value of
 `--enable-config-prefix=/etc' as reasonable
 here? i.e. the ability to override the config
 would default to `on', with access to config
 files restricted to /etc and subdirs.

Sure. However, /etc is fairly crowded which is why
I suggested /etc/cvs as an alternative.

  The downside is that src/sanity.sh tests for this
  case would be more painful.
 
 Well, we could test that the restricted paths
 were disabled, at least.

Sure.

  As far as your src/sanity.sh tests, I believe you
  should use the -c CONFIG-FILE to determine if cvs
  has been configured with this option or not before
  you fail the tests (granted STABLE does not have
  that available to sanity.sh)
 
 
 If it defaults to on, with config-prefix used as
 you suggest, would you still consider this last
 important?

No. If we go with a config-prefix, then defaulting to ON
would be okay in my opinion.

Note: It may be desirable to consider
config-prefix to be a prefered list of directories
with the last directory searched being
$CVSROOT/CVSROOT ... So,
   --config-prefix='/etc/cvs:/usr/local/etc/cvs'
would mean that the config file would be the first
of 
   /etc/cvs/$CVSROOT/config
   /usr/local/etc/cvs/$CVSROOT/config
   $CVSROOT/CVSROOT/config

that existed.

-- Mark
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFCi6PI3x41pRYZE/gRAqcEAJ9iujX545bLiG28OHM+2JXob/nl0QCgyTLa
fNJYVUPgm/bZ2ofYQB2fD3E=
=gGYI
-END PGP SIGNATURE-


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-05-17 Thread Derek Price
Patch actually attached this time.

Cheers,

Derek


Derek Price wrote:

I've implemented this as an option to server  pserver.  Installing as a
global option would have create problems in multiroot mode anyhow.

Preliminary patch against 1.11.x attached.  The final version will go
into feature - I'm not advocating putting this in stable, but this is
what I have now and I thought I would request a review.  This patch also
finally disables the sourcing of the ~/.cvsrc file for the server
commands as an added protection against a user setting the path to the
config file.

2005-05-17  Derek Price  [EMAIL PROTECTED]

* configure.in: Add --enable-config-override.
* main.c (main): Don't source .cvsrc in server mode.  Remove
obsolete comment.
* parseinfo.c (ConfigPath): New global.
(parse_config): Open ConfigPath when provided.
* server.c (server): Parse -c option.
* sanity.sh (server_usage): New static global.
(sever): Add tests of ConfigPath and .cvsrc.
  

I've been thinking about this more, and I'm starting to feel that as an
option to server/pserver/etc, this really isn't so insecure.  In
general, an admin will be able to and probably does restrict the
arguments to the server  pserver commands, and a user with shell access
to the server could run a hacked CVS against a repo or even alter a repo
directly anyhow, so the argument about security is mostly moot.

The only exception would be where the admin only used a setuid CVS
executable to restrict repo access to a specific CVS executable.  I'm
not sure how common this is however, as it also disables the ability to
use UNIX uids  gids for finer control over read  write access.

Regards,

Derek
  

Index: configure.in
===
RCS file: /cvs/ccvs/configure.in,v
retrieving revision 1.176.2.60
diff -u -p -r1.176.2.60 configure.in
--- configure.in18 Apr 2005 17:46:13 -  1.176.2.60
+++ configure.in17 May 2005 16:06:53 -
@@ -965,9 +965,32 @@ dnl end --enable-rootcommit
 dnl
 
 
+dnl
+dnl begin --enable-config-override
+dnl
+
+AC_ARG_ENABLE(
+  [config-override],
+  AC_HELP_STRING(
+[--enable-config-override],
+[Cause the CVS server commands to allow the config file to be specified
+ on the command line.  (enabled by default)]), ,
+  [enable_config_override=yes])
+
+if test x$enable_config_override = xyes; then
+  AC_DEFINE(ALLOW_CONFIG_OVERRIDE, 1,
+[Define this to allow the path to CVS's config file to be set on the
+ command line.])
+fi
+
+dnl
+dnl end --enable-config-override
+dnl
+
+
 
 dnl
-dnl end --enable-*
+dnl end --enables
 dnl
 
 
Index: src/main.c
===
RCS file: /cvs/ccvs/src/main.c,v
retrieving revision 1.172.4.14
diff -u -p -r1.172.4.14 main.c
--- src/main.c  9 Mar 2005 19:47:15 -   1.172.4.14
+++ src/main.c  17 May 2005 16:06:53 -
@@ -478,6 +478,17 @@ main (argc, argv)
use_cvsrc = 0;
 }
 
+#ifdef SERVER_SUPPORT
+/* Don't try and read a .cvsrc file if we are a server.  */
+if (optind  argc
+(!strcmp (argv[optind], pserver)
+# ifdef HAVE_KERBEROS
+   || !strcmp (argv[optind], kserver)
+# endif /* HAVE_KERBEROS */
+   || !strcmp (argv[optind], server)))
+   use_cvsrc = 0;
+#endif /* SERVER_SUPPORT */
+
 /*
  * Scan cvsrc file for global options.
  */
@@ -693,10 +704,7 @@ distribution kit for a complete list of 
if (strcmp (cvs_cmd_name, pserver) == 0)
{
/* The reason that --allow-root is not a command option
-  is mainly the comment in server() about how argc,argv
-  might be from .cvsrc.  I'm not sure about that, and
-  I'm not sure it is only true of command options, but
-  it seems easier to make it a global option.  */
+  is mainly that it seems easier to make it a global option.  */
 
/* Gets username and password from client, authenticates, then
   switches to run as that user and sends an ACK back to the
Index: src/parseinfo.c
===
RCS file: /cvs/ccvs/src/parseinfo.c,v
retrieving revision 1.37.4.8
diff -u -p -r1.37.4.8 parseinfo.c
--- src/parseinfo.c 16 Mar 2005 22:00:44 -  1.37.4.8
+++ src/parseinfo.c 17 May 2005 16:06:53 -
@@ -17,6 +17,9 @@
 #include history.h
 
 extern char *logHistory;
+#ifdef ALLOW_CONFIG_OVERRIDE
+char *ConfigPath;
+#endif
 
 /*
  * Parse the INFOFILE file for the specified REPOSITORY.  Invoke CALLPROC for
@@ -252,22 +255,24 @@ parse_config (cvsroot)
return 0;
 parsed = 1;
 
-infopath = xmalloc (strlen (cvsroot)
-   + sizeof (CVSROOTADM_CONFIG)
-   + sizeof (CVSROOTADM)
-   + 10);
-if (infopath == NULL)
+#ifdef ALLOW_CONFIG_OVERRIDE
+if (ConfigPath)
+   infopath = ConfigPath;
+ 

Re: history and val-tags locks.

2005-05-17 Thread Derek Price
Derek Price wrote:

Derek Price wrote:

  

I see your point.  What about `cvs server'?  I can see both setups being
useful...  an admin who allowed users access to the CVS repository would
probably prefer not to allow the config file to be specified whereas an
admin who restriced the command that SSH users could run to a particular
shell script that provided the -c option wouldn't mind...  perhaps it
should be a compile time option, with the default to disallow it.




On further consideration, if we are going to consider a configurable
config path with other CVS modes a security risk, then using it with
pserver has to be considered a security risk too.  There is nothing
stopping a creative user with shell access to a machine from using
pserver mode to access their repository.

I might argue that any administrator worried that much about security
should be disabling shell access to the machine anyhow, which would deal
with any insecurity resulting from a configurable config path, but I
don't feel so strongly about it that I wouldn't happily install it as a
compile-time option that defaults to off.
  



I've implemented this as an option to server  pserver.  Installing as a
global option would have create problems in multiroot mode anyhow.

Preliminary patch against 1.11.x attached.  The final version will go
into feature - I'm not advocating putting this in stable, but this is
what I have now and I thought I would request a review.  This patch also
finally disables the sourcing of the ~/.cvsrc file for the server
commands as an added protection against a user setting the path to the
config file.

2005-05-17  Derek Price  [EMAIL PROTECTED]

* configure.in: Add --enable-config-override.
* main.c (main): Don't source .cvsrc in server mode.  Remove
obsolete comment.
* parseinfo.c (ConfigPath): New global.
(parse_config): Open ConfigPath when provided.
* server.c (server): Parse -c option.
* sanity.sh (server_usage): New static global.
(sever): Add tests of ConfigPath and .cvsrc.
  

I've been thinking about this more, and I'm starting to feel that as an
option to server/pserver/etc, this really isn't so insecure.  In
general, an admin will be able to and probably does restrict the
arguments to the server  pserver commands, and a user with shell access
to the server could run a hacked CVS against a repo or even alter a repo
directly anyhow, so the argument about security is mostly moot.

The only exception would be where the admin only used a setuid CVS
executable to restrict repo access to a specific CVS executable.  I'm
not sure how common this is however, as it also disables the ability to
use UNIX uids  gids for finer control over read  write access.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-05-06 Thread Derek Price
Mark D. Baushke wrote:

 I believe that most modern glob() implementations may indeed internally
 be implemented using fnmatch(), but not all of them are. If you are
 considering using an internal glob() instead of relying on a library
 version from the system, then I don't have as much of a problem with it.


Okay, it's still short documentation, but I've attached a patch against
stable which implements the HistoryLogPath and HistorySearchPath config
options.  I don't think I want to try and import glob() on stable, so
HistorySearchPath currently only handles fnmatch() metacharacters in the
final element of the path name.

Any consenses on whether importing this to stable is justified to deal
with the log rotation issue?

Cheers,

Derek


Index: src/client.c
===
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.318.4.27
diff -u -p -r1.318.4.27 client.c
--- src/client.c17 Mar 2005 15:39:09 -  1.318.4.27
+++ src/client.c6 May 2005 18:26:21 -
@@ -4903,7 +4903,7 @@ start_rsh_server (root, to_server, from_
 /* Send an argument STRING.  */
 void
 send_arg (string)
-char *string;
+const char *string;
 {
 char buf[1];
 char *p = string;
@@ -5953,8 +5953,8 @@ client_notify (repository, update_dir, f
  */
 void
 option_with_arg (option, arg)
-char *option;
-char *arg;
+const char *option;
+const char *arg;
 {
 if (arg == NULL)
return;
Index: src/client.h
===
RCS file: /cvs/ccvs/src/client.h,v
retrieving revision 1.39.6.2
diff -u -p -r1.39.6.2 client.h
--- src/client.h20 Mar 2004 18:14:56 -  1.39.6.2
+++ src/client.h6 May 2005 18:26:21 -
@@ -83,7 +83,7 @@ void read_from_server PROTO((char *buf, 
 
 /* Internal functions that handle client communication to server, etc.  */
 int supported_request PROTO ((char *));
-void option_with_arg PROTO((char *option, char *arg));
+void option_with_arg PROTO((const char *option, const char *arg));
 
 /* Get the responses and then close the connection.  */
 extern int get_responses_and_close PROTO((void));
@@ -119,7 +119,7 @@ send_files PROTO((int argc, char **argv,
 
 /* Send an argument to the remote server.  */
 void
-send_arg PROTO((char *string));
+send_arg PROTO((const char *string));
 
 /* Send a string of single-char options to the remote server, one by one.  */
 void
Index: src/cvs.h
===
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.235.4.31
diff -u -p -r1.235.4.31 cvs.h
--- src/cvs.h   2 May 2005 17:06:56 -   1.235.4.31
+++ src/cvs.h   6 May 2005 18:26:21 -
@@ -931,3 +931,6 @@ extern void cvs_outerr PROTO ((const cha
 extern void cvs_flusherr PROTO ((void));
 extern void cvs_flushout PROTO ((void));
 extern void cvs_output_tagged PROTO ((const char *, const char *));
+
+/* From find_names.c.  */
+List *find_files PROTO ((const char *dir, const char *pat));
Index: src/find_names.c
===
RCS file: /cvs/ccvs/src/find_names.c,v
retrieving revision 1.30.6.2
diff -u -p -r1.30.6.2 find_names.c
--- src/find_names.c31 Jan 2005 22:15:11 -  1.30.6.2
+++ src/find_names.c6 May 2005 18:26:21 -
@@ -22,10 +22,11 @@
  */
 
 #include cvs.h
+#include assert.h
 
 static int find_dirs PROTO((char *dir, List * list, int checkadm,
List *entries));
-static int find_rcs PROTO((char *dir, List * list));
+static int find_rcs PROTO((const char *dir, List * list));
 static int add_subdir_proc PROTO((Node *, void *));
 static int register_subdir_proc PROTO((Node *, void *));
 
@@ -249,55 +250,134 @@ Find_Directories (repository, which, ent
 return (dirlist);
 }
 
-/*
- * Finds all the ,v files in the argument directory, and adds them to the
- * files list.  Returns 0 for success and non-zero if the argument directory
- * cannot be opened, in which case errno is set to indicate the error.
- * In the error case LIST is left in some reasonable state (unchanged, or
- * containing the files which were found before the error occurred).
+
+
+/* Finds all the files matching PAT in the directory DIR, and adds them to a
+ * new List.  Returns the new List for success and NULL if the argument
+ * directory cannot be opened, in which case errno is set to indicate the
+ * error.
+ *
+ * NOTES
+ *   If GNULIB ever gets a glob module, this function could be replaced with
+ *   the glob function and a few tweaks to callers.
+ *
+ *   Currently, users may come to rely on the way history uses the unsorted
+ *   list this function returns since I believe that the default inode ordering
+ *   should be roughly the order of file creation.  It might be a good idea to
+ *   discourage this in favor of a specific sort order since we may otherwise
+ *   get complaints like, I copied my repository to 

Re: history and val-tags locks.

2005-05-05 Thread Derek Price
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Mark D. Baushke wrote:

 Note that HistoryFile has an argument that would basically be run
 through strftime, to enable log rotation. Also see the HistorySearch,
 which would be used as a file glob to locate history files to be read
 for executions of the `cvs history' command.


 Hmmm I do not like the BSD glob(3) function and its introduction in
 CVS would not make sense given we already have POSIX fnmatch()
 available... It would be better to specify looking for history files in
 terms of fnmatch() semantics if that is what you intend to use to do the
 resolution.


Hey Mark,

Revisiting this since you brought it up, what don't you like about BSD
glob(3)?  As near as I can tell, the POSIX.2 glob spec implies that it
should basically be a wrapper for fnmatch() that opens intervening
directories and performs intermediate matches.  I ask because I am
looking at reimplementing glob at the moment to allow the history search
path to include multiple directories.  For example, to match a path like:

HistoryLogPath=/cvsroot/CVSROOT/history/%Y/%m/%d
HistorySearchPath=/cvsroot/CVSROOT/history/*/*/*

I need to open /cvsroot/CVSROOT/history, open each directory that
matches *, open each directory in those directories that matches *, then
open each file in those directories that matches *.

IT seems to me that it would be much easier to import the glibc glob()
function in GNULIB style, probably by actually first importing it into a
GNULIB module, then use that if the local glob() functoin doesn't look
useful.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCek0qLD1OTBfyMaQRAqLcAKDJv4b9hnU8xk8I4OhXmJplvW/ieQCg69KW
EfAvZGNU13K1FtFM6n4kb0I=
=xyNy
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-05-05 Thread Mark D. Baushke
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Derek Price [EMAIL PROTECTED] writes:

 Mark D. Baushke wrote:
 
  Note that HistoryFile has an argument that would basically be run
  through strftime, to enable log rotation. Also see the HistorySearch,
  which would be used as a file glob to locate history files to be read
  for executions of the `cvs history' command.
 
 
  Hmmm I do not like the BSD glob(3) function and its introduction in
  CVS would not make sense given we already have POSIX fnmatch()
  available... It would be better to specify looking for history files in
  terms of fnmatch() semantics if that is what you intend to use to do the
  resolution.
 
 
 Hey Mark,
 
 Revisiting this since you brought it up, what don't you like about BSD
 glob(3)?

Not all systems provide a glob(3) function. Those that do provide a
glob(3) function provide many different variations of implementation
because glob existed a long time before the more general fnmatch was
written.

If you are only planning to use the subset of flags specified by POSIX.2
for glob, then you might be okay with a number of implementations.
However, my recollection is that there were a number of implementations
that did follow POSIX.2 closely.

I think something like these flags are 'standard':

  GLOB_APPEND, GLOB_DOOFS, GLOB_ERR, GLOB_MARK, GLOB_NOCHECK,
  GLOB_NOESCAPE, GLOB_NOSORT

while some implementations will also provide additional flags like
these that are found on OpenBSD and FreeBSD:

  GLOB_ALTDIRFUNC, GLOB_BRACE, GLOB_MAGCHAR, GLOB_NOMAGIC, GLOB_QUOTE,
  GLOB_TILDE, GLOB_LIMIT

while some GNU implementations add

  GLOB_ONLYDIR, GLOB_PERIOD

some glob() implementations provide those extensions, but have slightly
different semantics for them.

As for return values, I believe that GLOB_NOSPACE, GLOB_ABORTED and
GLOB_NOMATCH are standard, but some implementations may return
GLOB_NOSYS to indicate that the function is not supported.

In some implementations gl_pathc and gl_offs are not of type size_t as
mandated by POSIX.2, but are instead 'int' which was used in many *BSD
implementatiosn for years. This can cause interesting problems unless
care is taken with how you use glob().

 As near as I can tell, the POSIX.2 glob spec implies that it should
 basically be a wrapper for fnmatch() that opens intervening
 directories and performs intermediate matches. I ask because I am
 looking at reimplementing glob at the moment to allow the history
 search path to include multiple directories. For example, to match a
 path like:
 
 HistoryLogPath=/cvsroot/CVSROOT/history/%Y/%m/%d
 HistorySearchPath=/cvsroot/CVSROOT/history/*/*/*
 
 I need to open /cvsroot/CVSROOT/history, open each directory that
 matches *, open each directory in those directories that matches *, then
 open each file in those directories that matches *.

 IT seems to me that it would be much easier to import the glibc glob()
 function in GNULIB style, probably by actually first importing it into a
 GNULIB module, then use that if the local glob() functoin doesn't look
 useful.

I believe that most modern glob() implementations may indeed internally
be implemented using fnmatch(), but not all of them are. If you are
considering using an internal glob() instead of relying on a library
version from the system, then I don't have as much of a problem with it.

-- Mark
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFCemka3x41pRYZE/gRAnCpAKCcCec+kR6cfo0aGPmk3G6iXnRi7gCgmFLA
Fry9D/09sR2uD7pb+mOLlsc=
=wiw0
-END PGP SIGNATURE-


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-05-02 Thread Derek Price
Derek Price wrote:

HistoryFile=$CVSROOT/CVSROOT/logs/history/%Y%m%d



So, if the history file path were configurable and run through
strftime(), I can either (1) cache the history file name the first time
it is determined and gaurantee that all the operations from the same
server execution are logged to the same history file, or (2),
re-strftime() each time history_write() is called, and guarantee that
given two consecutive lines of any single history file, there was no CVS
operation that occurred in the span of time between the lines.

I'm leaning towards (1) since it is a little easier to code, only
requires one set of system calls per CVS server run, and CVS always
sorts history lines before displaying them anyhow.  Anyone have any
different opinions?

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


history and val-tags locks.

2005-04-27 Thread Derek Price
Hey all,

I'm getting ready to make two changes, possibly on stable.

The first would be to add file locking for the CVSROOT/history and
CVSROOT/val-tags files.  I have some reports of massively corrupted
history files in large repositories, and I don't see any other likely
cause.  Similarly, I expect locking will also be necessary on val-tags
in such a large repository and the additional code will be minimal after
I add the history locking stuff.

I was thinking that I would make totally separate lock dirs for this
(#cvs.history.lock and #cvs.val-tags.lock or somesuch) to allow
concurrent write access to other CVSROOT files, history, and val-tags.
I don't think deadlock will be an issue - despite the fact that any
given process writing to the history file or val-tags might hold any
number of other locks, the history or val-tags lock will always be
last in the chain and only held for the duration of the write of a
single line to the file.  Does anyone see any problems with that design?

Since file corruption is the current consequence of not having these
locks, I thought this would be justified on stable.  Any objections?


The other set of changes I was considering would be to enable a number
of new keywords for the config file to allow CVS to search for config
files in new locations.  I would not be changing the default locations,
but the new setup would enable something like the following to be
specified in CVSROOT/config:

  CheckoutListFile  $CVSROOT/CVSROOT/admin/checkoutlist
  CommitInfoFile$CVSROOT/CVSROOT/triggers/commitinfo
  CVSIgnoreFile $CVSROOT/CVSROOT/cvsignore
  CVSWrappersFile   $CVSROOT/CVSROOT/admin/cvswrappers
  EditInfoFile  $CVSROOT/CVSROOT/triggers/editinfo
  HistoryFile   $CVSROOT/CVSROOT/logs/history/%Y%m%d
  HistorySearch $CVSROOT/CVSROOT/logs/history/*
  LogInfoFile   $CVSROOT/CVSROOT/triggers/loginfo
  ModulesFile   $CVSROOT/CVSROOT/modules
  NotifyFile$CVSROOT/CVSROOT/triggers/notify
  PasswdFile$CVSROOT/CVSROOT/admin/passwd
  RCSInfoFile   $CVSROOT/CVSROOT/triggers/rcsinfo
  ReadersFile   $CVSROOT/CVSROOT/admin/readers
  TagInfoFile   $CVSROOT/CVSROOT/triggers/taginfo
  ValTagsFile   $CVSROOT/CVSROOT/admin/val-tags
  VerifyMsgFile $CVSROOT/CVSROOT/triggers/verifymsg
  WritersFile   $CVSROOT/CVSROOT/admin/writers

Note that HistoryFile has an argument that would basically be run
through strftime, to enable log rotation.  Also see the HistorySearch,
which would be used as a file glob to locate history files to be read
for executions of the `cvs history' command.

Regards,

Derek


signature.asc
Description: OpenPGP digital signature
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: history and val-tags locks.

2005-04-27 Thread Mark D. Baushke
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Derek Price [EMAIL PROTECTED] writes:

 I'm getting ready to make two changes, possibly on stable.
 
 The first would be to add file locking for the CVSROOT/history and
 CVSROOT/val-tags files.  I have some reports of massively corrupted
 history files in large repositories, and I don't see any other likely
 cause.  Similarly, I expect locking will also be necessary on val-tags
 in such a large repository and the additional code will be minimal after
 I add the history locking stuff.
 
 I was thinking that I would make totally separate lock dirs for this
 (#cvs.history.lock and #cvs.val-tags.lock or somesuch) to allow
 concurrent write access to other CVSROOT files, history, and val-tags.
 I don't think deadlock will be an issue - despite the fact that any
 given process writing to the history file or val-tags might hold any
 number of other locks, the history or val-tags lock will always be
 last in the chain and only held for the duration of the write of a
 single line to the file.  Does anyone see any problems with that design?
 
 Since file corruption is the current consequence of not having these
 locks, I thought this would be justified on stable.  Any objections?

I have no objections. (I have seen such corrptions and as a result have
reduced the amount of history being logged in favor of custom hacks
using syslog().)

 The other set of changes I was considering would be to enable a number
 of new keywords for the config file to allow CVS to search for config
 files in new locations.  I would not be changing the default locations,
 but the new setup would enable something like the following to be
 specified in CVSROOT/config:
 
   CheckoutListFile  $CVSROOT/CVSROOT/admin/checkoutlist
   CommitInfoFile$CVSROOT/CVSROOT/triggers/commitinfo
   CVSIgnoreFile $CVSROOT/CVSROOT/cvsignore
   CVSWrappersFile   $CVSROOT/CVSROOT/admin/cvswrappers
   EditInfoFile  $CVSROOT/CVSROOT/triggers/editinfo
   HistoryFile   $CVSROOT/CVSROOT/logs/history/%Y%m%d
   HistorySearch $CVSROOT/CVSROOT/logs/history/*
   LogInfoFile   $CVSROOT/CVSROOT/triggers/loginfo
   ModulesFile   $CVSROOT/CVSROOT/modules
   NotifyFile$CVSROOT/CVSROOT/triggers/notify
   PasswdFile$CVSROOT/CVSROOT/admin/passwd
   RCSInfoFile   $CVSROOT/CVSROOT/triggers/rcsinfo
   ReadersFile   $CVSROOT/CVSROOT/admin/readers
   TagInfoFile   $CVSROOT/CVSROOT/triggers/taginfo
   ValTagsFile   $CVSROOT/CVSROOT/admin/val-tags
   VerifyMsgFile $CVSROOT/CVSROOT/triggers/verifymsg
   WritersFile   $CVSROOT/CVSROOT/admin/writers

Please do not break the assumption of keyword=value
in the CVSROOT/config file. So, the above would need to
look closer to this:

CheckoutListFile=$CVSROOT/CVSROOT/admin/checkoutlist
CommitInfoFile=$CVSROOT/CVSROOT/triggers/commitinfo
CVSIgnoreFile=$CVSROOT/CVSROOT/cvsignore
CVSWrappersFile=$CVSROOT/CVSROOT/admin/cvswrappers
EditInfoFile=$CVSROOT/CVSROOT/triggers/editinfo
HistoryFile=$CVSROOT/CVSROOT/logs/history/%Y%m%d
HistorySearch=$CVSROOT/CVSROOT/logs/history/*
LogInfoFile=$CVSROOT/CVSROOT/triggers/loginfo
ModulesFile=$CVSROOT/CVSROOT/modules
NotifyFile=$CVSROOT/CVSROOT/triggers/notify
PasswdFile=$CVSROOT/CVSROOT/admin/passwd
RCSInfoFile=$CVSROOT/CVSROOT/triggers/rcsinfo
ReadersFile=$CVSROOT/CVSROOT/admin/readers
TagInfoFile=$CVSROOT/CVSROOT/triggers/taginfo
ValTagsFile=$CVSROOT/CVSROOT/admin/val-tags
VerifyMsgFile=$CVSROOT/CVSROOT/triggers/verifymsg
WritersFile=$CVSROOT/CVSROOT/admin/writers

Am I correct in assuming that this change also assumes handling
expansions of internal CVS variables like $CVSROOT is being added to to
CVSROOT/config processing and that those are NOT general purpose
environment variables being added?

 Note that HistoryFile has an argument that would basically be run
 through strftime, to enable log rotation.  Also see the HistorySearch,
 which would be used as a file glob to locate history files to be read
 for executions of the `cvs history' command.

Hmmm I do not like the BSD glob(3) function and its introduction in
CVS would not make sense given we already have POSIX fnmatch()
available... It would be better to specify looking for history files in
terms of fnmatch() semantics if that is what you intend to use to do the
resolution.

Thanks,
-- Mark
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFCb+/Q3x41pRYZE/gRArfHAJ9atipXM5i0NRCQDrvyEXUlFv0o0ACgkoQ5
wJrHwWL2uAcNG/pwJqgmHc8=
=wyUH
-END PGP SIGNATURE-


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-04-27 Thread Derek Price
Mark D. Baushke wrote:

 Derek Price [EMAIL PROTECTED] writes:

 I'm getting ready to make two changes, possibly on stable.

 The first would be to add file locking for the CVSROOT/history and
 CVSROOT/val-tags files.  I have some reports of massively corrupted
 history files in large repositories, and I don't see any other likely
 cause.  Similarly, I expect locking will also be necessary on val-tags
 in such a large repository and the additional code will be minimal after
 I add the history locking stuff.

 I was thinking that I would make totally separate lock dirs for this
 (#cvs.history.lock and #cvs.val-tags.lock or somesuch) to allow
 concurrent write access to other CVSROOT files, history, and val-tags.
 I don't think deadlock will be an issue - despite the fact that any
 given process writing to the history file or val-tags might hold any
 number of other locks, the history or val-tags lock will always be
 last in the chain and only held for the duration of the write of a
 single line to the file.  Does anyone see any problems with that design?

 Since file corruption is the current consequence of not having these
 locks, I thought this would be justified on stable.  Any objections?


 I have no objections. (I have seen such corrptions and as a result have
 reduced the amount of history being logged in favor of custom hacks
 using syslog().)


Hrm.  That's interesting in some respects...  syslog would already
handle a bunch of the functionality I've been considering adding, but I
think it would be harder to hook into the `cvs history' command.  :(

 The other set of changes I was considering would be to enable a number
 of new keywords for the config file to allow CVS to search for config
 files in new locations.  I would not be changing the default locations,
 but the new setup would enable something like the following to be
 specified in CVSROOT/config:

   CheckoutListFile  $CVSROOT/CVSROOT/admin/checkoutlist
   CommitInfoFile$CVSROOT/CVSROOT/triggers/commitinfo
   CVSIgnoreFile $CVSROOT/CVSROOT/cvsignore
   CVSWrappersFile   $CVSROOT/CVSROOT/admin/cvswrappers
   EditInfoFile  $CVSROOT/CVSROOT/triggers/editinfo
   HistoryFile   $CVSROOT/CVSROOT/logs/history/%Y%m%d
   HistorySearch $CVSROOT/CVSROOT/logs/history/*
   LogInfoFile   $CVSROOT/CVSROOT/triggers/loginfo
   ModulesFile   $CVSROOT/CVSROOT/modules
   NotifyFile$CVSROOT/CVSROOT/triggers/notify
   PasswdFile$CVSROOT/CVSROOT/admin/passwd
   RCSInfoFile   $CVSROOT/CVSROOT/triggers/rcsinfo
   ReadersFile   $CVSROOT/CVSROOT/admin/readers
   TagInfoFile   $CVSROOT/CVSROOT/triggers/taginfo
   ValTagsFile   $CVSROOT/CVSROOT/admin/val-tags
   VerifyMsgFile $CVSROOT/CVSROOT/triggers/verifymsg
   WritersFile   $CVSROOT/CVSROOT/admin/writers


 Please do not break the assumption of keyword=value
 in the CVSROOT/config file. So, the above would need to
 look closer to this:


I won't.  My apologies for cutting and pasting the list above out of
context from an email I received.

 CheckoutListFile=$CVSROOT/CVSROOT/admin/checkoutlist
 CommitInfoFile=$CVSROOT/CVSROOT/triggers/commitinfo
 CVSIgnoreFile=$CVSROOT/CVSROOT/cvsignore
 CVSWrappersFile=$CVSROOT/CVSROOT/admin/cvswrappers
 EditInfoFile=$CVSROOT/CVSROOT/triggers/editinfo
 HistoryFile=$CVSROOT/CVSROOT/logs/history/%Y%m%d
 HistorySearch=$CVSROOT/CVSROOT/logs/history/*
 LogInfoFile=$CVSROOT/CVSROOT/triggers/loginfo
 ModulesFile=$CVSROOT/CVSROOT/modules
 NotifyFile=$CVSROOT/CVSROOT/triggers/notify
 PasswdFile=$CVSROOT/CVSROOT/admin/passwd
 RCSInfoFile=$CVSROOT/CVSROOT/triggers/rcsinfo
 ReadersFile=$CVSROOT/CVSROOT/admin/readers
 TagInfoFile=$CVSROOT/CVSROOT/triggers/taginfo
 ValTagsFile=$CVSROOT/CVSROOT/admin/val-tags
 VerifyMsgFile=$CVSROOT/CVSROOT/triggers/verifymsg
 WritersFile=$CVSROOT/CVSROOT/admin/writers

 Am I correct in assuming that this change also assumes handling
 expansions of internal CVS variables like $CVSROOT is being added to to
 CVSROOT/config processing and that those are NOT general purpose
 environment variables being added?


Yes.  I'll probably hook into the same function used to do this by the
trigger scripts, expand_path().  This should enable the same config file
to be used in multiple CVS roots as well.  An associated change I was
putting off talking about was adding a global `-c config_file' option
to cause CVS to look elsewhere for its configuration file.

 Note that HistoryFile has an argument that would basically be run
 through strftime, to enable log rotation.  Also see the HistorySearch,
 which would be used as a file glob to locate history files to be read
 for executions of the `cvs history' command.


 Hmmm I do not like the BSD glob(3) function and its introduction in
 CVS would not make sense given we already have POSIX fnmatch()
 available... It would be better to specify looking for history files in
 terms of fnmatch() semantics if that is what you intend to use to do the
 resolution

Re: history and val-tags locks.

2005-04-27 Thread Mark D. Baushke
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Derek Price [EMAIL PROTECTED] writes:

 Mark D. Baushke wrote:
 
  Derek Price [EMAIL PROTECTED] writes:
 
 I won't.  My apologies for cutting and pasting the list above out of
 context from an email I received.

Good, sorry for sounding alarmist. :-)

  Am I correct in assuming that this change also assumes handling
  expansions of internal CVS variables like $CVSROOT is being added to to
  CVSROOT/config processing and that those are NOT general purpose
  environment variables being added?
 
 Yes.  I'll probably hook into the same function used to do this by the
 trigger scripts, expand_path().  This should enable the same config file
 to be used in multiple CVS roots as well.

This sounds reasonable to me.

 An associated change I was putting off talking about was adding a
 global `-c config_file' option to cause CVS to look elsewhere for
 its configuration file.

I worry about the security implications of this one. I don't believe it
can be allowed for anythiner other than :pserver: mode where the
administrator takes care of arguments to the cvs executable directly.

If the user may provide a config file that specifies the commitinfo
triggers to use, it could subvert the intention of the repository
administrator. The administrator could get the same effect by putting a
symbolic link into CVSROOT for the config file... of course, it would be
well to ensure that rebuilding the repository database would not destroy
that link.

 That is what I meant.  I had thought that a file glob was not precise
 and referred to a whole class of path expansion, using patterns like
 name.??.* , and implemented by various functions including fnmatch.  I
 use the term in the same way I might specify regex matching when I
 don't see a need to be more precise (e.g. basic regex, extended
 regex, perl regex, etc.).  I completely intended and continue to
 intend to use the POSIX fnmatch() we've already imported from GNULIB to
 implement this matching and any similar matching in the future.

Good. (I was just trying to be very clear and not misrepresent your
intention.)

-- Mark
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFCcAWH3x41pRYZE/gRAtlWAKC2HoNtPj7aY8w/BHIisfEqU6lE3QCg2OU2
OwbgnsvZJaAt+rudYSHcxPc=
=lVJk
-END PGP SIGNATURE-


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-04-27 Thread Derek Price
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Mark D. Baushke wrote:

 An associated change I was putting off talking about was adding a
 global `-c config_file' option to cause CVS to look elsewhere for
 its configuration file.

 I worry about the security implications of this one. I don't believe it
 can be allowed for anythiner other than :pserver: mode where the
 administrator takes care of arguments to the cvs executable directly.

 If the user may provide a config file that specifies the commitinfo
 triggers to use, it could subvert the intention of the repository
 administrator. The administrator could get the same effect by putting a
 symbolic link into CVSROOT for the config file... of course, it would be
 well to ensure that rebuilding the repository database would not destroy
 that link.


I see your point.  What about `cvs server'?  I can see both setups being
useful...  an admin who allowed users access to the CVS repository would
probably prefer not to allow the config file to be specified whereas an
admin who restriced the command that SSH users could run to a particular
shell script that provided the -c option wouldn't mind...  perhaps it
should be a compile time option, with the default to disallow it.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCcAsgLD1OTBfyMaQRAo/CAKDwtOAvLA4p70qfyzIJ4WtwmUGX7ACfX3Ep
bTrGiunRznuULICLrBmxykQ=
=7I8L
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs


Re: history and val-tags locks.

2005-04-27 Thread Derek Price
Derek Price wrote:

 I see your point.  What about `cvs server'?  I can see both setups being
 useful...  an admin who allowed users access to the CVS repository would
 probably prefer not to allow the config file to be specified whereas an
 admin who restriced the command that SSH users could run to a particular
 shell script that provided the -c option wouldn't mind...  perhaps it
 should be a compile time option, with the default to disallow it.


On further consideration, if we are going to consider a configurable
config path with other CVS modes a security risk, then using it with
pserver has to be considered a security risk too.  There is nothing
stopping a creative user with shell access to a machine from using
pserver mode to access their repository.

I might argue that any administrator worried that much about security
should be disabling shell access to the machine anyhow, which would deal
with any insecurity resulting from a configurable config path, but I
don't feel so strongly about it that I wouldn't happily install it as a
compile-time option that defaults to off.

Regards,

Derek




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs