#625 assigned defect

Can't repair read-only dirnodes/mutable-files

Reported by: francois Owned by: warner
Priority: major Milestone: soon
Component: code-mutable Version: 1.3.0
Keywords: confidentiality integrity preservation verify repair newcaps tahoe-backup usability anti-censorship excess-authority Cc: francois@…
Launchpad Bug:

Description

Running a deep-check operation on a dirnode created with "tahoe backup" fails with this exception.

MutableFileNode instance has no attribute '_writekey'

Deep-check should probably silently avoid walking into ready-only running instead of raising any exception.

Attachments (1)

exception.html (8.1 KB) - added by francois at 2009-02-13T18:48:33Z.

Download all attachments as: .zip

Change History (22)

Changed at 2009-02-13T18:48:33Z by francois

comment:1 Changed at 2009-02-13T18:49:00Z by francois

  • Cc francois@… added

comment:2 Changed at 2009-02-13T20:41:21Z by warner

  • Owner set to warner
  • Status changed from new to assigned

Oops.

hm, check ought to be able to handle readonly dirnodes. Repair can't (because of the write-enabler problem), but check should.

I'll start by adding a test case. Thanks!

comment:3 Changed at 2009-02-13T20:53:00Z by warner

  • Component changed from code-storage to code-dirnodes
  • Summary changed from Deep-check chokes on readonly dirnodes to Deep-check chokes on readonly dirnodes that need repair

Ah, looking more carefully at your exception, I see that your node was trying to repair the readonly dirnode in question. I added a test case to make sure that checker can handle readonly dirnodes, and it passes.

So, yeah, deep-repair should refrain from trying to repair readonly mutable files and readonly dirnodes. I'll look at the deep-check-results and see if there's a good way to report that there were objects that need repair but which could not be repaired because they're readonly.

comment:4 Changed at 2009-02-13T22:58:30Z by warner

  • Milestone changed from undecided to 1.3.1

Drat, it looks like this is just too tricky to fix in time for 1.3.0: it requires a better test harness than we currently have, and some more thought about how the problem should be reported (I'm thinking that "repair_attempted" should be incremented, but "repair_succeeded" should not, and the per-file check-and-repair results should include a CannotRepairReadonlyMutableFileError failure).

So we've added a note to NEWS to mention the bug, and we'll put it high on the list for the 1.3.1 release.

comment:5 Changed at 2009-04-08T02:28:24Z by warner

  • Milestone changed from 1.4.0 to 1.5.0
  • Summary changed from Deep-check chokes on readonly dirnodes that need repair to Can't repair read-only dirnodes/mutable-files

Nope, this is getting bumped again.. we haven't made enough progress on it. The new test harness is in place (NoNetworkTestGrid), but I'm still not sure how we should handle it. The root issue is that readonly dirnode don't give us enough information to compute the Write Enabler (by design), and when repair needs to create new shares, it must give those shares a Write Enabler so that later writers can modify those shares.

We've considered changing the storage-server protocol to have the server validate shares (by checking their signatures): this would remove the need for Write Enablers, but would also obligate servers to know more about the client's data format (causing version dependencies between clients and servers) and increasing the workload of the servers. Despite these tradeoffs, I think we're likely to move in this direction when we implement Accounting, since Write Enablers are a nuisance (and require an encrypted transport layer, which it would be nice to avoid).

But until then, we may have to decide between being able to repair read-only mutable files, and being able to modify those shares later. One possible change (that wouldn't involve modifying any protocols) would be to have the repairer provide an all-zeros Write Enabler, and then if anyone tries to modify the share again later, have the server validate the signatures and replace the WE if they check out. This would have some security implications, in particular making it possible to cause rollbacks in certain situations.

comment:6 Changed at 2009-06-19T18:44:51Z by warner

  • Milestone changed from 1.5.0 to 1.6.0

bumping again, this won't be finished in June

comment:7 Changed at 2009-06-29T22:54:46Z by warner

  • Component changed from code-dirnodes to code-mutable

I'm going to try to make the june release at least tolerate (i.e. skip over) read-only dircaps, so deep-check-and-repair can work on all the files.

I've also got an idea about a relatively clean way to address this: use an all-zeros WE to ask the server to please validate the new share instead of relying upon the WE for access control. This will require significant (but compatible) changes to both the client and the server. Also note that the current mutable share format doesn't allow the server to validate the encrypted private key, but it can validate all the other bits. One criteria for the new DSA-based mutable file design (#217) is that every bit of the slot data must be validateable by the server (i.e. if we must embed key material in the share, it must be covered by the signature).

comment:8 Changed at 2009-12-06T00:00:10Z by davidsarah

  • Keywords repair newcaps added
  • Priority changed from minor to major

comment:9 Changed at 2009-12-06T00:02:55Z by davidsarah

#746 may be an instance of this.

comment:10 Changed at 2009-12-06T14:14:37Z by francois

  • Keywords tahoe-backup added

comment:11 Changed at 2009-12-20T15:43:15Z by davidsarah

  • Keywords verify added

comment:12 Changed at 2009-12-30T00:30:57Z by davidsarah

  • Keywords usability added

The current code seems to raise RepairRequiresWritecapError (see source:src/allmydata/mutable/repairer.py#L103), rather than the error in the description.

comment:13 Changed at 2010-01-26T15:39:46Z by zooko

  • Milestone changed from 1.6.0 to eventually

comment:14 Changed at 2010-02-11T01:20:30Z by davidsarah

  • Keywords confidentiality integrity added

See also #568 (make immutable check/verify/repair and mutable check/verify work given only a verify cap).

comment:15 Changed at 2010-02-11T01:28:49Z by davidsarah

  • Keywords preservation added

comment:16 Changed at 2010-02-27T08:54:09Z by davidsarah

  • Milestone changed from eventually to 1.7.0

comment:17 Changed at 2010-05-08T19:41:02Z by zooko

  • Milestone changed from 1.7.0 to 1.8.0

From comment:7: "This will require significant (but compatible) changes to both the client and the server.". Well I guess that means it isn't going to happen for v1.7 then! :-) Bumping to v1.8.0 instead of to "eventually" because it seems somewhat urgent and important to fix this.

comment:18 Changed at 2010-08-09T22:17:28Z by zooko

  • Milestone changed from 1.8.0 to soon

comment:19 Changed at 2010-10-29T04:36:21Z by zooko

Once this ticket is fixed then (as mentioned on #1234), we should change the user interfaces to diminish write caps to read caps when appropriate when emitting them or logging them, such as when reporting that a cap is unrecoverable.

comment:20 Changed at 2010-12-16T01:17:34Z by davidsarah

  • Keywords anti-censorship added

comment:21 Changed at 2011-10-18T19:39:53Z by davidsarah

  • Keywords excess-authority added
Note: See TracTickets for help on using tickets.