Fix updating shared prefs with enrollment information.
1. Replace apply() with commit() for shared preference updates.
Sometimes, the writes were not getting to persistent memory with
apply(). Since the availability of the enrollment related information
is critical to the functioning of the trusted device, change all apply()
to commit() and terminate enrollment on commit failure.
2. Handle various failures during enrollment and report it to the
registered clients as appropriate.
This fixes the problem where a Trusted device cannot be added after
removal.
Bug: 133351179
Test: Add, remove and add the same phone as a trusted device.
Change-Id: I78b0e9e279134c47e3d0e3572087caf9867648e4
diff --git a/service/src/com/android/car/trust/CarTrustAgentEnrollmentService.java b/service/src/com/android/car/trust/CarTrustAgentEnrollmentService.java
index 812cb63..cb0f908 100644
--- a/service/src/com/android/car/trust/CarTrustAgentEnrollmentService.java
+++ b/service/src/com/android/car/trust/CarTrustAgentEnrollmentService.java
@@ -16,6 +16,7 @@
package com.android.car.trust;
+import static android.car.trust.CarTrustAgentEnrollmentManager.ENROLLMENT_HANDSHAKE_FAILURE;
import static android.car.trust.CarTrustAgentEnrollmentManager.ENROLLMENT_NOT_ALLOWED;
import android.app.ActivityManager;
@@ -134,7 +135,6 @@
client.mListenerBinder.unlinkToDeath(client, 0);
}
mEnrollmentStateClients.clear();
- setEnrollmentHandshakeAccepted(false);
}
// Implementing the ICarTrustAgentEnrollment interface
@@ -148,13 +148,7 @@
if (!mTrustedDeviceService.getSharedPrefs()
.getBoolean(TRUSTED_DEVICE_ENROLLMENT_ENABLED_KEY, true)) {
Log.e(TAG, "Trusted Device Enrollment disabled");
- for (EnrollmentStateClient client : mEnrollmentStateClients) {
- try {
- client.mListener.onEnrollmentHandshakeFailure(null, ENROLLMENT_NOT_ALLOWED);
- } catch (RemoteException e) {
- Log.e(TAG, "onEnrollmentHandshakeFailure dispatch failed", e);
- }
- }
+ dispatchEnrollmentFailure(ENROLLMENT_NOT_ALLOWED);
return;
}
// Stop any current broadcasts
@@ -191,7 +185,7 @@
addEnrollmentServiceLog("enrollmentHandshakeAccepted");
mCarTrustAgentBleManager.sendMessage(device, CONFIRMATION_SIGNAL,
OperationType.ENCRYPTION_HANDSHAKE, /* isPayloadEncrypted= */ false);
- setEnrollmentHandshakeAccepted(true);
+ setEnrollmentHandshakeAccepted();
}
/**
@@ -201,7 +195,6 @@
*/
@Override
public void terminateEnrollmentHandshake() {
- setEnrollmentHandshakeAccepted(false);
addEnrollmentServiceLog("terminateEnrollmentHandshake");
// Disconnect from BLE
mCarTrustAgentBleManager.disconnectRemoteDevice();
@@ -269,7 +262,10 @@
public void setTrustedDeviceEnrollmentEnabled(boolean isEnabled) {
SharedPreferences.Editor editor = mTrustedDeviceService.getSharedPrefs().edit();
editor.putBoolean(TRUSTED_DEVICE_ENROLLMENT_ENABLED_KEY, isEnabled);
- editor.apply();
+ if (!editor.commit()) {
+ Log.wtf(TAG,
+ "Enrollment Failure: Commit to SharedPreferences failed. Enable? " + isEnabled);
+ }
}
/**
@@ -340,10 +336,10 @@
if (mRemoteEnrollmentDevice == null) {
Log.e(TAG, "onEscrowTokenAdded() but no remote device connected!");
removeEscrowToken(handle, uid);
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
return;
}
- // To conveniently get the user id to unlock when handle is received.
- mTrustedDeviceService.getSharedPrefs().edit().putInt(String.valueOf(handle), uid).apply();
+
mTokenActiveStateMap.put(handle, false);
for (EnrollmentStateClient client : mEnrollmentStateClients) {
try {
@@ -372,14 +368,18 @@
Iterator<String> iterator = deviceInfos.iterator();
while (iterator.hasNext()) {
String deviceInfoString = iterator.next();
- if (TrustedDeviceInfo.deserialize(deviceInfoString).getHandle()
- == handle) {
+ if (TrustedDeviceInfo.deserialize(deviceInfoString).getHandle() == handle) {
+ if (Log.isLoggable(TAG, Log.DEBUG)) {
+ Log.d(TAG, "Removing trusted device handle: " + handle);
+ }
iterator.remove();
break;
}
}
editor.putStringSet(String.valueOf(uid), deviceInfos);
- editor.apply();
+ if (!editor.commit()) {
+ Log.e(TAG, "EscrowToken removed, but shared prefs update failed");
+ }
}
/**
@@ -391,36 +391,65 @@
if (Log.isLoggable(TAG, Log.DEBUG)) {
Log.d(TAG, "onEscrowTokenActiveStateChanged: " + Long.toHexString(handle));
}
- mTokenActiveStateMap.put(handle, isTokenActive);
- dispatchEscrowTokenActiveStateChanged(handle, isTokenActive);
- if (isTokenActive) {
- Set<String> deviceInfo = mTrustedDeviceService.getSharedPrefs().getStringSet(
- String.valueOf(uid), new HashSet<>());
- String deviceName;
- if (mRemoteEnrollmentDevice.getName() != null) {
- deviceName = mRemoteEnrollmentDevice.getName();
- } else if (mDeviceName != null) {
- deviceName = mDeviceName;
- } else {
- deviceName = mContext.getString(R.string.trust_device_default_name);
+ if (mRemoteEnrollmentDevice == null || !isTokenActive) {
+ if (mRemoteEnrollmentDevice == null) {
+ Log.e(TAG,
+ "Device disconnected before sending back handle. Enrollment incomplete");
}
- addEnrollmentServiceLog("trustedDeviceAdded (handle:" + handle + ", uid:" + uid
- + ", addr:" + mRemoteEnrollmentDevice.getAddress()
- + ", name:" + deviceName + ")");
- deviceInfo.add(new TrustedDeviceInfo(handle, mRemoteEnrollmentDevice.getAddress(),
- deviceName).serialize());
- // To conveniently get the devices info regarding certain user.
- mTrustedDeviceService.getSharedPrefs().edit().putStringSet(String.valueOf(uid),
- deviceInfo).apply();
- if (Log.isLoggable(TAG, Log.DEBUG)) {
- Log.d(TAG, "Sending handle: " + handle);
+ if (!isTokenActive) {
+ Log.e(TAG, "Unexpected: Escrow Token activation failed");
}
- mCarTrustAgentBleManager.sendMessage(mRemoteEnrollmentDevice,
- mEncryptionKey.encryptData(Utils.longToBytes(handle)),
- OperationType.CLIENT_MESSAGE, /* isPayloadEncrypted= */ true);
- } else {
removeEscrowToken(handle, uid);
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
+ return;
}
+
+ mTokenActiveStateMap.put(handle, isTokenActive);
+ Set<String> deviceInfo = mTrustedDeviceService.getSharedPrefs().getStringSet(
+ String.valueOf(uid), new HashSet<>());
+ String deviceName;
+ if (mRemoteEnrollmentDevice.getName() != null) {
+ deviceName = mRemoteEnrollmentDevice.getName();
+ } else if (mDeviceName != null) {
+ deviceName = mDeviceName;
+ } else {
+ deviceName = mContext.getString(R.string.trust_device_default_name);
+ }
+ StringBuffer log = new StringBuffer()
+ .append("trustedDeviceAdded (handle:").append(handle)
+ .append(", uid:").append(uid)
+ .append(", addr:").append(mRemoteEnrollmentDevice.getAddress())
+ .append(", name:").append(deviceName).append(")");
+ addEnrollmentServiceLog(log.toString());
+ deviceInfo.add(new TrustedDeviceInfo(handle, mRemoteEnrollmentDevice.getAddress(),
+ deviceName).serialize());
+
+ // To conveniently get the devices info regarding certain user.
+ SharedPreferences.Editor editor = mTrustedDeviceService.getSharedPrefs().edit();
+ editor.putStringSet(String.valueOf(uid), deviceInfo);
+ if (!editor.commit()) {
+ Log.e(TAG, "Writing DeviceInfo to shared prefs Failed");
+ removeEscrowToken(handle, uid);
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
+ return;
+ }
+
+ // To conveniently get the user id to unlock when handle is received.
+ editor.putInt(String.valueOf(handle), uid);
+ if (!editor.commit()) {
+ Log.e(TAG, "Writing (handle, uid) to shared prefs Failed");
+ removeEscrowToken(handle, uid);
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
+ return;
+ }
+
+ if (Log.isLoggable(TAG, Log.DEBUG)) {
+ Log.d(TAG, "Sending handle: " + handle);
+ }
+ mCarTrustAgentBleManager.sendMessage(mRemoteEnrollmentDevice,
+ mEncryptionKey.encryptData(Utils.longToBytes(handle)),
+ OperationType.CLIENT_MESSAGE, /* isPayloadEncrypted= */ true);
+ dispatchEscrowTokenActiveStateChanged(handle, isTokenActive);
}
void onEnrollmentAdvertiseStartSuccess() {
@@ -444,9 +473,8 @@
}
void onRemoteDeviceConnected(BluetoothDevice device) {
- resetEncryptionState();
-
addEnrollmentServiceLog("onRemoteDeviceConnected (addr:" + device.getAddress() + ")");
+ resetEncryptionState();
synchronized (mRemoteDeviceLock) {
mRemoteEnrollmentDevice = device;
}
@@ -454,16 +482,21 @@
try {
client.mListener.onBleEnrollmentDeviceConnected(device);
} catch (RemoteException e) {
- Log.e(TAG, "onAdvertiseSuccess dispatch failed", e);
+ Log.e(TAG, "onRemoteDeviceConnected dispatch failed", e);
}
}
mCarTrustAgentBleManager.stopEnrollmentAdvertising();
}
void onRemoteDeviceDisconnected(BluetoothDevice device) {
- resetEncryptionState();
-
+ if (Log.isLoggable(TAG, Log.DEBUG)) {
+ Log.d(TAG, "Device Disconnected: " + device.getAddress() + " Enrollment State: "
+ + mEnrollmentState + " Encryption State: " + mEncryptionState);
+ }
addEnrollmentServiceLog("onRemoteDeviceDisconnected (addr:" + device.getAddress() + ")");
+ addEnrollmentServiceLog(
+ "Enrollment State: " + mEnrollmentState + " EncryptionState: " + mEncryptionState);
+ resetEncryptionState();
synchronized (mRemoteDeviceLock) {
mRemoteEnrollmentDevice = null;
}
@@ -471,7 +504,7 @@
try {
client.mListener.onBleEnrollmentDeviceDisconnected(device);
} catch (RemoteException e) {
- Log.e(TAG, "onAdvertiseSuccess dispatch failed", e);
+ Log.e(TAG, "onRemoteDeviceDisconnected dispatch failed", e);
}
}
}
@@ -521,6 +554,12 @@
Log.d(TAG, "Received device id: " + mDeviceId);
}
UUID uniqueId = mTrustedDeviceService.getUniqueId();
+ if (uniqueId == null) {
+ Log.e(TAG, "Cannot get Unique ID for the IHU");
+ resetEnrollmentStateOnFailure();
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
+ return;
+ }
if (Log.isLoggable(TAG, Log.DEBUG)) {
Log.d(TAG, "Sending device id: " + uniqueId.toString());
}
@@ -628,36 +667,44 @@
}
/**
+ * Reset the whole enrollment state. Disconnects the peer device and removes any escrow token
+ * that has not been activated.
+ *
+ * <p>This method should be called from any stage in the middle of enrollment where we
+ * encounter a failure.
+ */
+ private void resetEnrollmentStateOnFailure() {
+ terminateEnrollmentHandshake();
+ resetEncryptionState();
+ }
+
+ /**
* Resets the encryption status of this service.
*
* <p>This method should be called each time a device connects so that a new handshake can be
* started and encryption keys exchanged.
*/
private void resetEncryptionState() {
- setEnrollmentHandshakeAccepted(false);
-
mEncryptionRunner = EncryptionRunnerFactory.newRunner();
mHandshakeMessage = null;
mEncryptionKey = null;
mEncryptionState = HandshakeState.UNKNOWN;
+ mEnrollmentState = ENROLLMENT_STATE_NONE;
}
- private synchronized void setEnrollmentHandshakeAccepted(boolean accepted) {
- if (!accepted) {
- return;
- }
-
+ private synchronized void setEnrollmentHandshakeAccepted() {
if (mEncryptionRunner == null) {
Log.e(TAG, "Received notification that enrollment handshake was accepted, "
+ "but encryption was never set up.");
return;
}
-
HandshakeMessage message;
try {
message = mEncryptionRunner.verifyPin();
} catch (HandshakeException e) {
Log.e(TAG, "Error during PIN verification", e);
+ resetEnrollmentStateOnFailure();
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
return;
}
@@ -669,9 +716,12 @@
mEncryptionState = HandshakeState.FINISHED;
mEncryptionKey = message.getKey();
- mTrustedDeviceService.saveEncryptionKey(mDeviceId, mEncryptionKey.asBytes());
+ if (!mTrustedDeviceService.saveEncryptionKey(mDeviceId, mEncryptionKey.asBytes())) {
+ resetEnrollmentStateOnFailure();
+ dispatchEnrollmentFailure(ENROLLMENT_HANDSHAKE_FAILURE);
+ return;
+ }
mEnrollmentState++;
-
}
/**
@@ -843,6 +893,16 @@
}
}
+ private void dispatchEnrollmentFailure(int error) {
+ for (EnrollmentStateClient client : mEnrollmentStateClients) {
+ try {
+ client.mListener.onEnrollmentHandshakeFailure(null, error);
+ } catch (RemoteException e) {
+ Log.e(TAG, "onEnrollmentHandshakeFailure dispatch failed", e);
+ }
+ }
+ }
+
/**
* Class that holds onto client related information - listener interface, process that hosts the
* binder object etc.
diff --git a/service/src/com/android/car/trust/CarTrustAgentUnlockService.java b/service/src/com/android/car/trust/CarTrustAgentUnlockService.java
index e8a0afd..8808f50 100644
--- a/service/src/com/android/car/trust/CarTrustAgentUnlockService.java
+++ b/service/src/com/android/car/trust/CarTrustAgentUnlockService.java
@@ -83,7 +83,9 @@
public void setTrustedDeviceUnlockEnabled(boolean isEnabled) {
SharedPreferences.Editor editor = mTrustedDeviceService.getSharedPrefs().edit();
editor.putBoolean(TRUSTED_DEVICE_UNLOCK_ENABLED_KEY, isEnabled);
- editor.apply();
+ if (!editor.commit()) {
+ Log.wtf(TAG, "Unlock Enable Failed. Enable? " + isEnabled);
+ }
}
/**
* Set a delegate that implements {@link CarTrustAgentUnlockDelegate}. The delegate will be
diff --git a/service/src/com/android/car/trust/CarTrustedDeviceService.java b/service/src/com/android/car/trust/CarTrustedDeviceService.java
index 2948bec..9995aa7 100644
--- a/service/src/com/android/car/trust/CarTrustedDeviceService.java
+++ b/service/src/com/android/car/trust/CarTrustedDeviceService.java
@@ -202,8 +202,9 @@
}
} else {
mUniqueId = UUID.randomUUID();
- prefs.edit().putString(UNIQUE_ID_KEY, mUniqueId.toString()).commit();
- if (Log.isLoggable(TAG, Log.DEBUG)) {
+ if (!prefs.edit().putString(UNIQUE_ID_KEY, mUniqueId.toString()).commit()) {
+ mUniqueId = null;
+ } else if (Log.isLoggable(TAG, Log.DEBUG)) {
Log.d(TAG, "Generated new trusted unique id: "
+ prefs.getString(UNIQUE_ID_KEY, ""));
}
@@ -235,14 +236,19 @@
*
* @param deviceId did of trusted device
* @param encryptionKey encryption key
+ * @return {@code true} if the operation succeeded
*/
- void saveEncryptionKey(String deviceId, byte[] encryptionKey) {
+ boolean saveEncryptionKey(String deviceId, byte[] encryptionKey) {
byte[] encryptedKey = encryptWithKeyStore(KEY_ALIAS, encryptionKey);
- getSharedPrefs()
+ if (encryptedKey == null) {
+ return false;
+ }
+
+ return getSharedPrefs()
.edit()
.putString(PREF_ENCRYPTION_KEY_PREFIX + deviceId,
Base64.encodeToString(encryptedKey, Base64.DEFAULT))
- .apply();
+ .commit();
}
/**