Smart server jail breaks bzr+http with shared repos

Bug #348308 reported by Matt Nordhoff on 2009-03-25
52
This bug affects 7 people
Affects Status Importance Assigned to Milestone
Bazaar
High
Andrew Bennetts

Bug Description

The hpss jail improvements in revision 4194 of bzr.dev broke bzr+http operations against branches in shared repos:

$ bzr branch http://bzr.mattnordhoff.com/bzr/pytz/2009d/
bzr: ERROR: Server sent an unexpected error: ('error', "jail break: 'chroot-139142444:///pytz/'")

Standalone branches still work fine:

$ bzr branch http://bzr.mattnordhoff.com/bzr/pytz/pytz-2008f.btrees/
Branched 161 revision(s).

The server's log also gets spammed with tracebacks, e.g. "BzrError: jail break: 'chroot-139142444:///pytz/'".

Related branches

Matt Nordhoff (mnordhoff) wrote :

Note: I just reverted the copy of bzr used by my bzr+http server to
r4193, so http://bzr.mattnordhoff.com/ no longer exhibits this bug.

Lionel Dricot (ploum) wrote :

I confirm that upgrading to bzr 1.14 makes it impossible to use an existing bzr+http server. So it's kind of a "big" bug.

Workaround : downgrading to 1.13

John A Meinel (jameinel) wrote :

It seems like the chroot information should be based on the configuration, and not the first URL passed.
I don't have specific information about how it is working, but that might be an avenue to look at for debugging this.

Changed in bzr:
importance: Undecided → High
status: New → Triaged
Robert Collins (lifeless) wrote :

chroot information is dynamic based on the url and the root of the server;

it may be the wsgi glue.

John A Meinel (jameinel) wrote :

If I was guessing, I would say the chroot is being set based on the '.bzr/smart' you are posting to. So if your layout is:

repo/branch

Then your initial POST goes to 'http://..../repo/branch/.bzr/smart'

At that point, we can see that we don't have a repository (or maybe we see the repository is 1 level up from a request, etc).

Anyway, we've done things 2 different ways in the past. One is to post:

1) POST http://../repo/branch/.bzr/smart open_repository ..
2) POST http://../repo/.bzr/smart open_repository .

The main difference is whether the "Transport.clone('..')" was changing the base URL that we POST to, or whether it was just something changing the paths transmitted.

My guess is the currentcode is doing (1), and that the chroot is being based on the POST url, which means that the jail is causing us to fail. So at this point we could:

1) Change the client to go back to the old way, and have 'clone(..)' change the POST url. I think Andrew didn't like that form. I *think* because there was no guarantee that both URLs were valid to POST to, and we know we have something valid where we are at now.
2) Change the jail code to only jail at the level of config, rather than making it dynamic based on the POST location. I personally would be fine with this, considering the WSGI codes says "expose paths under /srv/bzr/repo" it seems fine to use that as the chroot jail for all requests.

John A Meinel wrote:
[...]
> 2) Change the jail code to only jail at the level of config, rather than
> making it dynamic based on the POST location. I personally would be fine with
> this, considering the WSGI codes says "expose paths under /srv/bzr/repo" it
> seems fine to use that as the chroot jail for all requests.

I think this option makes the most sense. It's consistent with how the TCP and
inet (SSH) servers work.

Matt Nordhoff (mnordhoff) wrote :

I just added an evil little monkeypatch to my bzr-smart script that changes the jail to your configured root (e.g. /srv/bzr/repo):

from bzrlib.smart import request

def setup_jail(self):
    # The backing transport is a chroot on /srv/bzr/repo
    transport = self._backing_transport.clone('/')
    request.jail_info.transports = [transport, self._backing_transport]

request.SmartServerRequest.setup_jail = setup_jail

Monkeypatching is wrong, and might break, of course, but at least it works right now.

Andrew Bennetts (spiv) wrote :

I have a fix for this, finally. See the linked branch. Will add test(s) and submit for review, hopefully will be part of 2.1.0.

With this plus a small bug fix for loggerhead, loggerhead's serve-branches script Just Works for me for serving a shared repo over bzr+http.

Changed in bzr:
assignee: nobody → Andrew Bennetts (spiv)
status: Triaged → Fix Committed
Andrew Bennetts (spiv) on 2009-10-26
Changed in bzr:
status: Fix Committed → Fix Released
John A Meinel (jameinel) on 2009-11-02
Changed in bzr:
milestone: none → 2.1.0b2
Matt Nordhoff (mnordhoff) wrote :

For posterity, I'm attaching a better (mostly just longer) version of the monkeypatch.

It verifies that the backing transport really is a chroot, so it should be a bit safer.

Credit goes to spiv, BTW. (Not blame, though!)

Vincent Ladeuil (vila) wrote :

@Matt, please do a merge proposal instead of adding a patch to this bug, it's far easier to track, thanks in advance !

Vincent Ladeuil wrote:
> @Matt, please do a merge proposal instead of adding a patch to this bug,
> it's far easier to track, thanks in advance !

This bug has long been fixed. The monkeypatch is just a workaround
applications can use if they need to run with an older (< 2.1) bzr.

Vincent Ladeuil (vila) wrote :

@Matt, ha ok, thanks for the explanation :D

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers