From f4394e83f844265399148e827c8c1930e64eba08 Mon Sep 17 00:00:00 2001
From: markchien <markchien@google.com>
Date: Wed, 20 May 2020 11:44:05 +0800
Subject: [PATCH] Fix TetheringServiceTest test WRITE_SETTINGS permission
 failure

AdoptShellPermissionIdentity can not pass permission check by
Settings#checkAndNoteWriteSettingsOperation. It would compare the caller
uid and its package name. See error below:
1. java.lang.SecurityException:
Specified package com.android.shell under uid 10239 but it is really 2000
2. java.lang.SecurityException:
uid 10245 does not have android.permission.UPDATE_APP_OPS_STATS.

Override the method and test if caller hold WRITE_SETTINGS directly.

Bug: 154869719
Test: TetheringTests, TetheringCoverageTests, NetworkStackNextTests,
NetworkStackCoverageTests

Change-Id: I2a60c4d66ef30028f9663159f85464ea815248e2
---
 .../networkstack/tethering/TetheringService.java |  7 ++++---
 .../tethering/MockTetheringService.java          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringService.java b/Tethering/src/com/android/networkstack/tethering/TetheringService.java
index 7d01273842..c11e86258d 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringService.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringService.java
@@ -264,10 +264,11 @@ public class TetheringService extends Service {
             if (onlyAllowPrivileged || mTethering.isTetherProvisioningRequired()) return false;
 
             int uid = Binder.getCallingUid();
+
             // If callerPkg's uid is not same as Binder.getCallingUid(),
             // checkAndNoteWriteSettingsOperation will return false and the operation will be
             // denied.
-            return TetheringService.checkAndNoteWriteSettingsOperation(mService, uid, callerPkg,
+            return mService.checkAndNoteWriteSettingsOperation(mService, uid, callerPkg,
                     callingAttributionTag, false /* throwException */);
         }
 
@@ -285,8 +286,8 @@ public class TetheringService extends Service {
      *
      * @return {@code true} iff the package is allowed to write settings.
      */
-    // TODO: Remove method and replace with direct call once R code is pushed to AOSP
-    private static boolean checkAndNoteWriteSettingsOperation(@NonNull Context context, int uid,
+    @VisibleForTesting
+    boolean checkAndNoteWriteSettingsOperation(@NonNull Context context, int uid,
             @NonNull String callingPackage, @Nullable String callingAttributionTag,
             boolean throwException) {
         return Settings.checkAndNoteWriteSettingsOperation(context, uid, callingPackage,
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
index 1c81c1247d..071a290e65 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
@@ -15,13 +15,20 @@
  */
 package com.android.networkstack.tethering;
 
+import static android.Manifest.permission.WRITE_SETTINGS;
+import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+
 import static org.mockito.Mockito.mock;
 
+import android.content.Context;
 import android.content.Intent;
 import android.net.ITetheringConnector;
 import android.os.Binder;
 import android.os.IBinder;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
 public class MockTetheringService extends TetheringService {
     private final Tethering mTethering = mock(Tethering.class);
 
@@ -35,6 +42,15 @@ public class MockTetheringService extends TetheringService {
         return mTethering;
     }
 
+    @Override
+    boolean checkAndNoteWriteSettingsOperation(@NonNull Context context, int uid,
+            @NonNull String callingPackage, @Nullable String callingAttributionTag,
+            boolean throwException) {
+        // Test this does not verify the calling package / UID, as calling package could be shell
+        // and not match the UID.
+        return context.checkCallingOrSelfPermission(WRITE_SETTINGS) == PERMISSION_GRANTED;
+    }
+
     public Tethering getTethering() {
         return mTethering;
     }