helper: client should verify ciphertext hashes and UEB #723

New Issue

warner · 2009-05-31T19:46:27Z

warner commented

2009-05-31 19:46:27 +00:00

Prompted by a question from Daira, I spent some time today reviewing the helper code, and realized that the client should be doing more verification of the data that the helper returns to it. Specifically, the client should:

locally compute the ciphertext hashes (flat and Merkle tree), in upload.EncryptAnUploadable
compare these against the versions returned by the helper, in AssistedUploader._build_verifycap upload_results.uri_extension_data
compute and compare the resulting UEB hash

This would prevent the helper from causing an integrity violation. With the present behavior, the helper can flip bits in the ciphertext (or upload a completely unrelated ciphertext of the same length) and return the resulting ciphertext hash to the trusting client. Because the client doesn't perform any validation of the response, it will simply build and return the resulting filecap. Later, when someone attempts a download with this filecap, they will receive the altered ciphertext (but it will match the hash provided by the helper) and try to decrypt it. Since we removed the plaintext hashes in changeset:7996131a0aa0b55c,changeset:7b21054c33d4651d,changeset:1e097766c9b4c873,changeset:db566db31a66e076, the downloader hash no way to check the plaintext either, and will return corrupted plaintext to the end user.

With the current codebase, this won't be too much work. But when
pycryptopp#18 (allow random-access AES-CTR encryption) is fixed, I'd like to improve the assisted-uploader code to be more efficient in the resumed-upload case (by not encrypting-then-discarding all the previously-uploaded data), at which point this locally-generate-hashes fix would become more difficult. Or rather, I might have to forego the resumed-upload improvement to retain the don't-rely-on-helper-for-integrity property that this ticket would provide.

Prompted by a question from Daira, I spent some time today reviewing the helper code, and realized that the client should be doing more verification of the data that the helper returns to it. Specifically, the client should: * locally compute the ciphertext hashes (flat and Merkle tree), in `upload.EncryptAnUploadable` * compare these against the versions returned by the helper, in `AssistedUploader._build_verifycap` `upload_results.uri_extension_data` * compute and compare the resulting UEB hash This would prevent the helper from causing an integrity violation. With the present behavior, the helper can flip bits in the ciphertext (or upload a completely unrelated ciphertext of the same length) and return the resulting ciphertext hash to the trusting client. Because the client doesn't perform any validation of the response, it will simply build and return the resulting filecap. Later, when someone attempts a download with this filecap, they will receive the altered ciphertext (but it will match the hash provided by the helper) and try to decrypt it. Since we removed the plaintext hashes in changeset:7996131a0aa0b55c,changeset:7b21054c33d4651d,changeset:1e097766c9b4c873,changeset:db566db31a66e076, the downloader hash no way to check the plaintext either, and will return corrupted plaintext to the end user. With the current codebase, this won't be too much work. But when [pycryptopp#18](http://allmydata.org/trac/pycryptopp/ticket/18) (allow random-access AES-CTR encryption) is fixed, I'd like to improve the assisted-uploader code to be more efficient in the resumed-upload case (by not encrypting-then-discarding all the previously-uploaded data), at which point this locally-generate-hashes fix would become more difficult. Or rather, I might have to forego the resumed-upload improvement to retain the don't-rely-on-helper-for-integrity property that this ticket would provide.

warner added the

labels 2009-05-31 19:46:27 +00:00

warner added this to the undecided milestone 2009-05-31 19:46:27 +00:00

tahoe-lafs modified the milestone from undecided to 1.7.0

2010-02-02 00:31:39 +00:00

davidsarah commented

2010-04-12 17:24:04 +00:00

Not sure if I'll be able to fit this in for 1.7, since I haven't looked at the code that uses the helper yet. Keeping it in that milestone for the time being.

tahoe-lafs modified the milestone from 1.7.0 to 1.8.0

2010-06-12 20:56:06 +00:00

tahoe-lafs modified the milestone from 1.8.0 to soon

2010-08-08 05:30:25 +00:00

Sign in to join this conversation.