New issue
Advanced search Search tips

Issue 832388 link

Starred by 0 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

Remove ChromeMainDelegateAndroid::SecureDataDirectory() after Chrome's min SDK >= 21

Project Member Reported by paulmiller@chromium.org, Apr 13 2018

Issue description

context: https://chromium-review.googlesource.com/c/chromium/src/+/984773/1/chrome/app/android/chrome_main_delegate_android.cc#70

Android >= 21 has Os.chown(), with which we can restrict app_chrome/ to rwx------. We'll do this immediately in PathUtils, rather than waiting for native initialization.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e34d9d20536109581eabd0fe61f8b92ac51497a

commit 5e34d9d20536109581eabd0fe61f8b92ac51497a
Author: Paul Miller <paulmiller@google.com>
Date: Wed Apr 18 20:06:36 2018

Android: Restrict data directory for both Chrome and WebView

Context.getDir() creates Chrome's data directory, app_chrome/, with
rwxrwx--x. ChromeMainDelegateAndroid::RunProcess() then limits this to
rwx------. RunProcess() goes out of its way to avoid granting any user
permissions that weren't already present, but this seems like a mistake;
it shouldn't be possible for app_chrome/ to have fewer permissions than
rwx------. So RunProcess is simplified to set the permissions to exactly
rwx------. Also don't print data_path in the error message because if
PathService::Get() failed, data_path is empty.

Also restrict WebView's directory, app_webview/, using Os.chown(). Doing
this in PathUtils covers both Chrome and WebView. However, Os.chown()
requires API >= 21, which is the case for WebView but not Chrome, so
Chrome's RunProcess() code must stay for now.

Rehabilitate //chrome/test:chrome_app_unittests to run on Android (crbug
609855 says it was broken but it seems to work now) and add a unit test
for the simplified native code.

BUG=832388, 609855 
internal bug b/19993402

Change-Id: I1bcfe72940ddc1fb23f2b0bef50775853843ea76
Reviewed-on: https://chromium-review.googlesource.com/984773
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551795}
[modify] https://crrev.com/5e34d9d20536109581eabd0fe61f8b92ac51497a/base/android/java/src/org/chromium/base/PathUtils.java
[modify] https://crrev.com/5e34d9d20536109581eabd0fe61f8b92ac51497a/chrome/app/android/chrome_main_delegate_android.cc
[modify] https://crrev.com/5e34d9d20536109581eabd0fe61f8b92ac51497a/chrome/app/android/chrome_main_delegate_android.h
[add] https://crrev.com/5e34d9d20536109581eabd0fe61f8b92ac51497a/chrome/app/android/chrome_main_delegate_android_unittest.cc
[modify] https://crrev.com/5e34d9d20536109581eabd0fe61f8b92ac51497a/chrome/test/BUILD.gn

Sign in to add a comment