Permission support for diagnostics.
This creates two permissions for access to diagnostic data:
- DIAGNOSTIC_READ, for read-only access to live and freeze frame data;
- DIAGNOSTIC_CLEAR, for deleting DTC data from the car.
It also extends the meaning of PERMISSION_VENDOR_EXTENSION to mean being allowed to read vendor-specific diagnostic sensor data.
Test: build
Bug: 35435164
For O-MR1.
Change-Id: I046bf6ae4a7aa2b2570ea5657bff9e1ce86edbce
diff --git a/car-lib/api/system-current.txt b/car-lib/api/system-current.txt
index 7a42d52..ef9f9b4 100644
--- a/car-lib/api/system-current.txt
+++ b/car-lib/api/system-current.txt
@@ -21,6 +21,8 @@
field public static final java.lang.String PERMISSION_CAR_CABIN = "android.car.permission.CAR_CABIN";
field public static final java.lang.String PERMISSION_CAR_CAMERA = "android.car.permission.CAR_CAMERA";
field public static final java.lang.String PERMISSION_CAR_CONTROL_AUDIO_VOLUME = "android.car.permission.CAR_CONTROL_AUDIO_VOLUME";
+ field public static final java.lang.String PERMISSION_CAR_DIAGNOSTIC_CLEAR = "android.car.permission.DIAGNOSTIC_CLEAR";
+ field public static final java.lang.String PERMISSION_CAR_DIAGNOSTIC_READ = "android.car.permission.DIAGNOSTIC_READ";
field public static final java.lang.String PERMISSION_CAR_HVAC = "android.car.permission.CAR_HVAC";
field public static final java.lang.String PERMISSION_CAR_PROJECTION = "android.car.permission.CAR_PROJECTION";
field public static final java.lang.String PERMISSION_CAR_RADIO = "android.car.permission.CAR_RADIO";
diff --git a/car-lib/src/android/car/Car.java b/car-lib/src/android/car/Car.java
index 313256e..44657e1 100644
--- a/car-lib/src/android/car/Car.java
+++ b/car-lib/src/android/car/Car.java
@@ -256,6 +256,24 @@
@SystemApi
public static final String PERMISSION_VMS_SUBSCRIBER = "android.car.permission.VMS_SUBSCRIBER";
+ /**
+ * Permissions necessary to read diagnostic information.
+ *
+ * @hide
+ */
+ @FutureFeature
+ @SystemApi
+ public static final String PERMISSION_CAR_DIAGNOSTIC_READ = "android.car.permission.DIAGNOSTIC_READ";
+
+ /**
+ * Permissions necessary to clear diagnostic information.
+ *
+ * @hide
+ */
+ @FutureFeature
+ @SystemApi
+ public static final String PERMISSION_CAR_DIAGNOSTIC_CLEAR = "android.car.permission.DIAGNOSTIC_CLEAR";
+
/** Type of car connection: platform runs directly in car. */
public static final int CONNECTION_TYPE_EMBEDDED = 5;
diff --git a/car-lib/src/android/car/hardware/CarDiagnosticEvent.java b/car-lib/src/android/car/hardware/CarDiagnosticEvent.java
index b9c2f61..fc205ec 100644
--- a/car-lib/src/android/car/hardware/CarDiagnosticEvent.java
+++ b/car-lib/src/android/car/hardware/CarDiagnosticEvent.java
@@ -296,7 +296,7 @@
* Diagnostic Troubleshooting Code (DTC) that was detected and caused this frame to be stored
* (if a freeze frame). Always null for a live frame.
*/
- public final String DTC;
+ public final String dtc;
public CarDiagnosticEvent(Parcel in) {
frameType = in.readInt();
@@ -315,7 +315,7 @@
int value = in.readInt();
intValues.put(key, value);
}
- DTC = (String)in.readValue(String.class.getClassLoader());
+ dtc = (String)in.readValue(String.class.getClassLoader());
// version 1 up to here
}
@@ -340,7 +340,7 @@
dest.writeInt(key);
dest.writeInt(intValues.get(key));
}
- dest.writeValue(DTC);
+ dest.writeValue(dtc);
}
public static final Parcelable.Creator<CarDiagnosticEvent> CREATOR =
@@ -364,7 +364,7 @@
this.timestamp = timestamp;
this.floatValues = floatValues;
this.intValues = intValues;
- this.DTC = dtc;
+ this.dtc = dtc;
}
public static class Builder {
@@ -372,7 +372,7 @@
private long mTimestamp = 0;
private SparseArray<Float> mFloatValues = new SparseArray<>();
private SparseIntArray mIntValues = new SparseIntArray();
- private String mDTC = null;
+ private String mDtc = null;
private Builder(int type) {
mType = type;
@@ -401,16 +401,38 @@
return this;
}
- public Builder withDTC(String DTC) {
- mDTC = DTC;
+ public Builder withDTC(String dtc) {
+ mDtc = dtc;
return this;
}
public CarDiagnosticEvent build() {
- return new CarDiagnosticEvent(mType, mTimestamp, mFloatValues, mIntValues, mDTC);
+ return new CarDiagnosticEvent(mType, mTimestamp, mFloatValues, mIntValues, mDtc);
}
}
+ /**
+ * Returns a copy of this CarDiagnosticEvent with all vendor-specific sensors removed.
+ * @hide
+ */
+ public CarDiagnosticEvent withVendorSensorsRemoved() {
+ SparseIntArray newIntValues = intValues.clone();
+ SparseArray<Float> newFloatValues = floatValues.clone();
+ for(int i = 0; i < intValues.size(); ++i) {
+ int key = intValues.keyAt(i);
+ if (key >= Obd2IntegerSensorIndex.LAST_SYSTEM) {
+ newIntValues.delete(key);
+ }
+ }
+ for(int i = 0; i < floatValues.size(); ++i) {
+ int key = floatValues.keyAt(i);
+ if (key >= Obd2FloatSensorIndex.LAST_SYSTEM) {
+ newFloatValues.delete(key);
+ }
+ }
+ return new CarDiagnosticEvent(frameType, timestamp, newFloatValues, newIntValues, dtc);
+ }
+
public boolean isLiveFrame() {
return CarDiagnosticManager.FRAME_TYPE_FLAG_LIVE == frameType;
}
@@ -422,7 +444,7 @@
public boolean isEmptyFrame() {
boolean empty = (0 == intValues.size());
empty &= (0 == floatValues.size());
- if (isFreezeFrame()) empty &= DTC.isEmpty();
+ if (isFreezeFrame()) empty &= dtc.isEmpty();
return empty;
}
@@ -454,7 +476,7 @@
+ "\tfloatValues: %s\n}",
isLiveFrame() ? "live" : "freeze",
timestamp,
- DTC,
+ dtc,
intValues.toString(),
floatValues.toString());
}
diff --git a/car-lib/src/android/car/hardware/CarDiagnosticManager.java b/car-lib/src/android/car/hardware/CarDiagnosticManager.java
index 01a436b..d3ad137 100644
--- a/car-lib/src/android/car/hardware/CarDiagnosticManager.java
+++ b/car-lib/src/android/car/hardware/CarDiagnosticManager.java
@@ -16,18 +16,19 @@
package android.car.hardware;
+import android.car.Car;
import android.car.CarApiUtil;
import android.car.CarLibLog;
import android.car.CarManagerBase;
import android.car.CarNotConnectedException;
import android.content.Context;
import android.os.Handler;
-import android.os.Handler.Callback;
import android.os.IBinder;
import android.os.RemoteException;
import android.util.Log;
import android.util.SparseArray;
+import com.android.car.internal.CarPermission;
import com.android.car.internal.CarRatedListeners;
import com.android.car.internal.SingleMessageHandler;
@@ -51,6 +52,8 @@
private CarDiagnosticEventListenerToService mListenerToService;
+ private final CarPermission mVendorExtensionPermission;
+
public CarDiagnosticManager(IBinder service, Context context, Handler handler) {
mService = ICarDiagnostic.Stub.asInterface(service);
mHandlerCallback = new SingleMessageHandler<CarDiagnosticEvent>(handler.getLooper(),
@@ -64,6 +67,7 @@
}
}
};
+ mVendorExtensionPermission = new CarPermission(context, Car.PERMISSION_VENDOR_EXTENSION);
}
@Override
@@ -284,10 +288,14 @@
return;
}
mLastUpdateTime = updateTime;
+ final boolean hasVendorExtensionPermission = mVendorExtensionPermission.checkGranted();
+ final CarDiagnosticEvent eventToDispatch = hasVendorExtensionPermission ?
+ event :
+ event.withVendorSensorsRemoved();
getListeners().forEach(new Consumer<OnDiagnosticEventListener>() {
@Override
public void accept(OnDiagnosticEventListener listener) {
- listener.onDiagnosticEvent(event);
+ listener.onDiagnosticEvent(eventToDispatch);
}
});
}
diff --git a/car-lib/src/com/android/car/internal/CarPermission.java b/car-lib/src/com/android/car/internal/CarPermission.java
new file mode 100644
index 0000000..0b3820f
--- /dev/null
+++ b/car-lib/src/com/android/car/internal/CarPermission.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2017 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.car.internal;
+
+import android.content.Context;
+import android.content.pm.PackageManager;
+import android.os.Binder;
+import android.os.Process;
+
+/**
+ * Represent an Android permission.
+ *
+ * @hide
+ */
+public class CarPermission {
+ private final Context mContext;
+ private final String mName;
+
+ /** @hide */
+ public CarPermission(Context context, String name) {
+ mContext = context;
+ mName = name;
+ }
+
+ /** @hide */
+ public boolean checkGranted() {
+ if (mName != null) {
+ if (Binder.getCallingUid() != Process.myUid()) {
+ return PackageManager.PERMISSION_GRANTED ==
+ mContext.checkCallingOrSelfPermission(mName);
+ }
+ }
+ return true;
+ }
+
+ /** @hide */
+ public void assertGranted() {
+ if (checkGranted()) return;
+ throw new SecurityException(
+ "client does not have permission:"
+ + mName
+ + " pid:"
+ + Binder.getCallingPid()
+ + " uid:"
+ + Binder.getCallingUid());
+ }
+}
diff --git a/service/AndroidManifest.xml b/service/AndroidManifest.xml
index 57b5def..5820221 100644
--- a/service/AndroidManifest.xml
+++ b/service/AndroidManifest.xml
@@ -85,6 +85,16 @@
android:protectionLevel="system|signature"
android:label="@string/car_permission_car_navigation_manager"
android:description="@string/car_permission_desc_car_navigation_manager" />
+ <permission
+ android:name="android.car.permission.DIAGNOSTIC_READ"
+ android:protectionLevel="system|signature"
+ android:label="@string/car_permission_label_diag_read"
+ android:description="@string/car_permission_desc_diag_read" />
+ <permission
+ android:name="android.car.permission.DIAGNOSTIC_CLEAR"
+ android:protectionLevel="dangerous"
+ android:label="@string/car_permission_label_diag_clear"
+ android:description="@string/car_permission_desc_diag_clear" />
<!-- may replace this with system permission if proper one is defined. -->
<permission
diff --git a/service/res/values/strings.xml b/service/res/values/strings.xml
index cf9b32a..c5c07f4 100644
--- a/service/res/values/strings.xml
+++ b/service/res/values/strings.xml
@@ -96,4 +96,14 @@
<string name="activity_blocked_string">The application is not allowed while driving.</string>
<!-- Blocking activity: Message on exit button. [CHAR LIMIT=NONE] -->
<string name="exit_now">Exit</string>
+
+ <!-- Permission text: apps can control diagnostic data [CHAR LIMIT=NONE] -->
+ <string name="car_permission_label_diag_read">Diagnostic Data</string>
+ <!-- Permission text: apps can read diagnostic data from the car [CHAR LIMIT=NONE] -->
+ <string name="car_permission_desc_diag_read">Read diagnostic data from the car</string>
+
+ <!-- Permission text: apps can control diagnostic data [CHAR LIMIT=NONE] -->
+ <string name="car_permission_label_diag_clear">Diagnostic Data</string>
+ <!-- Permission text: apps can clear diagnostic data from the car [CHAR LIMIT=NONE] -->
+ <string name="car_permission_desc_diag_clear">Clear diagnostic data from the car</string>
</resources>
diff --git a/service/src/com/android/car/CarDiagnosticService.java b/service/src/com/android/car/CarDiagnosticService.java
index cf295aa..5b4b675 100644
--- a/service/src/com/android/car/CarDiagnosticService.java
+++ b/service/src/com/android/car/CarDiagnosticService.java
@@ -18,23 +18,22 @@
import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.car.Car;
import android.car.annotation.FutureFeature;
import android.car.hardware.CarDiagnosticEvent;
import android.car.hardware.CarDiagnosticManager;
import android.car.hardware.ICarDiagnostic;
import android.car.hardware.ICarDiagnosticEventListener;
import android.content.Context;
-import android.content.pm.PackageManager;
-import android.os.Binder;
import android.os.Handler;
import android.os.IBinder;
import android.os.Looper;
import android.os.Message;
-import android.os.Process;
import android.os.RemoteException;
import android.os.SystemClock;
import android.util.ArrayMap;
import android.util.Log;
+import com.android.car.internal.CarPermission;
import com.android.car.Listeners.ClientWithRate;
import com.android.car.hal.DiagnosticHalService;
import com.android.internal.annotations.GuardedBy;
@@ -81,9 +80,16 @@
private final Context mContext;
+ private final CarPermission mDiagnosticReadPermission;
+
+ private final CarPermission mDiagnosticClearPermission;
+
public CarDiagnosticService(Context context, DiagnosticHalService diagnosticHal) {
mContext = context;
mDiagnosticHal = diagnosticHal;
+ mDiagnosticReadPermission = new CarPermission(mContext, Car.PERMISSION_CAR_DIAGNOSTIC_READ);
+ mDiagnosticClearPermission = new CarPermission(mContext,
+ Car.PERMISSION_CAR_DIAGNOSTIC_CLEAR);
}
@Override
@@ -217,23 +223,6 @@
return events;
}
- private void assertPermission(int frameType) {
- if (Binder.getCallingUid() != Process.myUid()) {
- switch (getDiagnosticPermission(frameType)) {
- case PackageManager.PERMISSION_GRANTED:
- break;
- default:
- throw new SecurityException(
- "client does not have permission:"
- + getPermissionName(frameType)
- + " pid:"
- + Binder.getCallingPid()
- + " uid:"
- + Binder.getCallingUid());
- }
- }
- }
-
@Override
public boolean registerOrUpdateDiagnosticListener(int frameType, int rate,
ICarDiagnosticEventListener listener) {
@@ -243,7 +232,7 @@
Listeners<DiagnosticClient> diagnosticListeners = null;
mDiagnosticLock.lock();
try {
- assertPermission(frameType);
+ mDiagnosticReadPermission.assertGranted();
diagnosticClient = findDiagnosticClientLocked(listener);
Listeners.ClientWithRate<DiagnosticClient> diagnosticClientWithRate = null;
if (diagnosticClient == null) {
@@ -314,21 +303,6 @@
return true;
}
- //TODO(egranata): handle permissions correctly
- private int getDiagnosticPermission(int frameType) {
- String permission = getPermissionName(frameType);
- int result = PackageManager.PERMISSION_GRANTED;
- if (permission != null) {
- return mContext.checkCallingOrSelfPermission(permission);
- }
- // If no permission is required, return granted.
- return result;
- }
-
- private String getPermissionName(int frameType) {
- return null;
- }
-
private boolean startDiagnostic(int frameType, int rate) {
Log.i(CarLog.TAG_DIAGNOSTIC, String.format("starting diagnostic %s at rate %d",
frameType, rate));
@@ -471,7 +445,7 @@
@Override
public boolean clearFreezeFrames(long... timestamps) {
- //TODO(egranata): verify permissions before executing operation
+ mDiagnosticClearPermission.assertGranted();
if (mDiagnosticHal.getDiagnosticCapabilities().isFreezeFrameClearSupported()) {
mFreezeFrameDiagnosticRecords.lock();
mDiagnosticHal.clearFreezeFrames(timestamps);
diff --git a/service/src/com/android/car/ICarImpl.java b/service/src/com/android/car/ICarImpl.java
index ae28c7f..61aae91 100644
--- a/service/src/com/android/car/ICarImpl.java
+++ b/service/src/com/android/car/ICarImpl.java
@@ -206,7 +206,7 @@
case Car.DIAGNOSTIC_SERVICE:
FeatureUtil.assertFeature(FeatureConfiguration.ENABLE_DIAGNOSTIC);
if (FeatureConfiguration.ENABLE_DIAGNOSTIC) {
- //TODO(egranata): handle permissions
+ assertAnyDiagnosticPermission(mContext);
return mCarDiagnosticService;
}
case Car.HVAC_SERVICE:
@@ -298,6 +298,13 @@
}
@FutureFeature
+ public static void assertAnyDiagnosticPermission(Context context) {
+ assertAnyPermission(context,
+ Car.PERMISSION_CAR_DIAGNOSTIC_READ,
+ Car.PERMISSION_CAR_DIAGNOSTIC_CLEAR);
+ }
+
+ @FutureFeature
public static void assertVmsPublisherPermission(Context context) {
assertPermission(context, Car.PERMISSION_VMS_PUBLISHER);
}
@@ -313,6 +320,16 @@
}
}
+ public static void assertAnyPermission(Context context, String... permissions) {
+ for (String permission : permissions) {
+ if (context.checkCallingOrSelfPermission(permission) ==
+ PackageManager.PERMISSION_GRANTED) {
+ return;
+ }
+ }
+ throw new SecurityException("requires any of " + Arrays.toString(permissions));
+ }
+
void dump(PrintWriter writer) {
writer.println("*FutureConfig, DEFAULT:" + FeatureConfiguration.DEFAULT);
//TODO dump all feature flags by reflection
diff --git a/tests/android_car_api_test/src/android/car/apitest/CarDiagnosticManagerTest.java b/tests/android_car_api_test/src/android/car/apitest/CarDiagnosticManagerTest.java
index b19592c..21fb5e0 100644
--- a/tests/android_car_api_test/src/android/car/apitest/CarDiagnosticManagerTest.java
+++ b/tests/android_car_api_test/src/android/car/apitest/CarDiagnosticManagerTest.java
@@ -78,6 +78,7 @@
/**
* Test that we can read live frame data over the diagnostic manager
+ *
* @throws Exception
*/
public void testLiveFrame() throws Exception {
@@ -90,6 +91,7 @@
/**
* Test that we can read well-formed freeze frame data over the diagnostic manager
+ *
* @throws Exception
*/
public void testFreezeFrames() throws Exception {
@@ -101,7 +103,7 @@
assertEquals(timestamp, freezeFrame.timestamp);
assertTrue(freezeFrame.isFreezeFrame());
assertFalse(freezeFrame.isEmptyFrame());
- assertNotSame("", freezeFrame.DTC);
+ assertNotSame("", freezeFrame.dtc);
}
}
}