From 560c3ac1dcc2f18a0b32aa60d8eabebfeb1d0001 Mon Sep 17 00:00:00 2001 From: markchien Date: Fri, 16 Aug 2019 15:33:28 +0800 Subject: [PATCH] Passing caller package name to setBluetoothTethering This is necessary to examine caller's permission. If caller's uid is not same as passing package name, SecurityException would be throwed. This change also clear the identity before calling BluetoothPan#setBluetoothTethering() in Tethering#setBluetoothTethering. This is fine because caller already pass permission check before in ConnectivityService. See the flow below: ConnectivityManager#startTethering -> ConnectivityService#startTethering -> Tethering#startTethering -> Tethering#setBluetoothTethering -> BluetoothPan#setBluetoothTethering Bug: 134649258 Test: -build, flash, boot -atest FrameworkNetTests -manual test with bluetooth OFF/ON Change-Id: I2140398ad3bbc8076f729c843f0515c654553aaf --- core/java/android/bluetooth/BluetoothPan.java | 8 ++++++-- .../android/server/connectivity/Tethering.java | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/core/java/android/bluetooth/BluetoothPan.java b/core/java/android/bluetooth/BluetoothPan.java index fb78789ba8ade..cfb363a0834c7 100644 --- a/core/java/android/bluetooth/BluetoothPan.java +++ b/core/java/android/bluetooth/BluetoothPan.java @@ -118,6 +118,8 @@ public final class BluetoothPan implements BluetoothProfile { */ public static final int PAN_OPERATION_SUCCESS = 1004; + private final Context mContext; + private BluetoothAdapter mAdapter; private final BluetoothProfileConnector mProfileConnector = new BluetoothProfileConnector(this, BluetoothProfile.PAN, @@ -136,6 +138,7 @@ public final class BluetoothPan implements BluetoothProfile { @UnsupportedAppUsage /*package*/ BluetoothPan(Context context, ServiceListener listener) { mAdapter = BluetoothAdapter.getDefaultAdapter(); + mContext = context; mProfileConnector.connect(context, listener); } @@ -287,11 +290,12 @@ public final class BluetoothPan implements BluetoothProfile { @UnsupportedAppUsage public void setBluetoothTethering(boolean value) { - if (DBG) log("setBluetoothTethering(" + value + ")"); + String pkgName = mContext.getOpPackageName(); + if (DBG) log("setBluetoothTethering(" + value + "), calling package:" + pkgName); final IBluetoothPan service = getService(); if (service != null && isEnabled()) { try { - service.setBluetoothTethering(value); + service.setBluetoothTethering(value, pkgName); } catch (RemoteException e) { Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable())); } diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index 86d12124785db..5fd5c4bebb2a7 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -455,7 +455,20 @@ public class Tethering extends BaseNetworkObserver { @Override public void onServiceConnected(int profile, BluetoothProfile proxy) { - ((BluetoothPan) proxy).setBluetoothTethering(enable); + // Clear identify is fine because caller already pass tethering permission at + // ConnectivityService#startTethering()(or stopTethering) before the control comes + // here. Bluetooth will check tethering permission again that there is + // Context#getOpPackageName() under BluetoothPan#setBluetoothTethering() to get + // caller's package name for permission check. + // Calling BluetoothPan#setBluetoothTethering() here means the package name always + // be system server. If calling identity is not cleared, that package's uid might + // not match calling uid and end up in permission denied. + final long identityToken = Binder.clearCallingIdentity(); + try { + ((BluetoothPan) proxy).setBluetoothTethering(enable); + } finally { + Binder.restoreCallingIdentity(identityToken); + } // TODO: Enabling bluetooth tethering can fail asynchronously here. // We should figure out a way to bubble up that failure instead of sending success. final int result = (((BluetoothPan) proxy).isTetheringOn() == enable)