While working on the checkout performance issues (see the thread
"issue #1429 progress: ra_dav changing analogously to ra_svn"), Ben
and I discovered an interesting security leak in the *current* code.
It turns out the solution to this leak is related to changes we were
planning anyway for issue #1429. So if you haven't read the
above-referenced thread, you should do so now, otherwise some of this
mail will be hard to understand.
Here's the security leak:
Currently, an ra_dav checkout or update works by sending a request for
a custom report. The request tells the server what kind of tree
(working copy) the client has. The server sends back a skelta
describing the changes the client needs to make to update that tree.
There are no file contents in the skelta, just little "<fetch-file/>"
markers alerting the client that it needs to do a GET for that file.
So while processing the skelta, the client is also issuing all these
GET requests for file contents. This is where mod_authz_svn would
kick in. Apache's authz framework just naturally checks read
authorization on each resource, when the GET request for that resource
arrives. Yay.
The alert reader will have already spotted the leak: although we do
not include file contents in the skelta, we unconditionally tell the
client the paths it needs to GET. So even if some GET requests would
fail for lack of authorization, the server has already revealed the
names and locations of those resources, which ought to be invisible.
The solution to the leak is related to our plans to improve
checkout/update performance:
Recall what we said in the "issue #1429 progress" thread, regarding
the plan to send back a full delta instead of a skelta (to avoid
latency-sensitive network turnarounds):
> 1. Now we have an authorization problem. Because no GET requests
> are being issued, mod_authz_svn doesn't work, because the
> standard Apache authorization mechanism never kicks in.
We were going to deal with that authorization problem by having
mod_dav_svn issue Apache "subrequests" as its streaming the delta back
to the client. A subrequest is a way for mod_dav_svn to invoke the
installed Apache authorization handler(s).
Now, suppose a subrequest reveals that a resource is not authorized --
what should we do then?
One answer is to error. The server stops sending the delta
immediately, and the client just bombs out with a checkout or update
error.
The problem with this is that it's worse than CVS. In CVS, if you
don't have authorization for (say) some files in a directory, due to
fancy Unix permissions or whatever, the server just skips those files
and sends warnings back to the client. The client prints out the
warnings as it goes, and at the end you have a valid working copy
that's just missing some files. You can still update, commit from it,
etc. Of course, updates will issue warnings each time, but other than
that everything works. You'd be surprised how many people actually
depend on this behavior :-).
(Also, CVS has the even more eye-bugging possibility that if a subdir
wasn't checked out due to authorization failure, you could try again
to check it out in place with a different, presumably authorized
username. This works because CVS associates the auth info with the
working copy. We could go there too.. But let's not. Let's solve one
problem at a time.)
It would be good for Subversion allowed such "incomplete"
checkouts/updates too. We already have a working copy concept of
incompleteness. Now that the server would too, it would be easy for
the update report to express it back to the client: if a dir contains
unauthorized objects, just include a self-closing "<incomplete/>" tag
for that directory, which the client would use to mark the directory
as incomplete. The client would also grow a new notification code, to
print out warnings about the incompletenesses as it goes, and then at
the end it would say (once) that there were incompletenesses above.
Here's the problem: currently, that incomplete bit can travel all the
way from the repository to the client, only to run up against a brick
wall: the editor interface. There's no way to tell the editor that a
directory should be left in incomplete state.
We propose, therefore, to add an 'incomplete' boolean flag to
svn_delta_editor_t->close_directory() and close_edit(), so we can
express this state.
Note that although I've been talking in terms of ra_dav, this flag is
for all RA layers. The concepts of partial authorization and
incompleteness apply to core Subversion, not just for some particular
RA layer, so this change benefits Subversion itself.
Obviously, once we have the concept of incompleteness, we can toy
around with various kinds of retry scenarios, mixed-auth working
copies, etc. I don't want to devote cycles to that right now; just
having incomplete working copies will be better than bombing out on
the error, so let's start there.
Note that even with this, there is still a subtle leak: the client
doesn't know the names of the prohibited objects, but it still knows
that at least one prohibited object exists. We say, c'est la vie :-).
It would be nice to have zero knowledge leak, but if don't tell the
client to mark that working copy dir as incomplete, then we get into a
whole messy question of how to efficiently report wc state to the
server for updates. Dante Says: Don't Go There.
Oh, I almost forgot, the Compatibility Plan:
At first, the server will only send back this "<incomplete/>" flag to
clients that know how to handle it. It will recognize such clients,
because they will be the ones that explicitly request the new full
delta update report. Reports that were not requested this way default
to skelta, and the skelta will not use the new incomplete tag at
first. However, starting now, clients will be able to handle the
incomplete tag in both skeltas and deltas. Thus, after a while we can
tell the server to start sending it even in skeltas, thus plugging the
security leak for all time.
Whew. That's it. If you read this far, buy yourself a beer :-).
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 28 22:51:52 2003