Code review comment for lp:~jml/bzr/allow-writes-change-84659

Revision history for this message
Andrew Bennetts (spiv) wrote :

Martin Pool wrote:
[...]
> Murphy's law (correctly stated) implies and experience confirms that
> if it is possible to offer anonymous write access to the whole
> internet, people will do it. I feel an obligation to at least reduce
> the occurrence of problems arising from it.

I agree, and that's why the default is read-only, and you have to use
apply a switch to enable writing at all. The hope was that at the point
the user makes a conscious decision to choose and type “--allow-writes”
they will have applied some thought to the consequences of that.

(And the default is also to only serve the cwd, not the whole
filesystem. That's not maximally paranoid by default, but it's pretty
good I think.)

> sshd sets some environment variables for the subprocess. On a brief
> inspection this doesn't seem to help us distinguish bzr started by
> "ssh host bzr --inet" and "ssh host" then restarting inetd from the
> shell, but there may be more detail or it might work as a partial
> protection.

Yeah, and if we infer things from environment variables it's easy to
imagine issues with older OpenSSH sshd versions, or non-OpenSSH sshd
implementations.

> Compatibility with existing clients is important.
>
> We can take a step forward by saying that at least for server mode,
> listening on --http or as a daemon, you need a special step to allow
> write access. This shouldn't break normal use, and people with a
> special use can think about what they want. Robert has a point with
> something like a "--auth" option indicating what authentication is
> acceptable.

Complicated rules for a command along the lines of “--auth=... is
mandatory unless --inet is provided” feel a bit messy to me. Maybe
what's needed is separate commands for “start a TCP server” and “start a
bzr+ssh” server? I'm not sure that that is the best answer, but the
different requirements for typical bzr+ssh vs. TCP are a bit suggestive
of it. And we're planning on adding --http to the mix...

-Andrew.

« Back to merge proposal