Re: history and val-tags locks.
-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/mai
Re: history and val-tags locks.
-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.
-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
Re: history and val-tags locks.
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.
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_OVE
Re: history and val-tags locks.
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 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 a
Re: history and val-tags locks.
-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.
-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.
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
Re: history and val-tags locks.
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
Re: history and val-tags locks.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: > >An associated change I was putting off talking about was adding a > >global `-c ' 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.
-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 ' 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.
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 = > 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 ' 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
Re: history and val-tags locks.
-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 = 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
history and val-tags locks.
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