[I know of no problem with the accuracy of this documentation change. However, I think this needs work as a way to document it. First of all, it offers the user too many options (root vs non-root), without much guidance/advice about choosing one or the other. Second of all, much of the existing text regarding the security implications of pserver is incorrect if run as a non-root user (for example, the consequences of a writable CVSROOT/passwd become much less severe, as it would merely allows one to access CVS as a different user, not become any user on the system). Third of all, it makes a long and confusing section longer and more confusing. I think that "pserver server as root" and "pserver server as non-root" should probably be separate sections. This is just a nit, but I don't like the text about how --allow-root has nothing to do with the root user. I realize that this is a potential source of confusion, but I guess my opinion is that the extra text is as likely to increase confusion as it is to resolve it (just by virtue of being extra; think of the reader in a hurry for example). Another issue, only partly a documentation issue, is whether one should be required to specify the correct user in CVSROOT/passwd if CVS is run as non-root (see comment in server.c (switch_to_user)). For example "joe::jsmith". Seems to me the answer should be yes; if the administrator set up CVSROOT/passwd for CVS to run as "jsmith" and it is only possible to run as "jjones", shouldn't CVS diagnose that? (As of the CVS development version of 1999-05-11, you do need to set up CVSROOT/passwd that way). Running as non-root won't work on some 4.4BSD-derived systems, which spit a warning to stderr when a non-root user calls setuid() (a rather bogus behavior IMHO but there you have it). The "=" syntax suggested below would be a way to avoid this problem. Or maybe it should be more like "joe::=" or whatever syntax would be best to say "don't switch users". That also might be a win in the face of portability problems with setuid() and friends. See also "id -un" in sanity.sh ("pserver" test). -kingdon] Date: Mon, 24 Nov 1997 19:44:38 -0600 From: Karl Fogel To: kingdon@cyclic.com Subject: CVS pserver as non-root seems to be already working The return value of switch_to_user() is never checked; inside switch_to_user(), the return values of setgid() and setuid() are also ignored. So CVS tries to switch to the user, fails, and simply continues running as whoever it was. I tested this a few ways and it always worked as expected (if the server ran as a user who had group permission to modify the repository, I could check out, update and commit changed files; if it ran as a user with no permission to change the repository, CVS couldn't create a lock and so wouldn't even check stuff out). Below is a documentation patch. (Btw, no, it didn't take me from the meeting until now to write this -- I went to a gym with Ben to work out too :-) ). *** doc/ChangeLog.orig Mon Nov 24 19:38:20 1997 --- doc/ChangeLog Mon Nov 24 19:38:40 1997 *************** *** 1,3 **** --- 1,11 ---- + 1997-11-24 Karl Fogel + + * cvs.texinfo (Password authentication server): document the fact + that CVS pserver can start as non-root. Also note that the "root" + in "--allow-root" has nothing to do with Unix's superuser, to + avoid confusion. This might be obvious, but then again we are + using "root" to mean two different things in adjacent paragraphs. + Sat Nov 22 12:29:58 1997 Jim Kingdon * cvsclient.texi (Entries Lines): Clarify options in entries line. *** doc/cvs.texinfo.orig Mon Nov 24 19:36:30 1997 --- doc/cvs.texinfo Mon Nov 24 19:37:37 1997 *************** *** 2366,2375 **** @samp{-T} option to specify a temporary directory. The @samp{--allow-root} option specifies the allowable ! @sc{cvsroot} directory. Clients which attempt to use a ! different @sc{cvsroot} directory will not be allowed to ! connect. If there is more than one @sc{cvsroot} directory which you want to allow, repeat the option. If your @code{inetd} wants a symbolic service name instead of a raw port number, then put this in --- 2366,2400 ---- @samp{-T} option to specify a temporary directory. The @samp{--allow-root} option specifies the allowable ! @sc{cvsroot} directory (in this option, ``root'' means ! the top of a directory tree and is unrelated to the ! Unix ``root'' superuser). Clients which attempt to use ! a different @sc{cvsroot} directory will not be allowed ! to connect. If there is more than one @sc{cvsroot} directory which you want to allow, repeat the option. + + In the above example, the @sc{cvs} server starts + running with the ``root'' user's identity, but before + accessing the repository it would switch to run as + whatever user the client specified during + authentication. @sc{cvs} could also start up as a regular + user: + + @example + 2401 stream tcp nowait jsmith /usr/local/bin/cvs + cvs --allow-root=/usr/cvsroot pserver + @end example + + However, if @sc{cvs} starts up as someone other than + the superuser, it won't then switch to run as someone + else---it will just continue as the original user. + Thus, above, the server would start up as @samp{jsmith} + and continue running as @samp{jsmith} no matter what + username the client specifies. Whether or not running + the @sc{cvs} server as a regular user would work well + at your site depends, among other things, on how the + repository file permissions are arranged with respect + to that user (@xref{File permissions}). If your @code{inetd} wants a symbolic service name instead of a raw port number, then put this in