Glenn Morris wrote: > Recently Savannah bzr was updated to bzr 2.6. > This breaks the bzr-hookless-email script that was being used for > commit notifications. [1] > > To try and fix this, I suggest installing the bzr-email plugin. > `apt-get install bzr-email' (the version from Debian testing is the > latest version).
Thank you for researching this problem and proposing a solution. That's awesome! > IIUC, historically Savannah had some theoretical security concerns > about bzr-email. I've looked at it and IMO the one issue that there > might be can be trivially patched away. [2] > But I don't really know why you went with bzr-hookless-email. This was before my time and I have no information on it. > If you are willing to install it, I can test it. I am willing to give it a try. Here are my comments. There will be enough delay that if there are other comments I will hear them before I get this done. But otherwise I will implement your plan as filed. > [1] If you want the details, > https://lists.ubuntu.com/archives/bazaar/2013q2/075520.html With the exception of your comment there that the 2.6 wasn't in the then stable Squeeze 6. It is in the now stable Wheezy 7. But vcs.sv is still at this instant running Squeeze 6 and would normally have had bzr 2.1.2. The bzr that was installed on vcs.sv was not from the Debian packages but from local compilation. > [2] The only issue I can see is this: > Anyone with write access to a bzr branch can set plugin options in > .bzr/branch/branch.conf (this is actually good, because it means we > will be able to control our own commit notifications without needing > to bug Savannah admins). If those are simply options I don't see the issue. But with security concerns it was probably possible to set configurations that executed commands. I assume. I haven't looked. > One option is "post_commit_mailer". > This is either 'smtplib' (an internal Python library), or an external > command like "/bin/mail". > (I am assuming smtplib will be the correct option for Savannah.) I don't think it matters. My personal preference is to use the local command. I would use /usr/sbin/sendmail as a general local mailer because that takes no configuration. But /bin/mail or /usr/bin/mailx would be okay too. In that role they are effectively simply a wrapper around /usr/sbin/sendmail. Editorial comment: I think the general preference that we often see in documentation for smtp is that people developing on an MS or Apple client don't usually have an mta running. Using smtp they can point off to their mail relay and it works for them. Using /usr/sbin/sendmail only works on GNU/BSD/Unix machines. But we have one of those. :-) > Someone could try and set this to something nasty like "rm -rf /". > So all we need to do is hard-code that option to "smtplib". > Patching emailer.py is one trivial way to do that (see end). That would be unpleasant. Talking out the problem.... It would run on the server with the user:group of the person who is running the commit, who is probably not the person who set up the attack. The permissions would be similar though. It would take quite a long time to traverse the entire filesystem slowing things down for a long time. It would eventually remove the project files that are accessible by that user:group. It wouldn't have permission to remove other projects files or anything else on the server. Worse would be if a single user were in several groups because then it would have permission to remove all of the files in all of the groups of that user. > Alternatively, I am told that options set in ~/.bzr/locations.conf > will take precedence over branch options. If bzr on Savannah runs > under a single user, that could be a better way to do it. AFAIK write access is through ssh+bzr invoking sv_membersh which is a restricted shell which screens incoming actions. Each process is run as a specific uid:gid and access is allowed or denied by group control. There isn't a single process owner. It is compartmentalized into individually owned processes. > If you want to review it, the code is at > https://launchpad.net/bzr-email If a python person would look that over it would be great. I am not a python person so must rely upon it being "obvious". > I suggest a patch something like: > > *************** > *** 206,212 **** > if mailer == 'smtplib': > self._send_using_smtplib() > else: > ! self._send_using_process() > finally: > self.repository.unlock() > self.branch.unlock() > --- 206,213 ---- > if mailer == 'smtplib': > self._send_using_smtplib() > else: > ! raise errors.BzrError("Bad value for post_commit_mailer") > ! # self._send_using_process() > finally: > self.repository.unlock() > self.branch.unlock() Seems okay. Throw an error if not one of the predefined possibilites. > *************** > *** 303,309 **** > opt_post_commit_to = ListOption('post_commit_to', > help='Address to send commit emails to.') > opt_post_commit_mailer = Option('post_commit_mailer', > ! help='Mail client to use.', default='mail') > opt_post_commit_url = Option('post_commit_url', > help='URL to mention for branch in post commit messages.') > opt_revision_mail_headers = ListOption('revision_mail_headers', > --- 304,310 ---- > opt_post_commit_to = ListOption('post_commit_to', > help='Address to send commit emails to.') > opt_post_commit_mailer = Option('post_commit_mailer', > ! help='Mail client to use.', default='smtplib') > opt_post_commit_url = Option('post_commit_url', > help='URL to mention for branch in post commit messages.') > opt_revision_mail_headers = ListOption('revision_mail_headers', Change the default to smtplib. > > (I am assuming smtplib will be the correct option for Savannah.) > > PS if not, patch would look like this. Replace /usr/bin/mail with > whatever the correct value is for Savannah. > > if mailer == 'smtplib': > self._send_using_smtplib() > ! elif mailer == '/usr/bin/mail': > self._send_using_process() > + else: > + raise errors.BzrError("Bad value for mailer option") I think this allows /usr/bin/mail as one of the allowable options. Which is fine. Require that mailer be one or the other and throw an exception otherwise. Looks okay to me. Because mailx is POSIX I think that should be /usr/bin/mailx and not /usr/bin/mail. But practically they are the same since the bsd-mail package is providing both. vcs:~# update-alternatives --display mailx mailx - auto mode link currently points to /usr/bin/bsd-mailx /usr/bin/bsd-mailx - priority 50 slave Mail.1.gz: /usr/share/man/man1/bsd-mailx.1.gz slave mail.1.gz: /usr/share/man/man1/bsd-mailx.1.gz slave mailx.1.gz: /usr/share/man/man1/bsd-mailx.1.gz slave mail: /usr/bin/bsd-mailx slave Mail: /usr/bin/bsd-mailx Current 'best' version is '/usr/bin/bsd-mailx'. As a practical matter on other systems /usr/bin/mail is usually the old /bin/mail program and doesn't support the -s subject option. But /usr/bin/mailx is the BSD traditional /bin/Mail program and does support the -s subject option. Of course GNU is not Unix and so this difference is really just one of those portability issues that never fully got converged everywhere regardless of POSIX standardizing on the SysV flavor of it. In SysV /bin/Mail with a capital would be out of place so they went with the /usr/bin/mailx name for it. Bob
