Photo Picker: The decoding service needs to enable seccomp-bpf |
||||||
Issue descriptionRight now the decoding service uses an isolated service without any native code, but that means we also don't enable seccomp-bpf as we do in our other untrusted processes ( Issue 166704 ). The added value is significant, so we should. The code currently lives in //content, but Robert volunteered to look into moving it to //sandbox so that we can load it independently of the rest of Chrome.
,
Jul 5 2017
I'm going to get the policy moved into //sandbox hopefully this week, but someone who knows how the Android build works will need to help with the .so split.
,
Jul 6 2017
,
Jul 6 2017
First CL: https://chromium-review.googlesource.com/c/562776/ There'll be one followup before this is usable for the photo picker.
,
Jul 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2255a2ede0b102f81ee5fcef6334215bfc76d1d commit f2255a2ede0b102f81ee5fcef6334215bfc76d1d Author: Robert Sesek <rsesek@chromium.org> Date: Fri Jul 07 16:02:39 2017 [Android] Move the baseline Seccomp-BPF policy from //content to //sandbox. This will allow other Service processes to access the BPF policy without requiring //content. Bug: 730066 Bug: 739879 Change-Id: I7466d57ce42a038cd61c743a96d86f1684a603b8 Reviewed-on: https://chromium-review.googlesource.com/562776 Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#484943} [modify] https://crrev.com/f2255a2ede0b102f81ee5fcef6334215bfc76d1d/content/common/BUILD.gn [delete] https://crrev.com/46eba0c5528b2f9124176efcc411fdc2d609039a/content/common/sandbox_linux/android/sandbox_bpf_base_policy_android.h [modify] https://crrev.com/f2255a2ede0b102f81ee5fcef6334215bfc76d1d/content/renderer/renderer_main_platform_delegate_android.cc [modify] https://crrev.com/f2255a2ede0b102f81ee5fcef6334215bfc76d1d/sandbox/linux/BUILD.gn [rename] https://crrev.com/f2255a2ede0b102f81ee5fcef6334215bfc76d1d/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc [add] https://crrev.com/f2255a2ede0b102f81ee5fcef6334215bfc76d1d/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.h
,
Jul 7 2017
Second CL: https://chromium-review.googlesource.com/c/563739/ After that lands, it should just be a few lines of code to add Seccomp with the right policy to the photo picker service.
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fb31977a3765bdece87afb6f5ae55331a627ade commit 5fb31977a3765bdece87afb6f5ae55331a627ade Author: Robert Sesek <rsesek@chromium.org> Date: Wed Jul 12 00:21:40 2017 Create a new sandbox::SeccompStarterAndroid class. This wraps common functionality for build- and run-time detection of applying the Seccomp-BPF sandbox on Android. Refactoring this out of //content will make it easier for other processes to start the Seccomp sandbox. Bug: 730066 Bug: 739879 Change-Id: Ib75003979e662865e9557c9c4f1d7b705c0692bf Reviewed-on: https://chromium-review.googlesource.com/563739 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Commit-Queue: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#485743} [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/content/public/renderer/BUILD.gn [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/content/public/renderer/seccomp_sandbox_status_android.h [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/content/renderer/renderer_main_platform_delegate_android.cc [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/content/renderer/seccomp_sandbox_status_android.cc [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/content/renderer/seccomp_sandbox_status_android.h [modify] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/sandbox/linux/BUILD.gn [add] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/sandbox/linux/seccomp-bpf-helpers/seccomp_starter_android.cc [add] https://crrev.com/5fb31977a3765bdece87afb6f5ae55331a627ade/sandbox/linux/seccomp-bpf-helpers/seccomp_starter_android.h
,
Jul 12 2017
The Android Seccomp-BPF policy and starter helper are now moved out of //content, so it should be possible to just depend on //sandbox to sandbox a new Android Service. The native code should pretty much just be a copy of this: https://chromium.googlesource.com/chromium/src/+/5fb31977a3765bdece87afb6f5ae55331a627ade/content/renderer/renderer_main_platform_delegate_android.cc#32 I left sdk_int and the device string injectable to SeccompStarterAndroid so //base JNI doesn't need to get pulled in -- it should be possible to just get them from android.os.Build.
,
Jul 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d4b1c1382e85b3a00de286b3db957156462e7385 commit d4b1c1382e85b3a00de286b3db957156462e7385 Author: Robert Sesek <rsesek@chromium.org> Date: Thu Jul 13 21:22:05 2017 Add missing dependency for //sandbox/linux:seccomp_starter_android Commit 5fb31977a3765bdece87afb6f5ae55331a627ade added this new target but didn't depend on the //sandbox:sandbox_features target so it failed to compile: In file included from ../../sandbox/linux/seccomp-bpf-helpers/seccomp_starter_android.cc:5: ../../sandbox/linux/seccomp-bpf-helpers/seccomp_starter_android.h:10:10: fatal error: 'sandbox/sandbox_features.h' file not found #include "sandbox/sandbox_features.h" NOTRY=true Bug: 742028 Bug: 730066 Bug: 739879 Change-Id: Ib0bc9f34f0cbf35be0f9f3cf412cd0fdcf678f3f Reviewed-on: https://chromium-review.googlesource.com/570514 Commit-Queue: Robert Sesek <rsesek@chromium.org> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Glenn Hartmann <hartmanng@chromium.org> Cr-Commit-Position: refs/heads/master@{#486479} [modify] https://crrev.com/d4b1c1382e85b3a00de286b3db957156462e7385/sandbox/linux/BUILD.gn
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/836222319593ae0e12107a5723d82e0d724c9b4a commit 836222319593ae0e12107a5723d82e0d724c9b4a Author: Peter Beverloo <peter@chromium.org> Date: Fri Jul 21 13:21:21 2017 Enable the seccomp-bpf sandbox for the Android photo picker This CL attempts to enable the seccomp-bpf sandbox, as we do for renderer processes on Android, for the new photo picker when it is supported by the device. The implementation loads the native library as if it's a Chrome child process, and then calls the native InitializePhotoPickerSandbox function to initialize the sandbox when available. UMA is logged. Because we end up loading the native library, this does create for an additional delay of ~700ms before the first photo is shown. There may be optimization opportunities by separating out the sandbox code in a separate shared library, but this is made difficult by (a) our build system using the crazy linker, and (b) the dependencies of //sandbox on //base. I've verified that the sandbox is enabled on two devices running different versions of Android, both by printing the status of the SeccompStarterAndroid, and through the following command: $ adb shell cat /proc/18424/status | grep Seccomp Seccomp: 2 (Where "2" means that seccomp-bpf is enabled.) BUG= 730066 Change-Id: I46e608bad8f69d3cf862c0953361c50f4c65c45c Reviewed-on: https://chromium-review.googlesource.com/577853 Reviewed-by: David Trainor <dtrainor@chromium.org> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Bo Liu <boliu@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Commit-Queue: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#488643} [modify] https://crrev.com/836222319593ae0e12107a5723d82e0d724c9b4a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java [modify] https://crrev.com/836222319593ae0e12107a5723d82e0d724c9b4a/chrome/browser/BUILD.gn [modify] https://crrev.com/836222319593ae0e12107a5723d82e0d724c9b4a/chrome/browser/android/DEPS [add] https://crrev.com/836222319593ae0e12107a5723d82e0d724c9b4a/chrome/browser/android/photo_picker_sandbox_bridge.cc [modify] https://crrev.com/836222319593ae0e12107a5723d82e0d724c9b4a/tools/metrics/histograms/histograms.xml
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec87aec251d5a4cf948c827fda3fe99f15b3275b commit ec87aec251d5a4cf948c827fda3fe99f15b3275b Author: Ojan Vafai <ojan@chromium.org> Date: Fri Jul 21 17:01:00 2017 Revert "Enable the seccomp-bpf sandbox for the Android photo picker" This reverts commit 836222319593ae0e12107a5723d82e0d724c9b4a. Reason for revert: Caused test failures on https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/44157 org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testSingleSelectionPhoto org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testMultiSelectionPhoto org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testNoSelection Original change's description: > Enable the seccomp-bpf sandbox for the Android photo picker > > This CL attempts to enable the seccomp-bpf sandbox, as we do for > renderer processes on Android, for the new photo picker when it is > supported by the device. > > The implementation loads the native library as if it's a Chrome child > process, and then calls the native InitializePhotoPickerSandbox function > to initialize the sandbox when available. UMA is logged. > > Because we end up loading the native library, this does create for an > additional delay of ~700ms before the first photo is shown. There may be > optimization opportunities by separating out the sandbox code in a separate > shared library, but this is made difficult by (a) our build system using > the crazy linker, and (b) the dependencies of //sandbox on //base. > > I've verified that the sandbox is enabled on two devices running > different versions of Android, both by printing the status of the > SeccompStarterAndroid, and through the following command: > > $ adb shell cat /proc/18424/status | grep Seccomp > Seccomp: 2 > > (Where "2" means that seccomp-bpf is enabled.) > > BUG= 730066 > > Change-Id: I46e608bad8f69d3cf862c0953361c50f4c65c45c > Reviewed-on: https://chromium-review.googlesource.com/577853 > Reviewed-by: David Trainor <dtrainor@chromium.org> > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> > Reviewed-by: Bo Liu <boliu@chromium.org> > Reviewed-by: Ilya Sherman <isherman@chromium.org> > Commit-Queue: Peter Beverloo <peter@chromium.org> > Cr-Commit-Position: refs/heads/master@{#488643} TBR=jorgelo@chromium.org,peter@chromium.org,isherman@chromium.org,boliu@chromium.org,dtrainor@chromium.org,twellington@chromium.org Change-Id: I2b0d24e90ce50bf2bc913cc7219f48475a34bbda No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 730066 Reviewed-on: https://chromium-review.googlesource.com/581628 Reviewed-by: Ojan Vafai <ojan@chromium.org> Commit-Queue: Ojan Vafai <ojan@chromium.org> Cr-Commit-Position: refs/heads/master@{#488686} [modify] https://crrev.com/ec87aec251d5a4cf948c827fda3fe99f15b3275b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java [modify] https://crrev.com/ec87aec251d5a4cf948c827fda3fe99f15b3275b/chrome/browser/BUILD.gn [modify] https://crrev.com/ec87aec251d5a4cf948c827fda3fe99f15b3275b/chrome/browser/android/DEPS [delete] https://crrev.com/e24f41667474d388ad8ace016323db825d4d89df/chrome/browser/android/photo_picker_sandbox_bridge.cc [modify] https://crrev.com/ec87aec251d5a4cf948c827fda3fe99f15b3275b/tools/metrics/histograms/histograms.xml
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc commit 91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc Author: Peter Beverloo <peter@chromium.org> Date: Thu Aug 31 10:50:51 2017 Enable the seccomp-bpf sandbox for the Android photo picker This CL attempts to enable the seccomp-bpf sandbox, as we do for renderer processes on Android, for the new photo picker when it is supported by the device. The implementation loads the native library as if it's a Chrome child process, and then calls the native InitializePhotoPickerSandbox function to initialize the sandbox when available. UMA is logged. Because we end up loading the native library, this does create for an additional delay of ~700ms before the first photo is shown. There may be optimization opportunities by separating out the sandbox code in a separate shared library, but this is made difficult by (a) our build system using the crazy linker, and (b) the dependencies of //sandbox on //base. I've verified that the sandbox is enabled on two devices running different versions of Android, both by printing the status of the SeccompStarterAndroid, and through the following command: $ adb shell cat /proc/18424/status | grep Seccomp Seccomp: 2 (Where "2" means that seccomp-bpf is enabled.) This is identical to the previous CL which was approved by dtrainor, jorgelo, bolui and isherman, modulo some testing additions kindly written by finnur. See: https://chromium-review.googlesource.com/577853 TBR=dtrainor, jorgelo, bolui and isherman BUG= 730066 Change-Id: I39b836c9ad38c57da850c5052c9db20477a5a56c Reviewed-on: https://chromium-review.googlesource.com/629078 Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> Commit-Queue: Finnur Thorarinsson <finnur@chromium.org> Cr-Commit-Position: refs/heads/master@{#498810} [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/android/javatests/src/org/chromium/chrome/browser/photo_picker/PhotoPickerDialogTest.java [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/browser/BUILD.gn [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/browser/android/DEPS [add] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/chrome/browser/android/photo_picker_sandbox_bridge.cc [modify] https://crrev.com/91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc/tools/metrics/histograms/histograms.xml
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9 commit c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9 Author: Henrik Boström <hbos@chromium.org> Date: Thu Aug 31 14:06:05 2017 Revert "Enable the seccomp-bpf sandbox for the Android photo picker" This reverts commit 91dadd7b3dd95102b50f0ceeabb3678ff4cb42fc. Reason for revert: Causes Android Tests (dbg) failures: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/45248 Original change's description: > Enable the seccomp-bpf sandbox for the Android photo picker > > This CL attempts to enable the seccomp-bpf sandbox, as we do for > renderer processes on Android, for the new photo picker when it is > supported by the device. > > The implementation loads the native library as if it's a Chrome child > process, and then calls the native InitializePhotoPickerSandbox function > to initialize the sandbox when available. UMA is logged. > > Because we end up loading the native library, this does create for an > additional delay of ~700ms before the first photo is shown. There may be > optimization opportunities by separating out the sandbox code in a separate > shared library, but this is made difficult by (a) our build system using > the crazy linker, and (b) the dependencies of //sandbox on //base. > > I've verified that the sandbox is enabled on two devices running > different versions of Android, both by printing the status of the > SeccompStarterAndroid, and through the following command: > > $ adb shell cat /proc/18424/status | grep Seccomp > Seccomp: 2 > > (Where "2" means that seccomp-bpf is enabled.) > > This is identical to the previous CL which was approved by > dtrainor, jorgelo, bolui and isherman, modulo some testing additions > kindly written by finnur. See: > > https://chromium-review.googlesource.com/577853 > > TBR=dtrainor, jorgelo, bolui and isherman > BUG= 730066 > > Change-Id: I39b836c9ad38c57da850c5052c9db20477a5a56c > Reviewed-on: https://chromium-review.googlesource.com/629078 > Reviewed-by: Peter Beverloo <peter@chromium.org> > Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> > Commit-Queue: Finnur Thorarinsson <finnur@chromium.org> > Cr-Commit-Position: refs/heads/master@{#498810} TBR=peter@chromium.org,finnur@chromium.org Change-Id: I4edd0888d888eb5bb73f30bc7ba60fdfdce77ba5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 730066 Reviewed-on: https://chromium-review.googlesource.com/645948 Reviewed-by: Henrik Boström <hbos@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#498854} [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/chrome/android/javatests/src/org/chromium/chrome/browser/photo_picker/PhotoPickerDialogTest.java [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/chrome/browser/BUILD.gn [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/chrome/browser/android/DEPS [delete] https://crrev.com/41df646478cc77acee7e2ee53730c51f992c69c9/chrome/browser/android/photo_picker_sandbox_bridge.cc [modify] https://crrev.com/c25e0826ce5f351cb6d0f3e43209ba0e8c8814c9/tools/metrics/histograms/histograms.xml
,
Aug 31 2017
Issue 760991 has been merged into this issue.
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0e3f840521678a541904f2cc1cdaedaeb617a86 commit c0e3f840521678a541904f2cc1cdaedaeb617a86 Author: Peter Beverloo <peter@chromium.org> Date: Thu Aug 31 16:08:30 2017 Enable the seccomp-bpf sandbox for the Android photo picker (With an increased timeout. We know this to be slow on *one* particular bot, but cannot reproduce this either locally or with trybots. Sorry for the noise.) This CL attempts to enable the seccomp-bpf sandbox, as we do for renderer processes on Android, for the new photo picker when it is supported by the device. The implementation loads the native library as if it's a Chrome child process, and then calls the native InitializePhotoPickerSandbox function to initialize the sandbox when available. UMA is logged. Because we end up loading the native library, this does create for an additional delay of ~700ms before the first photo is shown. There may be optimization opportunities by separating out the sandbox code in a separate shared library, but this is made difficult by (a) our build system using the crazy linker, and (b) the dependencies of //sandbox on //base. I've verified that the sandbox is enabled on two devices running different versions of Android, both by printing the status of the SeccompStarterAndroid, and through the following command: $ adb shell cat /proc/18424/status | grep Seccomp Seccomp: 2 (Where "2" means that seccomp-bpf is enabled.) This is identical to the previous CL which was approved by dtrainor, jorgelo, bolui and isherman, modulo some testing additions kindly written by finnur. See: https://chromium-review.googlesource.com/577853 TBR=dtrainor, jorgelo, bolui and isherman BUG= 730066 Change-Id: I5225512ced2be277a821a87f262f3f3179ab59b2 Reviewed-on: https://chromium-review.googlesource.com/646167 Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> Commit-Queue: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#498877} [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/android/javatests/src/org/chromium/chrome/browser/photo_picker/PhotoPickerDialogTest.java [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/browser/BUILD.gn [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/browser/android/DEPS [add] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/chrome/browser/android/photo_picker_sandbox_bridge.cc [modify] https://crrev.com/c0e3f840521678a541904f2cc1cdaedaeb617a86/tools/metrics/histograms/histograms.xml
,
Jan 9 2018
This is done.
,
Jan 9 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by owe...@chromium.org
, Jun 30 2017Owner: rsesek@chromium.org