-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ralf S. Engelschall <[EMAIL PROTECTED]> writes:
> On Sat, Oct 04, 2003, Mark D. Baushke wrote: > > > Ralf S. Engelschall <[EMAIL PROTECTED]> writes: > > > > > The patch adds two still missing and very important auditing hook > > > facilities to CVS: "CVSROOT/importinfo" for auditing "cvs import" > > > operations and "CVSROOT/admininfo" for auditing "cvs admin" operations. > > > I was prompted to implement these some years ago when I wrote OSSP > > > Shiela (see http://www.ossp.org/pkg/tool/shiela/ for details) and had > > > to recognize that it still could be circumvented by the two remaining > > > repository-destructive operations "cvs import" and "cvs admin". > > > > I believe it is still possible for a 'cvs import' to destroy important > > cvs branch and version tags. If the intent is to help lock down the > > 'cvs import' command, then more work is probably needed... > > Can you give more details, please? Why can "cvs import" still > destroy any tags if an "importinfo" filter would stop processing? My > "importinfo" hook is very early in the "cvs import" processing and AFAIK > there is no repository write access before it. I was under the impression that the importinfo_runproc was running the filter and passing the importinfo_vtag, the repository and the files being imported. Is the vendor branch getting to the script thru some other means? I suppose this is not a big deal as it should only really matter for tags that are for real RCS branches as opposed to CVS magic branches, but I wasn't 100% sure about failure modes... Another fairly minor matter is that the -k options is not known at import time and might be fairly critical to folks that might want the -kb default for 'binary' files to be checked... I find it less important to worry about the -k option on a cvs commit than on an import. However, I suppose that both of them really should be treated the same, so I am just noting in passing that a server on a Windows box may be at more of a disadvantage in understanding how the imported files are to be opened for verification than their non-windows counterparts. > > [...] > > I will note that the man pages are not the primary reference > > documentation for cvs, so it would be desirable for you to extend the > > docuemntation in the doc/cvs.texinfo file as well. > > I've looked at the doc/cvs.texinfo file and have not found any chapter > which really deals with the xxxxinfo hooks. There are just a few > references and small examples at various places for some hooks, but > no general documentation on them. So I decided to document them in > cvs(5), because there at least mostly all xxxxinfo hooks _are_ already > documented. I understand the documentation is light, but just as there is an '@node commitinfo' in doc/cvs.textinfo, there should be one for each of the two new files you are adding. > > It may not be argued that it is more complex to use multiple scripts and > > possibly accidentally introduce different standards for entry into the > > source base. I have a slight bias toward moving to a new importinfo > > script to augment what already exists rather than overloading the > > existing commitinfo with the added state caused by knowing that this is > > an import over multiple directories and files with two new tags being > > added to the repository... so, I have a slight bias in favor of your new > > feature in principle over reworking the commitinfo script. > > Especially, if you change the existing commitinfo script you too > easily could break lots of existing tools. Yes, I find this to be a good argument. It remains to be seen if the consensus of the others matches it. > > One thing that does concern me is that the tags for a cvsimport do > > not appear to get passed to either importinfo nor taginfo for checking. > > If the importinfo feature is to really check what the user is doing, is > > it not possible that the user is about to destroy a very important > > release tag that already exists as a side-effect of their import? > > Why are the tags not passed? At least for my "importinfo", the > vendor-tag is passed as an argument. Without this my OSSP Shiela > facility would not be able to control imports to different branches. Hmmm... I didn't notice this and it did not appear in the documentation for how the file is used... > > With regard to the admininfo trigger, I presume that users will either > > need to be a part of the CVS_ADMIN_GROUP or there will need to be no > > CVS_ADMIN_GROUP at all in order for the admininfo script to be run. In > > addition, only those options permitted by UserAdminOptions will get > > thru. Is that correct? If so, it probably deserves a mention in the > > doc/cvs.texinfo file. > > Correct. My current "admininfo" is processed _after_ the CVS_ADMIN_GROUP > stuff is processed. Good. I thought this was the case, but it should be documented for folks in the man page and cvs.texinfo node on the file. > > Also, there are many discrete operations possible with 'cvs admin' > > and they are very different. It is not clear to me how the script > > would know that a user might be deleting a revision in the file > > versus changing from -kkv to -kk keyword expansion mode or trying > > to unlock a revision that had somehow been locked. > > That's a good point. I think we should pass the arguments > to "cvs admin" through to the script. I'll see... Excellent. That would make a lot of sense to me. > > For example, might it be desirable to ensure that a replacement > > log message (-mrev:msg) conformed to the same standards as one > > that was given during a 'cvs commit' operation? > > Hmmm... my personal opinion here is that "cvs admin" is a low-level > operation and hence (once passed the "admininfo" filters) should be > able to do whatever is wished and does not have to conform to the same > standards as higher-level operations. But that's just my point of > view... Sure, it is one point of view. Right now the administrator is able to allow various of the switches to be used by anyone regardless of membership in the CVS_ADMIN_GROUP. However, if the administrator wanted to make some of the more powerful features available, they still might want to verify basic uses of the features. > > It might be that the cvs administrator would need/want the list > > of switches being passed to the admininfo script. A good design > > now might really give a cvs administrator more flexibility in > > what they allow their users to do for themselves. > > Yes, we should pass the options on the "cvs admin" command line to the > filter script. Agreed. Thanks! -- Mark -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQE/f0eR3x41pRYZE/gRAikfAJ9q1tMpYjuI08KRBTW5Sb3qQ0pcygCeIX/A AcJ1R/0hfuILqktScfLoN/4= =ehTR -----END PGP SIGNATURE----- _______________________________________________ Info-cvs mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/info-cvs
