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

[francois]

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.

[warner]

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!

[warner]

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.

[warner]

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.

[warner]

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.

[warner]

bumping again, this won't be finished in June

[warner]

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).

[davidsarah]

#746 may be an instance of this.

[davidsarah]

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

[davidsarah]

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

[zooko]

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.

[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.