-----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----- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCii4p3x41pRYZE/gRAt0tAJ9zzEWfVpWUuRA6qn6WKkoqzOXTkwCffpoI q5BKSwEnOKwbXHN2SPtjblc= =Bmf5 -----END PGP SIGNATURE----- _______________________________________________ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs