From 64f20dd1589c10a3da1544344e033ea0c95962ad Mon Sep 17 00:00:00 2001
From: "Philip P. Moltmann" <moltmann@google.com>
Date: Tue, 25 Jun 2019 08:41:51 -0700
Subject: [PATCH] Set LEGACY_STORAGE only from READ_EXTERNAL_STORAGE

Not from WRITE_EXTERNAL_STORAGE. Otherwise it is not clear what happens
if one of the permissions is white-listed and one not. This can lead to
a loop where we try to set LEGACY_STORAGE to two different values.

Fixes: 135763654, 135742960
Bug: 135933014
Test: atest RestrictedPermissionsTest
Change-Id: I35883f12525360fd7f760750505a27644342955c
---
 core/res/AndroidManifest.xml                  |  7 ++++
 core/res/res/values/attrs_manifest.xml        |  2 +
 .../SoftRestrictedPermissionPolicy.java       | 39 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml
index 890ad5e839f7e..6aff8b8f1d5c6 100644
--- a/core/res/AndroidManifest.xml
+++ b/core/res/AndroidManifest.xml
@@ -824,6 +824,11 @@
      grants your app this permission. If you don't need this permission, be sure your <a
      href="{@docRoot}guide/topics/manifest/uses-sdk-element.html#target">{@code
      targetSdkVersion}</a> is 4 or higher.
+     <p>Is this permission is not whitelisted for an app that targets an API level before
+     {@link android.os.Build.VERSION_CODES#Q} this permission cannot be granted to apps.</p>
+     <p>Is this permission is not whitelisted for an app that targets an API level
+     {@link android.os.Build.VERSION_CODES#Q} or later the app will be forced into isolated storage.
+     </p>
      -->
     <permission android:name="android.permission.READ_EXTERNAL_STORAGE"
         android:permissionGroup="android.permission-group.UNDEFINED"
@@ -845,6 +850,8 @@
          read/write files in your application-specific directories returned by
          {@link android.content.Context#getExternalFilesDir} and
          {@link android.content.Context#getExternalCacheDir}.
+         <p>Is this permission is not whitelisted for an app that targets an API level before
+         {@link android.os.Build.VERSION_CODES#Q} this permission cannot be granted to apps.</p>
     -->
     <permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"
         android:permissionGroup="android.permission-group.UNDEFINED"
diff --git a/core/res/res/values/attrs_manifest.xml b/core/res/res/values/attrs_manifest.xml
index 8dfb9694ac339..77fca8fe4d8e1 100644
--- a/core/res/res/values/attrs_manifest.xml
+++ b/core/res/res/values/attrs_manifest.xml
@@ -326,6 +326,8 @@
              grantable in its full form to apps that meet special criteria
              per platform policy. Otherwise, a weaker form of the permission
              would be granted. The weak grant depends on the permission.
+             <p>What weak grant means is described in the documentation of
+             the permissions.
         -->
         <flag name="softRestricted" value="0x8" />
         <!-- This permission is restricted immutably which means that its
diff --git a/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java b/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
index 1658833988976..d53f6854dfdf9 100644
--- a/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
+++ b/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
@@ -91,8 +91,7 @@ public abstract class SoftRestrictedPermissionPolicy {
             // Storage uses a special app op to decide the mount state and supports soft restriction
             // where the restricted state allows the permission but only for accessing the medial
             // collections.
-            case READ_EXTERNAL_STORAGE:
-            case WRITE_EXTERNAL_STORAGE: {
+            case READ_EXTERNAL_STORAGE: {
                 final int flags;
                 final boolean applyRestriction;
                 final boolean isWhiteListed;
@@ -148,6 +147,42 @@ public abstract class SoftRestrictedPermissionPolicy {
                     }
                 };
             }
+            case WRITE_EXTERNAL_STORAGE: {
+                final boolean isWhiteListed;
+                final int targetSDK;
+
+                if (appInfo != null) {
+                    final int flags = context.getPackageManager().getPermissionFlags(permission,
+                            appInfo.packageName, user);
+                    isWhiteListed = (flags & FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) != 0;
+                    targetSDK = appInfo.targetSdkVersion;
+                } else {
+                    isWhiteListed = false;
+                    targetSDK = 0;
+                }
+
+                return new SoftRestrictedPermissionPolicy() {
+                    @Override
+                    public int resolveAppOp() {
+                        return OP_NONE;
+                    }
+
+                    @Override
+                    public int getDesiredOpMode() {
+                        return MODE_DEFAULT;
+                    }
+
+                    @Override
+                    public boolean shouldSetAppOpIfNotDefault() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean canBeGranted() {
+                        return isWhiteListed || targetSDK >= Build.VERSION_CODES.Q;
+                    }
+                };
+            }
             default:
                 return DUMMY_POLICY;
         }