[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 backward-compatibility confidentiality review-needed  |   Launchpad_bug:            
--------------------------------------------------------------------------------------------------+

Comment(by davidsarah):

 [fixed formatting]

 Replying to [comment:43 kevan]:
 > The solution that I think ended up being the one that everyone agreed
 with was stated in comment:21. This was further elaborated on in
 comment:27. Paraphrasing:
 >
 >   * When reading an immutable directory, caps that we can interpret, and
 that are known to be mutable should be omitted entirely. When writing an
 immutable directory, the existence of caps that are mutable should be
 cause for an error and a failure.

 Yes. This should fail the operation with a MustBeDeepImmutableError.

 >   * When reading an immutable directory, unknown caps in the
 {{{rw_uri}}} slot should be silently ignored,

 In an immutable directory, the {{{rwcapdata}}} field that would contain
 the encrypted and mac'd {{{rw_uri}}} should always be empty (zero-length),
 since the directory has no writekey. In the original patches I ignored
 this field, but on reflection I think we should treat a non-empty
 {{{rwcapdata}}} in the same way as a netstring decoding error, i.e. raise
 ValueError. That's what the latest patch [new-all-diff.txt] does.

 We also test that an immutable directory's underlying filenode has no
 {{{.get_writekey()}}} method, so that the code to decrypt this field would
 fail anyway if it were mistakenly executed.

 > and unknown caps in the {{{ro_uri}}} slot should have an {{{ro.}}} or
 {{{imm.}}} prefix removed, and replaced with an {{{imm.}}} prefix.

 Yes.

 > When writing an immutable directory, unknown caps in the {{{ro_uri}}}
 slot should be prefixed with {{{imm.}}} if they are not already,

 No, this isn't necessary: the {{{ro_uri}}} slots of an immutable directory
 are implicitly immutable. The {{{imm.}}} will be added when the slot is
 read out by a client that doesn't understand the cap format, but it is not
 stored. In fact, {{{ro.}}} or {{{imm.}}} will be stripped if present (by
 {{{strip_prefix_for_ro}}} in unknown.py).

 (If the prefix were not stripped, then it would be more difficult to test
 what happens when {{{.imm}}} is absent -- this would be a case that the
 attacker could provoke that would not occur in normal operation.)

 > and the existence of unknown caps in the {{{rw_uri}}} slot should be
 cause for an error and a failure.

 (extrapolating from comment:20)

 Yes. Actually the existence of *any* cap in the {{{rw_uri}}} slot of an
 immutable directory is a cause for failure. However it is valid to provide
 a {{{rw_uri}}} in the JSON representation or in an URL parameter if it can
 be diminished to a read cap (handled by existing code in
 dirnode.py:create_from_cap), or in the latest version of the patch, if it
 already had an {{{imm.}}} prefix. In either of those cases, it will be
 moved to the {{{ro_uri}}} slot.

 >   * When reading a mutable directory, unknown caps in the {{{rw_uri}}}
 slot should be passed through as normal.

 When writing a mutable directory, unknown caps in the {{{rw_uri}}} slot
 should be passed through as normal, and unknown caps in the {{{ro_uri}}}
 slot should have any existing prefix removed and replaced with {{{ro.}}}.

 Not quite. If there was an existing {{{imm.}}} prefix, it should stay.
 (This would happen for an unknown cap that is copied from an immutable
 directory to a mutable one. It ensures that the constraint that the cap
 was supposed to refer to an immutable subtree is not forgotten.)

 Similarly to the immutable case, if an unknown cap is given in a
 {{{rw_uri}}} slot with no {{{ro_uri}}} and the cap already has a {{{ro.}}}
 prefix, then it will be moved to the {{{ro_uri}}} slot. (This makes it
 possible to link a {{{ro.}}} or {{{imm.}}}-prefixed unknown cap into an
 existing directory using the WUI, without needing a JSON request.)

 >   * When presented with a cap prefixed with {{{imm.}}} or {{{ro.}}},
 webapi servers should see if it is a cap that they understand without the
 prefix. If it is, they should attempt to verify that it matches the prefix
 -- in other words, that it is immutable if prefixed with {{{imm.}}} (this
 is suggested in comment:29).

 ... and read-only if prefixed with {{{ro.}}}. Yes.

 > Have I misunderstood anything? Have I missed anything? If not, I'll
 accept this ticket and start reviewing it.

 Please do, thanks.

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