Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Extract permission checking as a separate aspect

Browse Source 
Permission checking logic is about to get more complicated, so this
change extracts it to a separate aspect class, where it doesn't get
mixed up with validation logic.

Change-Id: Id1c718fdd233244537c51ec1802a25571671c91c
Bug: 163865561
This commit is contained in:
Ytai Ben-Tsvi
2020-04-29 14:32:23 -07:00
parent 2c344c0fec
commit 4f6f70cdd1
3 changed files with 280 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

267
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java Normal file
View File

@@ -0,0 +1,267 @@
/*
* Copyright (C) 2020 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 com.android.server.soundtrigger_middleware;
import android.Manifest;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.Context;
import android.content.PermissionChecker;
import android.media.soundtrigger_middleware.ISoundTriggerCallback;
import android.media.soundtrigger_middleware.ISoundTriggerMiddlewareService;
import android.media.soundtrigger_middleware.ISoundTriggerModule;
import android.media.soundtrigger_middleware.ModelParameterRange;
import android.media.soundtrigger_middleware.PhraseRecognitionEvent;
import android.media.soundtrigger_middleware.PhraseSoundModel;
import android.media.soundtrigger_middleware.RecognitionConfig;
import android.media.soundtrigger_middleware.RecognitionEvent;
import android.media.soundtrigger_middleware.SoundModel;
import android.media.soundtrigger_middleware.SoundTriggerModuleDescriptor;
import android.media.soundtrigger_middleware.Status;
import android.os.IBinder;
import android.os.RemoteException;
import android.os.ServiceSpecificException;
import java.io.PrintWriter;
import java.util.Objects;
/**
* This is a decorator of an {@link ISoundTriggerMiddlewareService}, which enforces permissions.
* <p>
* Every public method in this class, overriding an interface method, must follow the following
* pattern:
* <code><pre>
* @Override public T method(S arg) {
* // Permission check.
* checkPermissions();
* return mDelegate.method(arg);
* }
* </pre></code>
*
* {@hide}
*/
public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddlewareInternal, Dumpable {
private static final String TAG = "SoundTriggerMiddlewarePermission";
private final @NonNull ISoundTriggerMiddlewareInternal mDelegate;
private final @NonNull Context mContext;
public SoundTriggerMiddlewarePermission(
@NonNull ISoundTriggerMiddlewareInternal delegate, @NonNull Context context) {
mDelegate = delegate;
mContext = context;
}
@Override
public @NonNull
SoundTriggerModuleDescriptor[] listModules() throws RemoteException {
checkPermissions();
return mDelegate.listModules();
}
@Override
public @NonNull
ISoundTriggerModule attach(int handle,
@NonNull ISoundTriggerCallback callback) throws RemoteException {
checkPermissions();
return new ModuleWrapper(
mDelegate.attach(handle, new CallbackWrapper(callback)));
}
@Override
public void setCaptureState(boolean active) throws RemoteException {
// This is an internal call. No permissions needed.
mDelegate.setCaptureState(active);
}
// Override toString() in order to have the delegate's ID in it.
@Override
public String toString() {
return mDelegate.toString();
}
@Override
public IBinder asBinder() {
throw new UnsupportedOperationException(
"This implementation is not inteded to be used directly with Binder.");
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
* caller temporarily doesn't have the right permissions to use this service.
*/
private void checkPermissions() {
enforcePermission(Manifest.permission.RECORD_AUDIO);
enforcePermission(Manifest.permission.CAPTURE_AUDIO_HOTWORD);
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
* caller temporarily doesn't have the given permission.
*
* @param permission The permission to check.
*/
private void enforcePermission(String permission) {
final int status = PermissionChecker.checkCallingOrSelfPermissionForPreflight(mContext,
permission);
switch (status) {
case PermissionChecker.PERMISSION_GRANTED:
return;
case PermissionChecker.PERMISSION_HARD_DENIED:
throw new SecurityException(
String.format("Caller must have the %s permission.", permission));
case PermissionChecker.PERMISSION_SOFT_DENIED:
throw new ServiceSpecificException(Status.TEMPORARY_PERMISSION_DENIED,
String.format("Caller must have the %s permission.", permission));
default:
throw new RuntimeException("Unexpected perimission check result.");
}
}
@Override
public void dump(PrintWriter pw) {
if (mDelegate instanceof Dumpable) {
((Dumpable) mDelegate).dump(pw);
}
}
/**
* A wrapper around an {@link ISoundTriggerModule} implementation, to address the same aspects
* mentioned in {@link SoundTriggerModule} above. This class follows the same conventions.
*/
private class ModuleWrapper extends ISoundTriggerModule.Stub {
private final ISoundTriggerModule mDelegate;
ModuleWrapper(@NonNull ISoundTriggerModule delegate) {
mDelegate = delegate;
}
@Override
public int loadModel(@NonNull SoundModel model) throws RemoteException {
checkPermissions();
return mDelegate.loadModel(model);
}
@Override
public int loadPhraseModel(@NonNull PhraseSoundModel model) throws RemoteException {
checkPermissions();
return mDelegate.loadPhraseModel(model);
}
@Override
public void unloadModel(int modelHandle) throws RemoteException {
checkPermissions();
mDelegate.unloadModel(modelHandle);
}
@Override
public void startRecognition(int modelHandle, @NonNull RecognitionConfig config)
throws RemoteException {
checkPermissions();
mDelegate.startRecognition(modelHandle, config);
}
@Override
public void stopRecognition(int modelHandle) throws RemoteException {
checkPermissions();
mDelegate.stopRecognition(modelHandle);
}
@Override
public void forceRecognitionEvent(int modelHandle) throws RemoteException {
checkPermissions();
mDelegate.forceRecognitionEvent(modelHandle);
}
@Override
public void setModelParameter(int modelHandle, int modelParam, int value)
throws RemoteException {
checkPermissions();
mDelegate.setModelParameter(modelHandle, modelParam, value);
}
@Override
public int getModelParameter(int modelHandle, int modelParam) throws RemoteException {
checkPermissions();
return mDelegate.getModelParameter(modelHandle, modelParam);
}
@Override
@Nullable
public ModelParameterRange queryModelParameterSupport(int modelHandle, int modelParam)
throws RemoteException {
checkPermissions();
return mDelegate.queryModelParameterSupport(modelHandle,
modelParam);
}
@Override
public void detach() throws RemoteException {
checkPermissions();
mDelegate.detach();
}
// Override toString() in order to have the delegate's ID in it.
@Override
public String toString() {
return Objects.toString(mDelegate);
}
}
private class CallbackWrapper implements ISoundTriggerCallback {
private final ISoundTriggerCallback mDelegate;
private CallbackWrapper(ISoundTriggerCallback delegate) {
mDelegate = delegate;
}
@Override
public void onRecognition(int modelHandle, RecognitionEvent event) throws RemoteException {
mDelegate.onRecognition(modelHandle, event);
}
@Override
public void onPhraseRecognition(int modelHandle, PhraseRecognitionEvent event)
throws RemoteException {
mDelegate.onPhraseRecognition(modelHandle, event);
}
@Override
public void onRecognitionAvailabilityChange(boolean available) throws RemoteException {
mDelegate.onRecognitionAvailabilityChange(available);
}
@Override
public void onModuleDied() throws RemoteException {
mDelegate.onModuleDied();
}
@Override
public IBinder asBinder() {
return mDelegate.asBinder();
}
// Override toString() in order to have the delegate's ID in it.
@Override
public String toString() {
return Objects.toString(mDelegate);
}
}
}

10
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java
View File

@@ -93,7 +93,8 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic
return new ModuleService(mDelegate.attach(handle, callback));
}
@Override protected void dump(FileDescriptor fd, PrintWriter fout, String[] args) {
@Override
protected void dump(FileDescriptor fd, PrintWriter fout, String[] args) {
if (mDelegate instanceof Dumpable) {
((Dumpable) mDelegate).dump(fout);
}
@@ -182,9 +183,10 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic
publishBinderService(Context.SOUND_TRIGGER_MIDDLEWARE_SERVICE,
new SoundTriggerMiddlewareService(
new SoundTriggerMiddlewareLogging(
new SoundTriggerMiddlewareValidation(
new SoundTriggerMiddlewareImpl(factories,
new AudioSessionProviderImpl()),
new SoundTriggerMiddlewarePermission(
new SoundTriggerMiddlewareValidation(
new SoundTriggerMiddlewareImpl(factories,
new AudioSessionProviderImpl())),
getContext()))));
}
}

77
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java
View File

@@ -53,9 +53,9 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
/**
* This is a decorator of an {@link ISoundTriggerMiddlewareService}, which enforces permissions and
* correct usage by the client, as well as makes sure that exceptions representing a server
* malfunction do not get sent to the client.
* This is a decorator of an {@link ISoundTriggerMiddlewareService}, which enforces correct usage by
* the client, as well as makes sure that exceptions representing a server malfunction get sent to
* the client in a consistent manner, which cannot be confused with a client fault.
* <p>
* This is intended to extract the non-business logic out of the underlying implementation and thus
* make it easier to maintain each one of those separate aspects. A design trade-off is being made
@@ -70,8 +70,6 @@ import java.util.concurrent.atomic.AtomicReference;
* pattern:
* <code><pre>
* @Override public T method(S arg) {
* // Permission check.
* checkPermissions();
* // Input validation.
* ValidationUtil.validateS(arg);
* synchronized (this) {
@@ -95,9 +93,8 @@ import java.util.concurrent.atomic.AtomicReference;
* with client-server separation.
* <p>
* <b>Exception handling approach:</b><br>
* We make sure all client faults (permissions, argument and state validation) happen first, and
* would throw {@link SecurityException}, {@link IllegalArgumentException}/
* {@link NullPointerException} or {@link
* We make sure all client faults (argument and state validation) happen first, and
* would throw {@link IllegalArgumentException}/{@link NullPointerException} or {@link
* IllegalStateException}, respectively. All those exceptions are treated specially by Binder and
* will get sent back to the client.<br>
* Once this is done, any subsequent fault is considered either a recoverable (expected) or
@@ -130,13 +127,11 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
private AtomicReference<Boolean> mCaptureState = new AtomicReference<>();
private final @NonNull ISoundTriggerMiddlewareInternal mDelegate;
private final @NonNull Context mContext;
private Map<Integer, ModuleState> mModules;
public SoundTriggerMiddlewareValidation(
@NonNull ISoundTriggerMiddlewareInternal delegate, @NonNull Context context) {
@NonNull ISoundTriggerMiddlewareInternal delegate) {
mDelegate = delegate;
mContext = context;
}
/**
@@ -169,8 +164,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public @NonNull
SoundTriggerModuleDescriptor[] listModules() {
// Permission check.
checkPermissions();
// Input validation (always valid).
synchronized (this) {
@@ -193,8 +186,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public @NonNull ISoundTriggerModule attach(int handle,
@NonNull ISoundTriggerCallback callback) {
// Permission check.
checkPermissions();
// Input validation.
Objects.requireNonNull(callback);
Objects.requireNonNull(callback.asBinder());
@@ -244,40 +235,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
return mDelegate.toString();
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
* caller temporarily doesn't have the right permissions to use this service.
*/
void checkPermissions() {
enforcePermission(Manifest.permission.RECORD_AUDIO);
enforcePermission(Manifest.permission.CAPTURE_AUDIO_HOTWORD);
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
* caller temporarily doesn't have the given permission.
*
* @param permission The permission to check.
*/
void enforcePermission(String permission) {
final int status = PermissionChecker.checkCallingOrSelfPermissionForPreflight(mContext,
permission);
switch (status) {
case PermissionChecker.PERMISSION_GRANTED:
return;
case PermissionChecker.PERMISSION_HARD_DENIED:
throw new SecurityException(
String.format("Caller must have the %s permission.", permission));
case PermissionChecker.PERMISSION_SOFT_DENIED:
throw new ServiceSpecificException(Status.TEMPORARY_PERMISSION_DENIED,
String.format("Caller must have the %s permission.", permission));
default:
throw new RuntimeException("Unexpected perimission check result.");
}
}
@Override
public IBinder asBinder() {
throw new UnsupportedOperationException(
@@ -431,8 +388,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public int loadModel(@NonNull SoundModel model) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validateGenericModel(model);
@@ -455,8 +410,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public int loadPhraseModel(@NonNull PhraseSoundModel model) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validatePhraseModel(model);
@@ -479,8 +432,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void unloadModel(int modelHandle) {
// Permission check.
checkPermissions();
// Input validation (always valid).
synchronized (SoundTriggerMiddlewareValidation.this) {
@@ -510,8 +461,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void startRecognition(int modelHandle, @NonNull RecognitionConfig config) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validateRecognitionConfig(config);
@@ -547,8 +496,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void stopRecognition(int modelHandle) {
// Permission check.
checkPermissions();
// Input validation (always valid).
synchronized (SoundTriggerMiddlewareValidation.this) {
@@ -575,8 +522,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void forceRecognitionEvent(int modelHandle) {
// Permission check.
checkPermissions();
// Input validation (always valid).
synchronized (SoundTriggerMiddlewareValidation.this) {
@@ -602,8 +547,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void setModelParameter(int modelHandle, int modelParam, int value) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validateModelParameter(modelParam);
@@ -630,8 +573,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public int getModelParameter(int modelHandle, int modelParam) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validateModelParameter(modelParam);
@@ -659,8 +600,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
@Nullable
public ModelParameterRange queryModelParameterSupport(int modelHandle, int modelParam) {
// Permission check.
checkPermissions();
// Input validation.
ValidationUtil.validateModelParameter(modelParam);
@@ -689,8 +628,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
@Override
public void detach() {
// Permission check.
checkPermissions();
// Input validation (always valid).
synchronized (SoundTriggerMiddlewareValidation.this) {
@@ -714,7 +651,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
// Override toString() in order to have the delegate's ID in it.
@Override
public String toString() {
return Objects.toString(mDelegate.toString());
return Objects.toString(mDelegate);
}
private void detachInternal() {