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 src/server.c, the code looks reasonable
for what you are attempting.
-- Mark
-BEGIN PGP SIGNATURE-
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:
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.
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.
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.
-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.
-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.
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.
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.
-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.
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.
-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.
-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.
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
