Delete p2p file if metadata or payload verification fails.
Currently, if metadata or payload verification fails we don't delete
the p2p file. In the payload verification case, this is especially
problematic as other devices may pick up the bad (and complete)
payload still being shared, fail themselves, and then continue to
propagate the badness. The metadata verification case is slightly less
bad, since we only leave a .tmp file around in /var/cache/p2p.
Note that this problem never manifest itself unless someone
deliberately injects a bad payload. The severity is additionally
mitigated by the fact that p2p will turn itself off after ten update
attempts or two wall-clock days, whichever comes first.
I noticed this while working on CL:177631.
This CL fixes the problem. I verified the fix by serving a payload in
the following way:
Fail metadata verification:
$ ./devserver.py --test_image \
--private_key \
../update_engine/unittest_key.pem \
--private_key_for_metadata_hash_signature \
../update_engine/unittest_key.pem \
--public_key \
../update_engine/unittest_key2.pub.pem
Fail payload verification:
$ ./devserver.py --test_image \
--private_key \
../update_engine/unittest_key.pem \
--private_key_for_metadata_hash_signature \
../update_engine/unittest_key2.pem \
--public_key \
../update_engine/unittest_key2.pub.pem
to a device with p2p enabled and ensured that the p2p files were
deleted on failure.
BUG=chromium:323733
TEST=Manually tested, see above.
Change-Id: Ieb439972f5415727920ff5b0c22dbb86503f911c
Reviewed-on: https://chromium-review.googlesource.com/178132
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
diff --git a/download_action.cc b/download_action.cc
index 57070b5..ab33491 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -260,6 +260,9 @@
if (writer_ && !writer_->Write(bytes, length, &code_)) {
LOG(ERROR) << "Error " << code_ << " in DeltaPerformer's Write method when "
<< "processing the received payload -- Terminating processing";
+ // Delete p2p file, if applicable.
+ if (!p2p_file_id_.empty())
+ CloseP2PSharingFd(true);
// Don't tell the action processor that the action is complete until we get
// the TransferTerminated callback. Otherwise, this and the HTTP fetcher
// objects may get destroyed before all callbacks are complete.
@@ -293,6 +296,9 @@
if (code != kErrorCodeSuccess) {
LOG(ERROR) << "Download of " << install_plan_.download_url
<< " failed due to payload verification error.";
+ // Delete p2p file, if applicable.
+ if (!p2p_file_id_.empty())
+ CloseP2PSharingFd(true);
} else if (!delta_performer_->GetNewPartitionInfo(
&install_plan_.kernel_size,
&install_plan_.kernel_hash,