diff options
author | Ahmed ElArabawy <arabawy@google.com> | 2019-04-24 14:02:27 -0700 |
---|---|---|
committer | Ahmed ElArabawy <arabawy@google.com> | 2019-04-25 21:20:21 +0000 |
commit | c3ecaf2f7e60d3cd558915bf54a10f39151d2558 (patch) | |
tree | ad03ffefe5edae034297bd03b32936a35b4c72b7 /service | |
parent | a7a78153b4fa396c3e735444cdef3bba87aeb4f7 (diff) |
Move LOCK permission check to WifiServiceImpl
In current implementation the permission checks for WifiLock operations
is performed inside the WifiLockManager. However, as we are currently
running this part of the code from the Wifi handler thread, the check is not
effective and always succeeds.
In this commit, the permission checks are moved to the WifiServiceImpl
before switching to the wifi handler thread.
Bug: 130359136
Test: atest com.android.wifi.server
Test: atest android.permission.cts.NoWakeLockPermissionTest#testWifiLockAcquire
Change-Id: If307ebc693e9df67c2075159a3f394835d5b6cf4
Diffstat (limited to 'service')
-rw-r--r-- | service/java/com/android/server/wifi/WifiLockManager.java | 36 | ||||
-rw-r--r-- | service/java/com/android/server/wifi/WifiServiceImpl.java | 9 |
2 files changed, 22 insertions, 23 deletions
diff --git a/service/java/com/android/server/wifi/WifiLockManager.java b/service/java/com/android/server/wifi/WifiLockManager.java index ab0871554..e292a84d8 100644 --- a/service/java/com/android/server/wifi/WifiLockManager.java +++ b/service/java/com/android/server/wifi/WifiLockManager.java @@ -174,8 +174,7 @@ public class WifiLockManager { /** * Method allowing a calling app to acquire a Wifi WakeLock in the supplied mode. * - * This method verifies that the caller has permission to make the call and that the lock mode - * is a valid WifiLock mode. + * This method checks that the lock mode is a valid WifiLock mode. * @param lockMode int representation of the Wifi WakeLock type. * @param tag String passed to WifiManager.WifiLock * @param binder IBinder for the calling app @@ -184,28 +183,25 @@ public class WifiLockManager { * @return true if the lock was successfully acquired, false if the lockMode was invalid. */ public boolean acquireWifiLock(int lockMode, String tag, IBinder binder, WorkSource ws) { - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.WAKE_LOCK, null); if (!isValidLockMode(lockMode)) { throw new IllegalArgumentException("lockMode =" + lockMode); } - if (ws == null || ws.isEmpty()) { - ws = new WorkSource(Binder.getCallingUid()); - } else { - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.UPDATE_DEVICE_STATS, null); - } - return addLock(new WifiLock(lockMode, tag, binder, ws)); + + // Make a copy of the WorkSource before adding it to the WakeLock + // This is to make sure worksource value can not be changed by caller + // after function returns. + WorkSource newWorkSource = new WorkSource(ws); + + return addLock(new WifiLock(lockMode, tag, binder, newWorkSource)); } /** - * Method used by applications to release a WiFi Wake lock. This method checks permissions for - * the caller and if allowed, releases the underlying WifiLock(s). + * Method used by applications to release a WiFi Wake lock. * * @param binder IBinder for the calling app. * @return true if the lock was released, false if the caller did not hold any locks */ public boolean releaseWifiLock(IBinder binder) { - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.WAKE_LOCK, null); return releaseLock(binder); } @@ -261,9 +257,6 @@ public class WifiLockManager { * @param ws WorkSource to add to the existing WifiLock(s). */ public synchronized void updateWifiLockWorkSource(IBinder binder, WorkSource ws) { - // Does the caller have permission to make this call? - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.UPDATE_DEVICE_STATS, null); // Now check if there is an active lock WifiLock wl = findLockByBinder(binder); @@ -271,13 +264,10 @@ public class WifiLockManager { throw new IllegalArgumentException("Wifi lock not active"); } - WorkSource newWorkSource; - if (ws == null || ws.isEmpty()) { - newWorkSource = new WorkSource(Binder.getCallingUid()); - } else { - // Make a copy of the WorkSource before adding it to the WakeLock - newWorkSource = new WorkSource(ws); - } + // Make a copy of the WorkSource before adding it to the WakeLock + // This is to make sure worksource value can not be changed by caller + // after function returns. + WorkSource newWorkSource = new WorkSource(ws); if (mVerboseLoggingEnabled) { Slog.d(TAG, "updateWifiLockWakeSource: " + wl + ", newWorkSource=" + newWorkSource); diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index 2f2eeaf20..2a28be7f2 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -2816,6 +2816,9 @@ public class WifiServiceImpl extends BaseWifiService { .c(Binder.getCallingUid()) .c(lockMode).flush(); + // Check on permission to make this call + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.WAKE_LOCK, null); + // If no UID is provided in worksource, use the calling UID WorkSource updatedWs = (ws == null || ws.isEmpty()) ? new WorkSource(Binder.getCallingUid()) : ws; @@ -2838,6 +2841,10 @@ public class WifiServiceImpl extends BaseWifiService { public void updateWifiLockWorkSource(IBinder binder, WorkSource ws) { mLog.info("updateWifiLockWorkSource uid=%").c(Binder.getCallingUid()).flush(); + // Check on permission to make this call + mContext.enforceCallingOrSelfPermission( + android.Manifest.permission.UPDATE_DEVICE_STATS, null); + // If no UID is provided in worksource, use the calling UID WorkSource updatedWs = (ws == null || ws.isEmpty()) ? new WorkSource(Binder.getCallingUid()) : ws; @@ -2855,6 +2862,8 @@ public class WifiServiceImpl extends BaseWifiService { public boolean releaseWifiLock(IBinder binder) { mLog.info("releaseWifiLock uid=%").c(Binder.getCallingUid()).flush(); + // Check on permission to make this call + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.WAKE_LOCK, null); Mutable<Boolean> lockSuccess = new Mutable<>(); boolean runWithScissorsSuccess = mWifiInjector.getClientModeImplHandler().runWithScissors( () -> { |