diff options
author | Dianne Hackborn <hackbod@google.com> | 2013-09-11 09:51:14 -0700 |
---|---|---|
committer | Dianne Hackborn <hackbod@google.com> | 2013-09-11 09:59:47 -0700 |
commit | 95d785346b4dae808a2d8f77356175e55a572d96 (patch) | |
tree | 821f4566c90d25c624d6c85539bfbb6a0142754c | |
parent | ec7a6ea84578be91e04a54331ea4ca63e7fb69e1 (diff) | |
download | frameworks_base-95d785346b4dae808a2d8f77356175e55a572d96.zip frameworks_base-95d785346b4dae808a2d8f77356175e55a572d96.tar.gz frameworks_base-95d785346b4dae808a2d8f77356175e55a572d96.tar.bz2 |
Fix issue #10688644: Java crash in com.android.phone:
java.lang.SecurityException: Operation not allowed
There was a situation I wasn't taking into account -- components
declared by the system has a special ability to run in the processes
of other uids. This means that if that code loaded into another
process tries to do anything needing an app op verification, it will
fail, because it will say it is calling as the system package name but
it is not actually coming from the system uid.
To fix this, we add a new Context.getOpPackageName() to go along-side
getBasePackageName(). This is a special call for use by all app ops
verification, which will be initialized with either the base package
name, the actual package name, or now the default package name of the
process if we are creating a context for system code being loaded into
a non-system process.
I had to update all of the code doing app ops checks to switch to this
method to get the calling package name.
Also improve the security exception throw to have a more descriptive
error message.
Change-Id: Ic04f77b3938585b02fccabbc12d2f0dc62b9ef25
-rw-r--r-- | core/java/android/app/AppOpsManager.java | 16 | ||||
-rw-r--r-- | core/java/android/app/ApplicationPackageManager.java | 2 | ||||
-rw-r--r-- | core/java/android/app/ContextImpl.java | 25 | ||||
-rw-r--r-- | core/java/android/app/NotificationManager.java | 4 | ||||
-rw-r--r-- | core/java/android/content/ClipboardManager.java | 12 | ||||
-rw-r--r-- | core/java/android/content/ContentResolver.java | 2 | ||||
-rw-r--r-- | core/java/android/content/Context.java | 7 | ||||
-rw-r--r-- | core/java/android/content/ContextWrapper.java | 6 | ||||
-rw-r--r-- | core/java/android/os/PowerManager.java | 2 | ||||
-rw-r--r-- | core/java/android/os/SystemVibrator.java | 2 | ||||
-rw-r--r-- | media/java/android/media/AudioManager.java | 26 | ||||
-rw-r--r-- | policy/src/com/android/internal/policy/impl/PhoneWindowManager.java | 4 | ||||
-rw-r--r-- | telephony/java/android/telephony/TelephonyManager.java | 2 | ||||
-rw-r--r-- | test-runner/src/android/test/mock/MockContext.java | 6 | ||||
-rw-r--r-- | tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java | 6 | ||||
-rw-r--r-- | wifi/java/android/net/wifi/WifiManager.java | 4 |
16 files changed, 89 insertions, 37 deletions
diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index 64054c5..3e4795c 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -575,6 +575,10 @@ public class AppOpsManager { } } + private String buildSecurityExceptionMsg(int op, int uid, String packageName) { + return packageName + " from uid " + uid + " not allowed to perform " + sOpNames[op]; + } + /** * Do a quick check for whether an application might be able to perform an operation. * This is <em>not</em> a security check; you must use {@link #noteOp(int, int, String)} @@ -595,7 +599,7 @@ public class AppOpsManager { try { int mode = mService.checkOperation(op, uid, packageName); if (mode == MODE_ERRORED) { - throw new SecurityException("Operation not allowed"); + throw new SecurityException(buildSecurityExceptionMsg(op, uid, packageName)); } return mode; } catch (RemoteException e) { @@ -650,7 +654,7 @@ public class AppOpsManager { try { int mode = mService.noteOperation(op, uid, packageName); if (mode == MODE_ERRORED) { - throw new SecurityException("Operation not allowed"); + throw new SecurityException(buildSecurityExceptionMsg(op, uid, packageName)); } return mode; } catch (RemoteException e) { @@ -672,7 +676,7 @@ public class AppOpsManager { /** @hide */ public int noteOp(int op) { - return noteOp(op, Process.myUid(), mContext.getBasePackageName()); + return noteOp(op, Process.myUid(), mContext.getOpPackageName()); } /** @hide */ @@ -710,7 +714,7 @@ public class AppOpsManager { try { int mode = mService.startOperation(getToken(mService), op, uid, packageName); if (mode == MODE_ERRORED) { - throw new SecurityException("Operation not allowed"); + throw new SecurityException(buildSecurityExceptionMsg(op, uid, packageName)); } return mode; } catch (RemoteException e) { @@ -732,7 +736,7 @@ public class AppOpsManager { /** @hide */ public int startOp(int op) { - return startOp(op, Process.myUid(), mContext.getBasePackageName()); + return startOp(op, Process.myUid(), mContext.getOpPackageName()); } /** @@ -749,6 +753,6 @@ public class AppOpsManager { } public void finishOp(int op) { - finishOp(op, Process.myUid(), mContext.getBasePackageName()); + finishOp(op, Process.myUid(), mContext.getOpPackageName()); } } diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index ab2739d..e522b78 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -1275,7 +1275,7 @@ final class ApplicationPackageManager extends PackageManager { int newState, int flags) { try { mPM.setApplicationEnabledSetting(packageName, newState, flags, - mContext.getUserId(), mContext.getBasePackageName()); + mContext.getUserId(), mContext.getOpPackageName()); } catch (RemoteException e) { // Should never happen! } diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 7ff7562..fe8c506 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -183,6 +183,7 @@ class ContextImpl extends Context { /*package*/ LoadedApk mPackageInfo; private String mBasePackageName; + private String mOpPackageName; private Resources mResources; /*package*/ ActivityThread mMainThread; private Context mOuterContext; @@ -679,6 +680,12 @@ class ContextImpl extends Context { return mBasePackageName != null ? mBasePackageName : getPackageName(); } + /** @hide */ + @Override + public String getOpPackageName() { + return mOpPackageName != null ? mOpPackageName : getBasePackageName(); + } + @Override public ApplicationInfo getApplicationInfo() { if (mPackageInfo != null) { @@ -1961,6 +1968,7 @@ class ContextImpl extends Context { public ContextImpl(ContextImpl context) { mPackageInfo = context.mPackageInfo; mBasePackageName = context.mBasePackageName; + mOpPackageName = context.mOpPackageName; mResources = context.mResources; mMainThread = context.mMainThread; mContentResolver = context.mContentResolver; @@ -1977,7 +1985,21 @@ class ContextImpl extends Context { final void init(LoadedApk packageInfo, IBinder activityToken, ActivityThread mainThread, Resources container, String basePackageName, UserHandle user) { mPackageInfo = packageInfo; - mBasePackageName = basePackageName != null ? basePackageName : packageInfo.mPackageName; + if (basePackageName != null) { + mBasePackageName = mOpPackageName = basePackageName; + } else { + mBasePackageName = packageInfo.mPackageName; + ApplicationInfo ainfo = packageInfo.getApplicationInfo(); + if (ainfo.uid == Process.SYSTEM_UID && ainfo.uid != Process.myUid()) { + // Special case: system components allow themselves to be loaded in to other + // processes. For purposes of app ops, we must then consider the context as + // belonging to the package of this process, not the system itself, otherwise + // the package+uid verifications in app ops will fail. + mOpPackageName = ActivityThread.currentPackageName(); + } else { + mOpPackageName = mBasePackageName; + } + } mResources = mPackageInfo.getResources(mainThread); mResourcesManager = ResourcesManager.getInstance(); @@ -2011,6 +2033,7 @@ class ContextImpl extends Context { final void init(Resources resources, ActivityThread mainThread, UserHandle user) { mPackageInfo = null; mBasePackageName = null; + mOpPackageName = null; mResources = resources; mMainThread = mainThread; mContentResolver = new ApplicationContentResolver(this, mainThread, user); diff --git a/core/java/android/app/NotificationManager.java b/core/java/android/app/NotificationManager.java index dbafc78..3ee4306 100644 --- a/core/java/android/app/NotificationManager.java +++ b/core/java/android/app/NotificationManager.java @@ -133,7 +133,7 @@ public class NotificationManager } if (localLOGV) Log.v(TAG, pkg + ": notify(" + id + ", " + notification + ")"); try { - service.enqueueNotificationWithTag(pkg, mContext.getBasePackageName(), tag, id, + service.enqueueNotificationWithTag(pkg, mContext.getOpPackageName(), tag, id, notification, idOut, UserHandle.myUserId()); if (id != idOut[0]) { Log.w(TAG, "notify: id corrupted: sent " + id + ", got back " + idOut[0]); @@ -158,7 +158,7 @@ public class NotificationManager } if (localLOGV) Log.v(TAG, pkg + ": notify(" + id + ", " + notification + ")"); try { - service.enqueueNotificationWithTag(pkg, mContext.getBasePackageName(), tag, id, + service.enqueueNotificationWithTag(pkg, mContext.getOpPackageName(), tag, id, notification, idOut, user.getIdentifier()); if (id != idOut[0]) { Log.w(TAG, "notify: id corrupted: sent " + id + ", got back " + idOut[0]); diff --git a/core/java/android/content/ClipboardManager.java b/core/java/android/content/ClipboardManager.java index 69f9d4a..73e6fd0 100644 --- a/core/java/android/content/ClipboardManager.java +++ b/core/java/android/content/ClipboardManager.java @@ -122,7 +122,7 @@ public class ClipboardManager extends android.text.ClipboardManager { if (clip != null) { clip.prepareToLeaveProcess(); } - getService().setPrimaryClip(clip, mContext.getBasePackageName()); + getService().setPrimaryClip(clip, mContext.getOpPackageName()); } catch (RemoteException e) { } } @@ -132,7 +132,7 @@ public class ClipboardManager extends android.text.ClipboardManager { */ public ClipData getPrimaryClip() { try { - return getService().getPrimaryClip(mContext.getBasePackageName()); + return getService().getPrimaryClip(mContext.getOpPackageName()); } catch (RemoteException e) { return null; } @@ -144,7 +144,7 @@ public class ClipboardManager extends android.text.ClipboardManager { */ public ClipDescription getPrimaryClipDescription() { try { - return getService().getPrimaryClipDescription(mContext.getBasePackageName()); + return getService().getPrimaryClipDescription(mContext.getOpPackageName()); } catch (RemoteException e) { return null; } @@ -155,7 +155,7 @@ public class ClipboardManager extends android.text.ClipboardManager { */ public boolean hasPrimaryClip() { try { - return getService().hasPrimaryClip(mContext.getBasePackageName()); + return getService().hasPrimaryClip(mContext.getOpPackageName()); } catch (RemoteException e) { return false; } @@ -166,7 +166,7 @@ public class ClipboardManager extends android.text.ClipboardManager { if (mPrimaryClipChangedListeners.size() == 0) { try { getService().addPrimaryClipChangedListener( - mPrimaryClipChangedServiceListener, mContext.getBasePackageName()); + mPrimaryClipChangedServiceListener, mContext.getOpPackageName()); } catch (RemoteException e) { } } @@ -213,7 +213,7 @@ public class ClipboardManager extends android.text.ClipboardManager { */ public boolean hasText() { try { - return getService().hasClipboardText(mContext.getBasePackageName()); + return getService().hasClipboardText(mContext.getOpPackageName()); } catch (RemoteException e) { return false; } diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index e914604..995ca97 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -261,7 +261,7 @@ public abstract class ContentResolver { public ContentResolver(Context context) { mContext = context != null ? context : ActivityThread.currentApplication(); - mPackageName = mContext.getBasePackageName(); + mPackageName = mContext.getOpPackageName(); } /** @hide */ diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index 8df5bee..7b15e63 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -435,6 +435,13 @@ public abstract class Context { /** @hide Return the name of the base context this context is derived from. */ public abstract String getBasePackageName(); + /** @hide Return the package name that should be used for app ops calls from + * this context. This is the same as {@link #getBasePackageName()} except in + * cases where system components are loaded into other app processes, in which + * case this will be the name of the primary package in that process (so that app + * ops uid verification will work with the name). */ + public abstract String getOpPackageName(); + /** Return the full application info for this context's package. */ public abstract ApplicationInfo getApplicationInfo(); diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index e09d367..a708dad 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -141,6 +141,12 @@ public class ContextWrapper extends Context { return mBase.getBasePackageName(); } + /** @hide */ + @Override + public String getOpPackageName() { + return mBase.getOpPackageName(); + } + @Override public ApplicationInfo getApplicationInfo() { return mBase.getApplicationInfo(); diff --git a/core/java/android/os/PowerManager.java b/core/java/android/os/PowerManager.java index 52e5f38..5e0d489 100644 --- a/core/java/android/os/PowerManager.java +++ b/core/java/android/os/PowerManager.java @@ -407,7 +407,7 @@ public final class PowerManager { */ public WakeLock newWakeLock(int levelAndFlags, String tag) { validateWakeLockParameters(levelAndFlags, tag); - return new WakeLock(levelAndFlags, tag, mContext.getBasePackageName()); + return new WakeLock(levelAndFlags, tag, mContext.getOpPackageName()); } /** @hide */ diff --git a/core/java/android/os/SystemVibrator.java b/core/java/android/os/SystemVibrator.java index e66fb28..700f80d 100644 --- a/core/java/android/os/SystemVibrator.java +++ b/core/java/android/os/SystemVibrator.java @@ -39,7 +39,7 @@ public class SystemVibrator extends Vibrator { } public SystemVibrator(Context context) { - mPackageName = context.getBasePackageName(); + mPackageName = context.getOpPackageName(); mService = IVibratorService.Stub.asInterface( ServiceManager.getService("vibrator")); } diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java index 1dcf0e9..d1f7156 100644 --- a/media/java/android/media/AudioManager.java +++ b/media/java/android/media/AudioManager.java @@ -551,10 +551,10 @@ public class AudioManager { IAudioService service = getService(); try { if (mUseMasterVolume) { - service.adjustMasterVolume(direction, flags, mContext.getBasePackageName()); + service.adjustMasterVolume(direction, flags, mContext.getOpPackageName()); } else { service.adjustStreamVolume(streamType, direction, flags, - mContext.getBasePackageName()); + mContext.getOpPackageName()); } } catch (RemoteException e) { Log.e(TAG, "Dead object in adjustStreamVolume", e); @@ -582,9 +582,9 @@ public class AudioManager { IAudioService service = getService(); try { if (mUseMasterVolume) { - service.adjustMasterVolume(direction, flags, mContext.getBasePackageName()); + service.adjustMasterVolume(direction, flags, mContext.getOpPackageName()); } else { - service.adjustVolume(direction, flags, mContext.getBasePackageName()); + service.adjustVolume(direction, flags, mContext.getOpPackageName()); } } catch (RemoteException e) { Log.e(TAG, "Dead object in adjustVolume", e); @@ -612,10 +612,10 @@ public class AudioManager { IAudioService service = getService(); try { if (mUseMasterVolume) { - service.adjustMasterVolume(direction, flags, mContext.getBasePackageName()); + service.adjustMasterVolume(direction, flags, mContext.getOpPackageName()); } else { service.adjustSuggestedStreamVolume(direction, suggestedStreamType, flags, - mContext.getBasePackageName()); + mContext.getOpPackageName()); } } catch (RemoteException e) { Log.e(TAG, "Dead object in adjustSuggestedStreamVolume", e); @@ -634,7 +634,7 @@ public class AudioManager { public void adjustMasterVolume(int steps, int flags) { IAudioService service = getService(); try { - service.adjustMasterVolume(steps, flags, mContext.getBasePackageName()); + service.adjustMasterVolume(steps, flags, mContext.getOpPackageName()); } catch (RemoteException e) { Log.e(TAG, "Dead object in adjustMasterVolume", e); } @@ -786,9 +786,9 @@ public class AudioManager { IAudioService service = getService(); try { if (mUseMasterVolume) { - service.setMasterVolume(index, flags, mContext.getBasePackageName()); + service.setMasterVolume(index, flags, mContext.getOpPackageName()); } else { - service.setStreamVolume(streamType, index, flags, mContext.getBasePackageName()); + service.setStreamVolume(streamType, index, flags, mContext.getOpPackageName()); } } catch (RemoteException e) { Log.e(TAG, "Dead object in setStreamVolume", e); @@ -854,7 +854,7 @@ public class AudioManager { public void setMasterVolume(int index, int flags) { IAudioService service = getService(); try { - service.setMasterVolume(index, flags, mContext.getBasePackageName()); + service.setMasterVolume(index, flags, mContext.getOpPackageName()); } catch (RemoteException e) { Log.e(TAG, "Dead object in setMasterVolume", e); } @@ -1569,7 +1569,7 @@ public class AudioManager { IAudioService service = getService(); try { service.adjustLocalOrRemoteStreamVolume(streamType, direction, - mContext.getBasePackageName()); + mContext.getOpPackageName()); } catch (RemoteException e) { Log.e(TAG, "Dead object in adjustLocalOrRemoteStreamVolume", e); } @@ -1996,7 +1996,7 @@ public class AudioManager { try { status = service.requestAudioFocus(streamType, durationHint, mICallBack, mAudioFocusDispatcher, getIdForAudioFocusListener(l), - mContext.getBasePackageName() /* package name */); + mContext.getOpPackageName() /* package name */); } catch (RemoteException e) { Log.e(TAG, "Can't call requestAudioFocus() on AudioService due to "+e); } @@ -2018,7 +2018,7 @@ public class AudioManager { try { service.requestAudioFocus(streamType, durationHint, mICallBack, null, MediaFocusControl.IN_VOICE_COMM_FOCUS_ID, - mContext.getBasePackageName()); + mContext.getOpPackageName()); } catch (RemoteException e) { Log.e(TAG, "Can't call requestAudioFocusForCall() on AudioService due to "+e); } diff --git a/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java b/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java index 27bf38cc..8a285e3 100644 --- a/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java +++ b/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java @@ -3637,7 +3637,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { ? AudioManager.ADJUST_RAISE : AudioManager.ADJUST_LOWER, 0, - mContext.getBasePackageName()); + mContext.getOpPackageName()); } catch (RemoteException e) { Log.w(TAG, "IAudioService.adjustStreamVolume() threw RemoteException " + e); } finally { @@ -4964,7 +4964,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { owningPackage = win.getOwningPackage(); } else { owningUid = android.os.Process.myUid(); - owningPackage = mContext.getBasePackageName(); + owningPackage = mContext.getOpPackageName(); } if (pattern.length == 1) { // One-shot vibration diff --git a/telephony/java/android/telephony/TelephonyManager.java b/telephony/java/android/telephony/TelephonyManager.java index 40201ad..7d8b64f 100644 --- a/telephony/java/android/telephony/TelephonyManager.java +++ b/telephony/java/android/telephony/TelephonyManager.java @@ -309,7 +309,7 @@ public class TelephonyManager { */ public List<NeighboringCellInfo> getNeighboringCellInfo() { try { - return getITelephony().getNeighboringCellInfo(mContext.getBasePackageName()); + return getITelephony().getNeighboringCellInfo(mContext.getOpPackageName()); } catch (RemoteException ex) { return null; } catch (NullPointerException ex) { diff --git a/test-runner/src/android/test/mock/MockContext.java b/test-runner/src/android/test/mock/MockContext.java index 252a14e..0d9cd18 100644 --- a/test-runner/src/android/test/mock/MockContext.java +++ b/test-runner/src/android/test/mock/MockContext.java @@ -112,6 +112,12 @@ public class MockContext extends Context { throw new UnsupportedOperationException(); } + /** @hide */ + @Override + public String getOpPackageName() { + throw new UnsupportedOperationException(); + } + @Override public ApplicationInfo getApplicationInfo() { throw new UnsupportedOperationException(); diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java index a1f2697..b9294ab 100644 --- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java +++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java @@ -1090,6 +1090,12 @@ public final class BridgeContext extends Context { } @Override + public String getOpPackageName() { + // pass + return null; + } + + @Override public ApplicationInfo getApplicationInfo() { return mApplicationInfo; } diff --git a/wifi/java/android/net/wifi/WifiManager.java b/wifi/java/android/net/wifi/WifiManager.java index 5f5d54f..2a5b4da 100644 --- a/wifi/java/android/net/wifi/WifiManager.java +++ b/wifi/java/android/net/wifi/WifiManager.java @@ -833,7 +833,7 @@ public class WifiManager { */ public List<BatchedScanResult> getBatchedScanResults() { try { - return mService.getBatchedScanResults(mContext.getBasePackageName()); + return mService.getBatchedScanResults(mContext.getOpPackageName()); } catch (RemoteException e) { return null; } @@ -883,7 +883,7 @@ public class WifiManager { */ public List<ScanResult> getScanResults() { try { - return mService.getScanResults(mContext.getBasePackageName()); + return mService.getScanResults(mContext.getOpPackageName()); } catch (RemoteException e) { return null; } |