ContainerListenerImpl is reading untrusted input, make sure it has a fuzz test to avoid security issues
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c27c5e6107f6d0c4025973203359f52a513583b8 commit c27c5e6107f6d0c4025973203359f52a513583b8 Author: Ian Barkley-Yeung <iby@google.com> Date: Thu Nov 15 10:16:38 2018 vm_tools: Make cicerone's Service testable To allow some fuzz testing of cicerone, make Service testable: 1. Shut down grpc_server_tremplin_ so that ~Service eventually completes. 2. Allow dependency injection of DBus to allow tests to mock out DBus 3. Add methods to access grpc services we want to test BUG=chromium:904028 TEST=Ran container, shut down container Change-Id: If9e3cdfa89b585d98556d2a3dc280d9efc95fe42 Reviewed-on: https://chromium-review.googlesource.com/1330098 Commit-Ready: Ian Barkley-Yeung <iby@chromium.org> Tested-by: Ian Barkley-Yeung <iby@chromium.org> Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com> [modify] https://crrev.com/c27c5e6107f6d0c4025973203359f52a513583b8/vm_tools/cicerone/service.cc [modify] https://crrev.com/c27c5e6107f6d0c4025973203359f52a513583b8/vm_tools/cicerone/main.cc [modify] https://crrev.com/c27c5e6107f6d0c4025973203359f52a513583b8/vm_tools/cicerone/service.h
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/34b0c27da9c1972e4ef9cc777e069e152b41fc26 commit 34b0c27da9c1972e4ef9cc777e069e152b41fc26 Author: Ian Barkley-Yeung <iby@google.com> Date: Tue Jan 08 09:59:11 2019 vm_tools: Add unit test for ContainerListenerImpl This is a basic unit test for ContainerListenerImpl. I'm only doing basic testing in this commit; this change is more about getting the framework up and running than testing every corner case. Once this is done, I plan to use the same framework to do fuzz testing. Note that the framework is designed with this in mind (that's why ServiceTestingHelper is in its own file and not derived from testing::Test.) BUG=chromium:904028 TEST=Ran cros_run_unit_tests --board=eve --packages vm_host_tools Change-Id: I4ac6fea9cb7fc248506c4524d216c0e975beddbe Reviewed-on: https://chromium-review.googlesource.com/1376509 Commit-Ready: Ian Barkley-Yeung <iby@chromium.org> Tested-by: Ian Barkley-Yeung <iby@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/tremplin_listener_impl.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/tremplin_listener_impl.h [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/service_testing_helper.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/container_listener_impl.cc [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/dbus_message_testing_helper.cc [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/tremplin_test_stub.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/client.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/virtual_machine.h [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/tremplin_test_stub.h [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/service.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/main.cc [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/host.gypi [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/service.h [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/dbus_message_testing_helper.h [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/container_listener_impl.h [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/service_testing_helper.h [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/system_api/dbus/vm_cicerone/dbus-constants.h [modify] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/virtual_machine.cc [add] https://crrev.com/34b0c27da9c1972e4ef9cc777e069e152b41fc26/vm_tools/cicerone/container_listener_impl_test.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/db6f5e5c17517bed00881166dafdc941a1fa5143 commit db6f5e5c17517bed00881166dafdc941a1fa5143 Author: Ian Barkley-Yeung <iby@google.com> Date: Fri Jan 11 17:47:57 2019 vm_tools: Fix issues where cicerone could lock up Fix issues where bad messages could lock up cicerone. Found by cicerone fuzz test! BUG=chromium:904028 TEST=Started and stopped Crostini, ran programs in Crostini, opened URLs from within Crostini. Change-Id: I34d990123fd11aed162d7109bb1d6fa39468d6f2 Reviewed-on: https://chromium-review.googlesource.com/1403904 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Ian Barkley-Yeung <iby@chromium.org> Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com> [modify] https://crrev.com/db6f5e5c17517bed00881166dafdc941a1fa5143/vm_tools/cicerone/service.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/a3a8f5f1482f619b0533c7d5d4c7a9559651f867 commit a3a8f5f1482f619b0533c7d5d4c7a9559651f867 Author: Fergus Dall <sidereal@google.com> Date: Wed Jan 16 09:46:50 2019 vm_tools: Speed up ServiceTestingHelper destructor The fuzz test in https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1407456 spends approximatly 99% of its run time in the destructor for ServiceTestingHelper. This CL changes the destructor to shutdown the tremplin server before waiting for the ServiceTestingHelper::DestroyService to complete. This reduces the run time of the method by around 30%, from ~3 seconds to ~2 seconds. BUG=chromium:904028 TEST=Ran unit tests, ran fuzz test while timing this method Change-Id: I809c1c46f58ad0bcac5e2169efd3281d738fdda8 Reviewed-on: https://chromium-review.googlesource.com/1409669 Commit-Ready: Fergus Dall <sidereal@google.com> Tested-by: Fergus Dall <sidereal@google.com> Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> Reviewed-by: Ian Barkley-Yeung <iby@chromium.org> [modify] https://crrev.com/a3a8f5f1482f619b0533c7d5d4c7a9559651f867/vm_tools/cicerone/service_testing_helper.cc
Comment 1 by bugdroid1@chromium.org
, Nov 15