diff options
10 files changed, 220 insertions, 44 deletions
diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index 1ebb551df961..b28c2f462db2 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -967,7 +967,7 @@ public class Binder implements IBinder { * By default, we use the calling uid since we can always trust it. */ private static volatile BinderInternal.WorkSourceProvider sWorkSourceProvider = - Binder::getCallingUid; + (x) -> Binder.getCallingUid(); /** * Sets the work source provider. @@ -991,21 +991,23 @@ public class Binder implements IBinder { // Entry point from android_util_Binder.cpp's onTransact private boolean execTransact(int code, long dataObj, long replyObj, int flags) { - final int workSourceUid = sWorkSourceProvider.resolveWorkSourceUid(); - final long origWorkSource = ThreadLocalWorkSource.setUid(workSourceUid); + // At that point, the parcel request headers haven't been parsed so we do not know what + // WorkSource the caller has set. Use calling uid as the default. + final int callingUid = Binder.getCallingUid(); + final long origWorkSource = ThreadLocalWorkSource.setUid(callingUid); try { - return execTransactInternal(code, dataObj, replyObj, flags, workSourceUid); + return execTransactInternal(code, dataObj, replyObj, flags, callingUid); } finally { ThreadLocalWorkSource.restore(origWorkSource); } } - private boolean execTransactInternal(int code, long dataObj, long replyObj, - int flags, int workSourceUid) { + private boolean execTransactInternal(int code, long dataObj, long replyObj, int flags, + int callingUid) { // Make sure the observer won't change while processing a transaction. final BinderInternal.Observer observer = sObserver; final CallSession callSession = - observer != null ? observer.callStarted(this, code, workSourceUid) : null; + observer != null ? observer.callStarted(this, code, UNSET_WORKSOURCE) : null; Parcel data = Parcel.obtain(dataObj); Parcel reply = Parcel.obtain(replyObj); // theoretically, we should call transact, which will call onTransact, @@ -1045,6 +1047,10 @@ public class Binder implements IBinder { Trace.traceEnd(Trace.TRACE_TAG_ALWAYS); } if (observer != null) { + // The parcel RPC headers have been called during onTransact so we can now access + // the worksource uid from the parcel. + final int workSourceUid = sWorkSourceProvider.resolveWorkSourceUid( + data.readCallingWorkSourceUid()); observer.callEnded(callSession, data.dataSize(), reply.dataSize(), workSourceUid); } } diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java index 97d72f01dbb3..97c0a13e7a2b 100644 --- a/core/java/android/os/BinderProxy.java +++ b/core/java/android/os/BinderProxy.java @@ -493,8 +493,17 @@ public final class BinderProxy implements IBinder { // Make sure the listener won't change while processing a transaction. final Binder.ProxyTransactListener transactListener = sTransactListener; Object session = null; + if (transactListener != null) { + final int origWorkSourceUid = Binder.getCallingWorkSourceUid(); session = transactListener.onTransactStarted(this, code); + + // Allow the listener to update the work source uid. We need to update the request + // header if the uid is updated. + final int updatedWorkSourceUid = Binder.getCallingWorkSourceUid(); + if (origWorkSourceUid != updatedWorkSourceUid) { + data.replaceCallingWorkSourceUid(updatedWorkSourceUid); + } } try { diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index b9cdcc096962..79e9ba5f5bcc 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -333,6 +333,12 @@ public final class Parcel { private static native void nativeWriteInterfaceToken(long nativePtr, String interfaceName); private static native void nativeEnforceInterface(long nativePtr, String interfaceName); + @CriticalNative + private static native boolean nativeReplaceCallingWorkSourceUid( + long nativePtr, int workSourceUid); + @CriticalNative + private static native int nativeReadCallingWorkSourceUid(long nativePtr); + /** Last time exception with a stack trace was written */ private static volatile long sLastWriteExceptionStackTrace; /** Used for throttling of writing stack trace, which is costly */ @@ -613,6 +619,35 @@ public final class Parcel { } /** + * Writes the work source uid to the request headers. + * + * <p>It requires the headers to have been written/read already to replace the work source. + * + * @return true if the request headers have been updated. + * + * @hide + */ + public boolean replaceCallingWorkSourceUid(int workSourceUid) { + return nativeReplaceCallingWorkSourceUid(mNativePtr, workSourceUid); + } + + /** + * Reads the work source uid from the request headers. + * + * <p>Unlike other read methods, this method does not read the parcel at the current + * {@link #dataPosition}. It will set the {@link #dataPosition} before the read and restore the + * position after reading the request header. + * + * @return the work source uid or {@link Binder#UNSET_WORKSOURCE} if headers have not been + * written/parsed yet. + * + * @hide + */ + public int readCallingWorkSourceUid() { + return nativeReadCallingWorkSourceUid(mNativePtr); + } + + /** * Write a byte array into the parcel at the current {@link #dataPosition}, * growing {@link #dataCapacity} if needed. * @param b Bytes to place into the parcel. diff --git a/core/java/com/android/internal/os/BinderInternal.java b/core/java/com/android/internal/os/BinderInternal.java index 5b699791ea77..aa4846f6059e 100644 --- a/core/java/com/android/internal/os/BinderInternal.java +++ b/core/java/com/android/internal/os/BinderInternal.java @@ -96,9 +96,10 @@ public class BinderInternal { * <p>The implementation should never execute a binder call since it is called during a * binder transaction. * + * @param untrustedWorkSourceUid The work source set by the caller. * @return the uid of the process to attribute the binder transaction to. */ - int resolveWorkSourceUid(); + int resolveWorkSourceUid(int untrustedWorkSourceUid); } /** diff --git a/core/jni/android_os_Parcel.cpp b/core/jni/android_os_Parcel.cpp index 3b59321024fd..d80c071c3e26 100644 --- a/core/jni/android_os_Parcel.cpp +++ b/core/jni/android_os_Parcel.cpp @@ -670,6 +670,24 @@ static jlong android_os_Parcel_getBlobAshmemSize(jlong nativePtr) return 0; } +static jint android_os_Parcel_readCallingWorkSourceUid(jlong nativePtr) +{ + Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr); + if (parcel != NULL) { + return parcel->readCallingWorkSourceUid(); + } + return IPCThreadState::kUnsetWorkSource; +} + +static jboolean android_os_Parcel_replaceCallingWorkSourceUid(jlong nativePtr, jint uid) +{ + Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr); + if (parcel != NULL) { + return parcel->replaceCallingWorkSourceUid(uid); + } + return false; +} + // ---------------------------------------------------------------------------- static const JNINativeMethod gParcelMethods[] = { @@ -740,6 +758,11 @@ static const JNINativeMethod gParcelMethods[] = { // @CriticalNative {"nativeGetBlobAshmemSize", "(J)J", (void*)android_os_Parcel_getBlobAshmemSize}, + + // @CriticalNative + {"nativeReadCallingWorkSourceUid", "(J)I", (void*)android_os_Parcel_readCallingWorkSourceUid}, + // @CriticalNative + {"nativeReplaceCallingWorkSourceUid", "(JI)Z", (void*)android_os_Parcel_replaceCallingWorkSourceUid}, }; const char* const kParcelPathName = "android/os/Parcel"; diff --git a/core/tests/coretests/src/android/os/BinderWorkSourceService.java b/core/tests/coretests/src/android/os/BinderWorkSourceService.java index 3bca5fbab486..46bd67da371d 100644 --- a/core/tests/coretests/src/android/os/BinderWorkSourceService.java +++ b/core/tests/coretests/src/android/os/BinderWorkSourceService.java @@ -38,11 +38,11 @@ public class BinderWorkSourceService extends Service { } public void setWorkSourceProvider(int uid) { - Binder.setWorkSourceProvider(() -> uid); + Binder.setWorkSourceProvider((x) -> uid); } public void clearWorkSourceProvider() { - Binder.setWorkSourceProvider(Binder::getCallingUid); + Binder.setWorkSourceProvider((x) -> Binder.getCallingUid()); } }; diff --git a/core/tests/coretests/src/android/os/BinderWorkSourceTest.java b/core/tests/coretests/src/android/os/BinderWorkSourceTest.java index e16a3dbe4a26..b14c88f7341e 100644 --- a/core/tests/coretests/src/android/os/BinderWorkSourceTest.java +++ b/core/tests/coretests/src/android/os/BinderWorkSourceTest.java @@ -27,6 +27,7 @@ import android.platform.test.annotations.Presubmit; import androidx.test.InstrumentationRegistry; import androidx.test.filters.LargeTest; +import androidx.test.filters.Suppress; import androidx.test.runner.AndroidJUnit4; import org.junit.After; @@ -97,6 +98,8 @@ public class BinderWorkSourceTest { public void tearDown() throws Exception { sContext.unbindService(mConnection); sContext.unbindService(mNestedConnection); + Binder.setProxyTransactListener(null); + ThreadLocalWorkSource.clear(); } @Test @@ -124,6 +127,28 @@ public class BinderWorkSourceTest { } @Test + public void setWorkSource_propagatedFromBinderProxyListener() throws Exception { + Binder.setProxyTransactListener(new Binder.PropagateWorkSourceTransactListener()); + Binder.clearCallingWorkSource(); + ThreadLocalWorkSource.setUid(UID); + assertEquals(UID, mService.getIncomingWorkSourceUid()); + } + + @Test + public void threadWorkSourceNotPropagated() throws Exception { + Binder.clearCallingWorkSource(); + ThreadLocalWorkSource.setUid(UID); + assertEquals(UID_NONE, mService.getIncomingWorkSourceUid()); + } + + @Test + public void setWorkSource_propagatedFromBinderProxyListener_unset() throws Exception { + Binder.setProxyTransactListener(new Binder.PropagateWorkSourceTransactListener()); + Binder.clearCallingWorkSource(); + assertEquals(UID_NONE, mService.getIncomingWorkSourceUid()); + } + + @Test public void restoreWorkSource() throws Exception { Binder.setCallingWorkSourceUid(UID); long token = Binder.clearCallingWorkSource(); @@ -158,18 +183,6 @@ public class BinderWorkSourceTest { } @Test - public void nestedSetWorkSouceNotPropagated() throws Exception { - Binder.setCallingWorkSourceUid(UID); - - int[] workSources = mNestedService.nestedCall(); - assertEquals(UID, workSources[0]); - // No UID propagated. - assertEquals(UID_NONE, workSources[1]); - // Initial work source restored. - assertEquals(UID, Binder.getCallingWorkSourceUid()); - } - - @Test public void workSourceProvider_default() throws Exception { Binder.clearCallingWorkSource(); mService.clearWorkSourceProvider(); @@ -177,6 +190,7 @@ public class BinderWorkSourceTest { } @Test + @Suppress // WorkSourceProvider is currently invoked only at the end of the binder call. public void workSourceProvider_customProvider() throws Exception { Binder.clearCallingWorkSource(); mService.clearWorkSourceProvider(); @@ -188,4 +202,16 @@ public class BinderWorkSourceTest { mService.clearWorkSourceProvider(); } } + + @Test + public void nestedSetWorkSouceNotPropagated() throws Exception { + Binder.setCallingWorkSourceUid(UID); + + int[] workSources = mNestedService.nestedCall(); + assertEquals(UID, workSources[0]); + // No UID propagated. + assertEquals(UID_NONE, workSources[1]); + // Initial work source restored. + assertEquals(UID, Binder.getCallingWorkSourceUid()); + } } diff --git a/core/tests/coretests/src/android/os/ParcelTest.java b/core/tests/coretests/src/android/os/ParcelTest.java new file mode 100644 index 000000000000..0eba2edbb3c3 --- /dev/null +++ b/core/tests/coretests/src/android/os/ParcelTest.java @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.os; + +import static org.junit.Assert.assertEquals; + +import android.platform.test.annotations.Presubmit; + +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@Presubmit +@RunWith(AndroidJUnit4.class) +public class ParcelTest { + private static final int WORK_SOURCE_1 = 1000; + private static final int WORK_SOURCE_2 = 1002; + private static final String INTERFACE_TOKEN_1 = "IBinder interface token"; + private static final String INTERFACE_TOKEN_2 = "Another IBinder interface token"; + + @Test + public void testCallingWorkSourceUidAfterWrite() { + Parcel p = Parcel.obtain(); + // Method does not throw if replaceCallingWorkSourceUid is called before requests headers + // are added. + assertEquals(false, p.replaceCallingWorkSourceUid(WORK_SOURCE_1)); + assertEquals(Binder.UNSET_WORKSOURCE, p.readCallingWorkSourceUid()); + + // WorkSource can be updated. + p.writeInterfaceToken(INTERFACE_TOKEN_1); + assertEquals(true, p.replaceCallingWorkSourceUid(WORK_SOURCE_1)); + assertEquals(WORK_SOURCE_1, p.readCallingWorkSourceUid()); + + // WorkSource can be updated to unset value. + assertEquals(true, p.replaceCallingWorkSourceUid(Binder.UNSET_WORKSOURCE)); + assertEquals(Binder.UNSET_WORKSOURCE, p.readCallingWorkSourceUid()); + + p.recycle(); + } + + @Test + public void testCallingWorkSourceUidAfterEnforce() { + Parcel p = Parcel.obtain(); + // Write headers manually so that we do not invoke #writeInterfaceToken. + p.writeInt(1); // strict mode header + p.writeInt(WORK_SOURCE_1); // worksource header. + p.writeString(INTERFACE_TOKEN_1); // interface token. + p.setDataPosition(0); + + p.enforceInterface(INTERFACE_TOKEN_1); + assertEquals(WORK_SOURCE_1, p.readCallingWorkSourceUid()); + + // WorkSource can be updated. + assertEquals(true, p.replaceCallingWorkSourceUid(WORK_SOURCE_1)); + assertEquals(WORK_SOURCE_1, p.readCallingWorkSourceUid()); + + p.recycle(); + } + + @Test + public void testParcelWithMultipleHeaders() { + Parcel p = Parcel.obtain(); + Binder.setCallingWorkSourceUid(WORK_SOURCE_1); + p.writeInterfaceToken(INTERFACE_TOKEN_1); + Binder.setCallingWorkSourceUid(WORK_SOURCE_2); + p.writeInterfaceToken(INTERFACE_TOKEN_2); + p.setDataPosition(0); + + // WorkSource is from the first header. + p.enforceInterface(INTERFACE_TOKEN_1); + assertEquals(WORK_SOURCE_1, p.readCallingWorkSourceUid()); + p.enforceInterface(INTERFACE_TOKEN_2); + assertEquals(WORK_SOURCE_1, p.readCallingWorkSourceUid()); + + p.recycle(); + } +} diff --git a/services/core/java/com/android/server/BinderCallsStatsService.java b/services/core/java/com/android/server/BinderCallsStatsService.java index 8b7f321eb087..eafa0e2a6786 100644 --- a/services/core/java/com/android/server/BinderCallsStatsService.java +++ b/services/core/java/com/android/server/BinderCallsStatsService.java @@ -61,11 +61,11 @@ public class BinderCallsStatsService extends Binder { mAppIdWhitelist = new ArraySet<>(); } - public int resolveWorkSourceUid() { + public int resolveWorkSourceUid(int untrustedWorkSourceUid) { final int callingUid = getCallingUid(); final int appId = UserHandle.getAppId(callingUid); if (mAppIdWhitelist.contains(appId)) { - final int workSource = getCallingWorkSourceUid(); + final int workSource = untrustedWorkSourceUid; final boolean isWorkSourceSet = workSource != Binder.UNSET_WORKSOURCE; return isWorkSourceSet ? workSource : callingUid; } @@ -88,10 +88,6 @@ public class BinderCallsStatsService extends Binder { return Binder.getCallingUid(); } - protected int getCallingWorkSourceUid() { - return Binder.getCallingWorkSourceUid(); - } - private ArraySet<Integer> createAppidWhitelist(Context context) { // Use a local copy instead of mAppIdWhitelist to prevent concurrent read access. final ArraySet<Integer> whitelist = new ArraySet<>(); @@ -186,7 +182,7 @@ public class BinderCallsStatsService extends Binder { } else { Binder.setObserver(null); Binder.setProxyTransactListener(null); - Binder.setWorkSourceProvider(Binder::getCallingUid); + Binder.setWorkSourceProvider((x) -> Binder.getCallingUid()); } mEnabled = enabled; mBinderCallsStats.reset(); diff --git a/services/tests/servicestests/src/com/android/server/BinderCallsStatsServiceTest.java b/services/tests/servicestests/src/com/android/server/BinderCallsStatsServiceTest.java index f75617ec5200..d459163fed9a 100644 --- a/services/tests/servicestests/src/com/android/server/BinderCallsStatsServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/BinderCallsStatsServiceTest.java @@ -40,14 +40,10 @@ public class BinderCallsStatsServiceTest { protected int getCallingUid() { return Process.myUid(); } - - protected int getCallingWorkSourceUid() { - return 1; - } }; workSourceProvider.systemReady(InstrumentationRegistry.getContext()); - assertEquals(1, workSourceProvider.resolveWorkSourceUid()); + assertEquals(1, workSourceProvider.resolveWorkSourceUid(1)); } @Test @@ -57,14 +53,10 @@ public class BinderCallsStatsServiceTest { // System process uid which as UPDATE_DEVICE_STATS. return 1001; } - - protected int getCallingWorkSourceUid() { - return 1; - } }; workSourceProvider.systemReady(InstrumentationRegistry.getContext()); - assertEquals(1, workSourceProvider.resolveWorkSourceUid()); + assertEquals(1, workSourceProvider.resolveWorkSourceUid(1)); } @Test @@ -74,13 +66,9 @@ public class BinderCallsStatsServiceTest { // UID without permissions. return Integer.MAX_VALUE; } - - protected int getCallingWorkSourceUid() { - return 1; - } }; workSourceProvider.systemReady(InstrumentationRegistry.getContext()); - assertEquals(Integer.MAX_VALUE, workSourceProvider.resolveWorkSourceUid()); + assertEquals(Integer.MAX_VALUE, workSourceProvider.resolveWorkSourceUid(1)); } } |