Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ef880523 authored by William Escande's avatar William Escande
Browse files

Remove MODIFY_PHONE_STATE permission from startVoiceRecognition 

Multiple reason:
1. This API was wrongly annotated with the permission, looking at the
   symmetrical stopVoiceRecognition that is no enforcing the permission

2. Legacy code was only enforcing the permission if:
  * The device passed was null (impossible according to Fwk code)
  * the device passed was not the active device AND:
    * the previous active device is not disconnected but is null
      (impossible to be both null and not disconnected)
    * Or the flag for managing the sco by audio framework is off and
      there is ongoing leaudio on the previous device and the
      connectAudio on the new device failed so it try to call
      removeActive that in turn will call stopScoUsingVirtualVoiceCall
      that require the permission, but it's impossible since we only do
      this when mVirtualCallStarted is false and here it has to be true

Since this permission was indicated on the API but never enforce, and it
looks like it shouldn't be, removing the enforcement that was put in
place with aosp/3160581 and updating the documentation of the API
accordingly

This CL is also updating the code to clarify some impossible path
discover while looking at this issue:
* startVoiceRecognition wants a non null device. if it's null it's a
  malicious binder call
* setActiveDevice has a impossible situation in which a null device
  could be connected, I removed the null device

Bug: 357736834
Fix: 357736834
Bug: 349682934
Test: m Bluetooth
Flag: Exempt partial revert
Change-Id: I0c9cef34776db20d732ab766c89206d4967a3971
parent 176136b1

android/app/aidl/android/bluetooth/IBluetoothHeadset.aidl

+1 −1
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ interface IBluetoothHeadset { 
    List<BluetoothDevice> getDevicesMatchingConnectionStates(in int[] states, in AttributionSource attributionSource);

    @JavaPassthrough(annotation="@android.annotation.RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)")

    int getConnectionState(in BluetoothDevice device, in AttributionSource attributionSource);

    @JavaPassthrough(annotation="@android.annotation.RequiresPermission(allOf={android.Manifest.permission.BLUETOOTH_CONNECT,android.Manifest.permission.MODIFY_PHONE_STATE})")

    @JavaPassthrough(annotation="@android.annotation.RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)")

    boolean startVoiceRecognition(in BluetoothDevice device, in AttributionSource attributionSource);

    @JavaPassthrough(annotation="@android.annotation.RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)")

    boolean stopVoiceRecognition(in BluetoothDevice device, in AttributionSource attributionSource);

android/app/src/com/android/bluetooth/hfp/HeadsetService.java

+5 −12
Original line number Diff line number Diff line
@@ -635,7 +635,8 @@ public class HeadsetService extends ProfileService { 
                return false;

            }



            service.enforceCallingOrSelfPermission(MODIFY_PHONE_STATE, null);

            requireNonNull(device);



            return service.startVoiceRecognition(device);

        }
