Refactor canAccessScanResult

WifiPermissionUtil.canAccessScanResults is now void. Where it would
return false before it now throws a SecurityException.

Also fixed an existing test that is passing for the wrong reason.
The test should be checking for a failure, but the CAN_READ_PEER_MAC_ADDRESS
permission made it pass.

Bug: 73160471
Test: compile, run ./frameworks/opt/net/wifi/tests/wifitests/runtests.sh
Change-Id: If32a546ee646e6c8e786a23c017c86f33be7e304

4 files changed, 19 insertions, 28 deletions

diff --git a/service/java/com/android/server/wifi/ScoredNetworkEvaluator.java b/service/java/com/android/server/wifi/ScoredNetworkEvaluator.java
index 223423eb2..9bb764ea6 100644
--- a/service/java/com/android/server/wifi/ScoredNetworkEvaluator.java
+++ b/service/java/com/android/server/wifi/ScoredNetworkEvaluator.java
@@ -114,13 +114,12 @@ public class ScoredNetworkEvaluator implements WifiNetworkSelector.NetworkEvalua
         String packageName = mNetworkScoreManager.getActiveScorerPackage();
         if (networkScorerAppData == null || packageName == null) return false;
         int uid = networkScorerAppData.packageUid;
-        boolean allow;
         try {
-            allow = mWifiPermissionsUtil.canAccessScanResults(packageName, uid);
+            mWifiPermissionsUtil.enforceCanAccessScanResults(packageName, uid);
+            return true;
         } catch (SecurityException e) {
-            allow = false;
+            return false;
         }
-        return allow;
     }
 
     @Override
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 43c549388..65d16c603 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -1942,13 +1942,13 @@ public class WifiServiceImpl extends IWifiManager.Stub {
                         == PackageManager.PERMISSION_GRANTED) {
                     hideDefaultMacAddress = false;
                 }
-                if (mWifiPermissionsUtil.canAccessScanResults(callingPackage, uid)) {
-                    hideBssidAndSsid = false;
-                }
+                mWifiPermissionsUtil.enforceCanAccessScanResults(callingPackage, uid);
+                hideBssidAndSsid = false;
             } catch (RemoteException e) {
                 Log.e(TAG, "Error checking receiver permission", e);
             } catch (SecurityException e) {
-                Log.e(TAG, "Security exception checking receiver permission", e);
+                Log.e(TAG, "Security exception checking receiver permission"
+                        + ", hiding ssid and bssid", e);
             }
             if (hideDefaultMacAddress) {
                 result.setMacAddress(WifiInfo.DEFAULT_MAC_ADDRESS);
@@ -1974,9 +1974,7 @@ public class WifiServiceImpl extends IWifiManager.Stub {
         int uid = Binder.getCallingUid();
         long ident = Binder.clearCallingIdentity();
         try {
-            if (!mWifiPermissionsUtil.canAccessScanResults(callingPackage, uid)) {
-                return new ArrayList<ScanResult>();
-            }
+            mWifiPermissionsUtil.enforceCanAccessScanResults(callingPackage, uid);
             final List<ScanResult> scanResults = new ArrayList<>();
             boolean success = mWifiInjector.getWifiStateMachineHandler().runWithScissors(() -> {
                 scanResults.addAll(mScanRequestProxy.getScanResults());
@@ -1985,6 +1983,8 @@ public class WifiServiceImpl extends IWifiManager.Stub {
                 Log.e(TAG, "Failed to post runnable to fetch scan results");
             }
             return scanResults;
+        } catch (SecurityException e) {
+            return new ArrayList<ScanResult>();
         } finally {
             Binder.restoreCallingIdentity(ident);
         }
diff --git a/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java b/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java
index b525555a0..fdad6574e 100644
--- a/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java
+++ b/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java
@@ -3459,7 +3459,6 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub {
          */
         private WifiP2pDeviceList getPeers(Bundle pkg, int uid) {
             String pkgName = pkg.getString(WifiP2pManager.CALLING_PACKAGE);
-            boolean scanPermission = false;
             WifiPermissionsUtil wifiPermissionsUtil;
             // getPeers() is guaranteed to be invoked after Wifi Service is up
             // This ensures getInstance() will return a non-null object now
@@ -3468,13 +3467,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub {
             }
             wifiPermissionsUtil = mWifiInjector.getWifiPermissionsUtil();
             try {
-                scanPermission = wifiPermissionsUtil.canAccessScanResults(pkgName, uid);
-            } catch (SecurityException e) {
-                Log.e(TAG, "Security Exception, cannot access peer list");
-            }
-            if (scanPermission) {
+                wifiPermissionsUtil.enforceCanAccessScanResults(pkgName, uid);
                 return new WifiP2pDeviceList(mPeers);
-            } else {
+            } catch (SecurityException e) {
+                Log.v(TAG, "Security Exception, cannot access peer list");
                 return new WifiP2pDeviceList();
             }
         }
diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
index 0f333d498..3d838645d 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
@@ -166,12 +166,12 @@ public class WifiPermissionsUtil {
     }
 
     /**
-     * API to determine if the caller has permissions to get scan results.
+     * API to determine if the caller has permissions to get scan results. Throws SecurityException
+     * if the caller has no permission.
      * @param pkgName package name of the application requesting access
      * @param uid The uid of the package
-     * @return boolean true or false if permissions is granted
      */
-    public boolean canAccessScanResults(String pkgName, int uid) throws SecurityException {
+    public void enforceCanAccessScanResults(String pkgName, int uid) throws SecurityException {
         mAppOps.checkPackage(uid, pkgName);
         // Check if the calling Uid has CAN_READ_PEER_MAC_ADDRESS permission.
         boolean canCallingUidAccessLocation = checkCallerHasPeersMacAddressPermission(uid);
@@ -192,22 +192,18 @@ public class WifiPermissionsUtil {
         if (!canCallingUidAccessLocation && !canAppPackageUseLocation) {
             // also check if it is a connectivity app
             if (!appTypeConnectivity) {
-                mLog.tC("Denied: no location permission");
-                return false;
+                throw new SecurityException("UID " + uid + " has no location permission");
             }
         }
         // Check if Wifi Scan request is an operation allowed for this App.
         if (!isScanAllowedbyApps(pkgName, uid)) {
-            mLog.tC("Denied: app wifi scan not allowed");
-            return false;
+            throw new SecurityException("UID " + uid + " has no wifi scan permission");
         }
         // If the User or profile is current, permission is granted
         // Otherwise, uid must have INTERACT_ACROSS_USERS_FULL permission.
         if (!isCurrentProfile(uid) && !checkInteractAcrossUsersFull(uid)) {
-            mLog.tC("Denied: Profile not permitted");
-            return false;
+            throw new SecurityException("UID " + uid + " profile not permitted");
         }
-        return true;
     }
 
     /**