[tahoe-dev] [tahoe-lafs] #833: reject mutable children when *reading* an immutable dirnode

[tahoe-lafs]

#833: reject mutable children when *reading* an immutable dirnode
-------------------------------------------------------------+--------------
 Reporter:  warner                                           |           Owner:  davidsarah
     Type:  defect                                           |          Status:  assigned  
 Priority:  critical                                         |       Milestone:  1.6.0     
Component:  code-dirnodes                                    |         Version:  1.5.0     
 Keywords:  integrity forward-compatibility confidentiality  |   Launchpad_bug:            
-------------------------------------------------------------+--------------

Comment(by davidsarah):

 Replying to [comment:26 davidsarah]:
 > Note: the intent of the patch I'm writing for this ticket is not to
 alter the current behaviour that a client cannot change or add a dir entry
 containing a cap that it does not understand. (It would not be able to
 attenuate the cap in case it is a write cap.)

 I've changed my mind on this. In all but two of the webapi interfaces that
 involve creating or changing child dirnodes, the operation either creates
 the caps that it adds (in which case they are known), or is given them in
 the format {"rw_uri": ..., "ro_uri": ...}. So if the webapi client knows
 how to attenuate the cap (or already has it in that form) but the webapi
 server -- i.e. storage client -- doesn't, then that's OK.

 The two (almost equivalent) interfaces that don't take a cap with separate
 "rw_uri" and "ro_uri" slots, are [source:docs/frontends/webapi.txt#L647
 PUT /uri/$DIRCAP/SUBDIRS../CHILDNAME?t=uri] and POST
 /uri/$DIRCAP/SUBDIRS../?t=uri&name=CHILDNAME&uri=CHILDCAP, i.e. the "hard
 link existing cap" operations. In that case we don't know how to attenuate
 the cap, so we cannot safely accept an unknown cap to be added.

 However, I don't think there's any reason to restrict the other
 operations. We obtain no security benefit from doing so, because a
 malicious client (with the relevant writecap) can always write whatever it
 likes into a directory. The important thing is to make sure that clients
 who obtain caps ''from'' the directory are not confused.

 So, I propose to remove the following check in
 source:src/allmydata/dirnode.py#L402 (and similarly at L427):
 {{{
   if isinstance(child_node, UnknownNode):
       # don't be willing to pack unknown nodes: we might accidentally
       # put some write-authority into the rocap slot because we don't
       # know how to diminish the URI they gave us. We don't even know
       # if they gave us a readcap or a writecap.
       msg = "cannot pack unknown node as child %s" % str(name)
       raise CannotPackUnknownNodeError(msg)
 }}}

 (and remove {{{CannotPackUnknownNodeError}}}). Instead, the logic used by
 a webapi server when packing (encoding) and unpacking (decoding) a dirnode
 will be as follows:

 If the directory is immutable,
  * always omit {{{rw_uri}}} slots (i.e. silently ignore whatever was
 there) when unpacking.
  * raise an error (fail the webapi operation with no side effects on the
 directory) if an {{{rw_uri}}} slot was present when packing.
  * if the packed {{{ro_uri}}} is unknown, strip any "ro." or "imm." prefix
 and then prepend "imm.".

 If the directory is mutable,
  * pass through any {{{rw_uri}}} slots when packing and unpacking.
  * if the packed {{{ro_uri}}} is unknown and it does not already have a
 "ro." or "imm." prefix, then prepend "ro.".

 An URI is "unknown" if [source:src/allmydata/uri.py#L504 {{{from_string}}}
 in uri.py] returns an instance of UnknownURI.

 So the effect is that the URI returned in the {{{ro_uri}}} slot will be
 unusable by ''correct'' clients if it does not meet the constraint that it
 should have met to be in that slot -- i.e. being either read-only or deep-
 immutable.

-- 
Ticket URL: <http://allmydata.org/trac/tahoe/ticket/833#comment:27>
tahoe-lafs <http://allmydata.org>
secure decentralized file storage grid