@@ -1106,10 +1107,6 @@ public class HeadsetService extends ProfileService { 
                                + mActiveDevice);

                return false;

            }

            if (device == null) {

                Log.i(TAG, "device is null, use active device " + mActiveDevice + " instead");

                device = mActiveDevice;

            }

            boolean pendingRequestByHeadset = false;

            if (mVoiceRecognitionTimeoutEvent != null) {

                if (!mVoiceRecognitionTimeoutEvent.mVoiceRecognitionDevice.equals(device)) {
@@ -1130,7 +1127,7 @@ public class HeadsetService extends ProfileService { 
                }

                pendingRequestByHeadset = true;

            }

            if (!Objects.equals(device, mActiveDevice) && !setActiveDevice(device)) {

            if (!device.equals(mActiveDevice) && !setActiveDevice(device)) {

                Log.w(TAG, "startVoiceRecognition: failed to set " + device + " as active");

                return false;

            }
@@ -1454,12 +1451,8 @@ public class HeadsetService extends ProfileService { 
                                    + previousActiveDevice

                                    + " with status code "

                                    + disconnectStatus);

                    if (previousActiveDevice == null) {

                        removeActiveDevice();

                    } else {

                    mActiveDevice = previousActiveDevice;

                    mNativeInterface.setActiveDevice(previousActiveDevice);

                    }

                    return false;

                }

                if (Utils.isScoManagedByAudioEnabled()) {

android/app/tests/unit/src/com/android/bluetooth/hfp/HeadsetServiceAndStateMachineTest.java

+0 −47
Original line number Diff line number Diff line
@@ -878,53 +878,6 @@ public class HeadsetServiceAndStateMachineTest { 
        Utils.setIsScoManagedByAudioEnabled(false);

    }



    /**

     * Test to verify the following behavior regarding AG initiated voice recognition in the

     * successful scenario 1. AG starts voice recognition and notify the Bluetooth stack via {@link

     * BluetoothHeadset#startVoiceRecognition(BluetoothDevice)} to indicate that voice recognition

     * has started, BluetoothDevice is null in this case 2. AG send +BVRA:1 to current active HF 3.

     * AG start SCO connection if SCO has not been started

     *

     * <p>Reference: Section 4.25, Page 64/144 of HFP 1.7.1 specification

     */

    @Test

    public void testVoiceRecognition_SingleAgInitiatedSuccessNullInput() {

        // Connect HF

        BluetoothDevice device = TestUtils.getTestDevice(mAdapter, 0);

        connectTestDevice(device);

        // Make device active

        assertThat(mHeadsetService.setActiveDevice(device)).isTrue();

        mTestLooper.dispatchAll();

        verify(mNativeInterface).setActiveDevice(device);

        assertThat(mHeadsetService.getActiveDevice()).isEqualTo(device);

        // Start voice recognition on null argument should go to active device

        assertThat(mHeadsetService.startVoiceRecognition(null)).isTrue();

        mTestLooper.dispatchAll();

        verify(mNativeInterface).startVoiceRecognition(device);

    }



    /**

     * Test to verify the following behavior regarding AG initiated voice recognition in the

     * successful scenario 1. AG starts voice recognition and notify the Bluetooth stack via {@link

     * BluetoothHeadset#startVoiceRecognition(BluetoothDevice)} to indicate that voice recognition

     * has started, BluetoothDevice is null and active device is null 2. The call should fail

     *

     * <p>Reference: Section 4.25, Page 64/144 of HFP 1.7.1 specification

     */

    @Test

    public void testVoiceRecognition_SingleAgInitiatedFailNullActiveDevice() {

        // Connect HF

        BluetoothDevice device = TestUtils.getTestDevice(mAdapter, 0);

        connectTestDevice(device);

        // Make device active

        assertThat(mHeadsetService.setActiveDevice(null)).isTrue();

        mTestLooper.dispatchAll();

        assertThat(mHeadsetService.getActiveDevice()).isNull();

        // Start voice recognition on null argument should fail

        assertThat(mHeadsetService.startVoiceRecognition(null)).isFalse();

        mTestLooper.dispatchAll();

    }



    /**

     * Test to verify the following behavior regarding AG stops voice recognition in the successful

     * scenario 1. AG stops voice recognition and notify the Bluetooth stack via {@link

framework/api/current.txt

+1 −1
Original line number Diff line number Diff line
@@ -797,7 +797,7 @@ package android.bluetooth { 
    method @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public boolean isNoiseReductionSupported(@NonNull android.bluetooth.BluetoothDevice);

    method @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public boolean isVoiceRecognitionSupported(@NonNull android.bluetooth.BluetoothDevice);

    method @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public boolean sendVendorSpecificResultCode(android.bluetooth.BluetoothDevice, String, String);

    method @RequiresPermission(allOf={android.Manifest.permission.BLUETOOTH_CONNECT, android.Manifest.permission.MODIFY_PHONE_STATE}) public boolean startVoiceRecognition(android.bluetooth.BluetoothDevice);

    method @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public boolean startVoiceRecognition(android.bluetooth.BluetoothDevice);

    method @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public boolean stopVoiceRecognition(android.bluetooth.BluetoothDevice);

    field @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public static final String ACTION_AUDIO_STATE_CHANGED = "android.bluetooth.headset.profile.action.AUDIO_STATE_CHANGED";

    field @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT) public static final String ACTION_CONNECTION_STATE_CHANGED = "android.bluetooth.headset.profile.action.CONNECTION_STATE_CHANGED";

framework/java/android/bluetooth/BluetoothHeadset.java

+1 −5
Original line number Diff line number Diff line
@@ -701,11 +701,7 @@ public final class BluetoothHeadset implements BluetoothProfile { 
     */

    @RequiresLegacyBluetoothPermission

    @RequiresBluetoothConnectPermission

    @RequiresPermission(

            allOf = {

                BLUETOOTH_CONNECT,

                MODIFY_PHONE_STATE,

            })

    @RequiresPermission(BLUETOOTH_CONNECT)

    public boolean startVoiceRecognition(BluetoothDevice device) {

        if (DBG) log("startVoiceRecognition()");

        final IBluetoothHeadset service = getService();