Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-4322

Path-based authz can prevent cp (e.g., branching, tagging) unexpectedly.

    XMLWordPrintableJSON

    Details

      Description

      Path-based authorization on subdirectories can cause copying (especially
      branching/tagging) to fail in a way users probably don't expect.
      
      Scenario:
      
      Imagine a user U who can ls, checkout, etc the trunk/ directory.  But say that
      directory has a subdirectory, /trunk/private, that is invisible to U (i.e., U
      does not have read nor write access to /trunk/private).  If U tries to branch or
      tag trunk, she will discover to her surprise that she cannot do it.  Even though
      U can check out trunk, she U can't make a branch of it, even though she
      supposedly has write access to /branches and, as far as she knows, read access
      to /trunk.
      
      Belwo is an IRC conversation about it, heavily edited for clarity/flow.  Thanks
      to danielsh and philipm for thinking through the implementation oddity that
      would be involved in fixing this; hence the "needsdesign" tag.  (Note that this
      started as http://subversion.tigris.org/issues/show_bug.cgi?id=2662#desc29, and
      then was made into this separate issue because it's not really about wildcards
      per se, although http://subversion.tigris.org/issues/show_bug.cgi?id=2662#desc30
      explains the connection between the two issues.)
      
       <kfogel>   suppose I have "* = rw" for / of repos R, and a sub-authz
                  rule for "R:/trunk/private" that says "* = " with the
                  next line saying "@privategroup = rw".
       
       <kfogel>   Now, suppose I have a user U, who is *not* a member
                  of @privategroup.  U tries to make a branch of trunk (and
                  trunk has a 'private' subdirectory at the time U tries to
                  make the branch):
       
       <kfogel>     svn --username U cp -m "Seeing what happens."       \
                    http://svn.red-bean.com/repos/R/trunk               \
                    http://svn.red-bean.com/repos/R/branches/newbranch
       
       <kfogel>   Now U will get this error:
       
       <kfogel>     svn: access to '/repos/R/!svn/bc/2494/trunk' forbidden
       
       <kfogel>   Should U be able to make the branch, and just not have a
                  private/ subdir in the branch?  Or is an error the right
                  reaction?  (In which case, the error is quite confusing,
                  but then again, it kind of has to be, because it can't
                  reveal the existence of the private subdirectory.)
       
       <danielsh> kfogel: if you allow the branch to be created, what would
                  'svn log -qvl1' on the resulting revision print for that
                  user? 
       
       <danielsh> kfogel: point is that branches/newbranch/private is NOT
                  covered by authz rules
       
       <kfogel>   The resulting revision (the one that created the branch)
                  would not *contain* the private/ directory, so there's no
                  log adjustment to be made anyway.  It should just print
                  normal results.
       
       <kfogel>   (We already have an accepted 'svn log' behavior for
                  revisions that contain changes to paths one isn't allowed
                  to see.  When I have U do 'svn log' on a revision that
                  modified something in /trunk/private/, svn has a reasonable
                  behavior right now -- namely, to show me nothing).
       
       <kfogel>   danielsh: if some other user (one who was a member of
                  @privategroup) had done the cp, things might be different
       
       <kfogel>   danielsh: in that case, when that privileged user does "svn
                  log", they'll see everything, and the private/ dir will be
                  in the branch; and when user U (unprivileged) does 'svn
                  log' on that revision, they will see whatever they would
                  see on any other revision that involves a mixture of
                  visible and invisible things.
       
       <danielsh> kfogel: but if the revision doesn't contain the private/
                  dir, you have to do a "delete private/" in the FS, and then
                  you have in the revision a readable path with unreadable
                  copyfrom... and I'm just not sure how we handle that.
       
       <philipm>  How could the user make a copy that doesn't include the
                  authz prohibited child?
       
       <danielsh> kfogel: I'm tryig to see if allowing the copy to go through
                  (sans private/) would make sense 
       
       <kfogel>   danielsh: asking more about desired behavior than implementation
       
       <kfogel>   philipm: right now, they can do a checkout of trunk/.
       
       <kfogel>   philipm: so, my point is, they reasonably expect that they
                  can do a cp of something they can already do a checkout or
                  an ls of.
       
       <kfogel>   philipm: then they try it, and get a weird error.
       
       <philipm>  Can they copy the working copy?
       
       <kfogel>   philipm: sure... heh, ummm... I wonder if that works right.
                  Testing now!
       
       <philipm>  If you try to commit the copy and delete the excluded child
                  the FS will reject the commit.
       
       <philipm>  It rejects it at the copy stage.
       
       <philipm>  It wouldn't be a cheap copy. It would be a cheap copy with
                  modifications.
       
       <danielsh> it should let the copy go through, but delete the private/
                  child, and hide the existence of the deletion from
                  unauthn'd users.
       
       <kfogel>     $ svn cp --username U -m                         \
                      "Creating a branch from wc this time."         \
                      . http://svn.red-bean.com/repos/R/branches/ug
       
                    svn: Directory 'private' is missing
                    svn: Directory 'private' is missing
       
       <kfogel>   philipm, danielsh: (yes, it printed that twice)
       
       <kfogel>   philipm, danielsh: so, in 1.6.7, the server reveals to the
                  client the name of the forbidden directory.  If that's
                  still true in 1.7.x, we have a problem.
       
       <philipm>  They have always been revealed
       
       <kfogel>   philipm: I didn't realize that.
       
       <danielsh> 19:11:42 @danielsh | it should let the copy go
                  through, but delete the private/ child, and hide
                  the existence of the deletion from unauthn'd users.
       
       <danielsh> kfogel, * : ^^^ is that implementable?  Would it work? 
       
       <kfogel>   danielsh: in my opinion, that's the right user-visible
                  behavior.  How hard it is to implement, I have no idea.
       
       <kfogel>   danielsh: (I am going to note all this in #2662, though, so
                  at least we don't have to think it all through again)
       
       <danielsh> kfogel: you would create the revision as a copy + delete of
                  a child, and then hide paths that have unreadable
                  copyfrom... ? 
       
       <danielsh> not sure if that works, easy to implement, makes sense, etc
       
       <danielsh> keywords.add("needsdesign") # :-) 
       
       <kfogel>   danielsh: Mmmmm.  I think that's right.  In other words, if
                  user U does 'svn log' on the new revision, U doesn't see
                  the deletion of the privileged path.  However, if a
                  privileged user does 'svn log', they can see that deletion
                  too.  Agree about the "needsdesign" -- #2662 is already
                  marked with that :-).
       
       <danielsh> kfogel: I'm sure there is a boatload of corner cases in
                  this back-of-the-envelope design ... 
       
       <kfogel>   danielsh: yup
      

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              kfogel Karl Fogel
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated: