From ed3f6793aaf8f38f3bb702d00cd1e25baa6d1a1b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 29 Jun 2023 18:17:00 +0000 Subject: Replace isWeaverAvailable() with getWeaverService() isWeaverAvailable() has to be called before weaverVerify() and weaverEnroll(); otherwise mWeaver might still be null. However, this was not enforced and was easy to forget to do. This CL therefore changes the design slightly to make weaverVerify() and weaverEnroll() take IWeaver as an argument. To get the IWeaver, getWeaverService() is called, taking the place of isWeaverAvailable(). Note that this is analogous to LockSettingsService#getGateKeeperService. Bug: 287705522 Test: Treehugger Merged-In: I70c26232a9b300a7e6e2c815db6c143836c42ec3 Change-Id: I70c26232a9b300a7e6e2c815db6c143836c42ec3 (cherry picked from commit 922aaa24fac9be3c62439d9ad1faa81e38b5a1b3) --- .../locksettings/SyntheticPasswordManager.java | 68 +++++++++++++--------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index e8fd6f88359c..f00ff8f2226e 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -531,7 +531,7 @@ class SyntheticPasswordManager { } } - private IWeaver getWeaverService() { + private @Nullable IWeaver getWeaverServiceInternal() { // Try to get the AIDL service first try { IWeaver aidlWeaver = IWeaver.Stub.asInterface( @@ -563,15 +563,20 @@ class SyntheticPasswordManager { return LockPatternUtils.isAutoPinConfirmFeatureAvailable(); } - private synchronized boolean isWeaverAvailable() { - if (mWeaver != null) { - return true; + /** + * Returns a handle to the Weaver service, or null if Weaver is unavailable. Note that not all + * devices support Weaver. + */ + private synchronized @Nullable IWeaver getWeaverService() { + IWeaver weaver = mWeaver; + if (weaver != null) { + return weaver; } // Re-initialize weaver in case there was a transient error preventing access to it. - IWeaver weaver = getWeaverService(); + weaver = getWeaverServiceInternal(); if (weaver == null) { - return false; + return null; } final WeaverConfig weaverConfig; @@ -579,19 +584,18 @@ class SyntheticPasswordManager { weaverConfig = weaver.getConfig(); } catch (RemoteException | ServiceSpecificException e) { Slog.e(TAG, "Failed to get weaver config", e); - return false; + return null; } if (weaverConfig == null || weaverConfig.slots <= 0) { Slog.e(TAG, "Invalid weaver config"); - return false; + return null; } mWeaver = weaver; mWeaverConfig = weaverConfig; mPasswordSlotManager.refreshActiveSlots(getUsedWeaverSlots()); Slog.i(TAG, "Weaver service initialized"); - - return true; + return weaver; } /** @@ -601,7 +605,7 @@ class SyntheticPasswordManager { * * @return the value stored in the weaver slot, or null if the operation fails */ - private byte[] weaverEnroll(int slot, byte[] key, @Nullable byte[] value) { + private byte[] weaverEnroll(IWeaver weaver, int slot, byte[] key, @Nullable byte[] value) { if (slot == INVALID_WEAVER_SLOT || slot >= mWeaverConfig.slots) { throw new IllegalArgumentException("Invalid slot for weaver"); } @@ -614,7 +618,7 @@ class SyntheticPasswordManager { value = SecureRandomUtils.randomBytes(mWeaverConfig.valueSize); } try { - mWeaver.write(slot, key, value); + weaver.write(slot, key, value); } catch (RemoteException e) { Slog.e(TAG, "weaver write binder call failed, slot: " + slot, e); return null; @@ -643,7 +647,7 @@ class SyntheticPasswordManager { * the verification is successful, throttled or failed. If successful, the bound secret * is also returned. */ - private VerifyCredentialResponse weaverVerify(int slot, byte[] key) { + private VerifyCredentialResponse weaverVerify(IWeaver weaver, int slot, byte[] key) { if (slot == INVALID_WEAVER_SLOT || slot >= mWeaverConfig.slots) { throw new IllegalArgumentException("Invalid slot for weaver"); } @@ -654,7 +658,7 @@ class SyntheticPasswordManager { } final WeaverReadResponse readResponse; try { - readResponse = mWeaver.read(slot, key); + readResponse = weaver.read(slot, key); } catch (RemoteException e) { Slog.e(TAG, "weaver read failed, slot: " + slot, e); return VerifyCredentialResponse.ERROR; @@ -846,14 +850,15 @@ class SyntheticPasswordManager { int slot = loadWeaverSlot(protectorId, userId); destroyState(WEAVER_SLOT_NAME, protectorId, userId); if (slot != INVALID_WEAVER_SLOT) { - if (!isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver == null) { Slog.e(TAG, "Cannot erase Weaver slot because Weaver is unavailable"); return; } Set usedSlots = getUsedWeaverSlots(); if (!usedSlots.contains(slot)) { Slogf.i(TAG, "Erasing Weaver slot %d", slot); - weaverEnroll(slot, null, null); + weaverEnroll(weaver, slot, null, null); mPasswordSlotManager.markSlotDeleted(slot); } else { Slogf.i(TAG, "Weaver slot %d was already reused; not erasing it", slot); @@ -931,13 +936,14 @@ class SyntheticPasswordManager { Slogf.i(TAG, "Creating LSKF-based protector %016x for user %d", protectorId, userId); - if (isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver != null) { // Weaver is available, so make the protector use it to verify the LSKF. Do this even // if the LSKF is empty, as that gives us support for securely deleting the protector. int weaverSlot = getNextAvailableWeaverSlot(); Slogf.i(TAG, "Enrolling LSKF for user %d into Weaver slot %d", userId, weaverSlot); - byte[] weaverSecret = weaverEnroll(weaverSlot, stretchedLskfToWeaverKey(stretchedLskf), - null); + byte[] weaverSecret = weaverEnroll(weaver, weaverSlot, + stretchedLskfToWeaverKey(stretchedLskf), null); if (weaverSecret == null) { throw new IllegalStateException( "Fail to enroll user password under weaver " + userId); @@ -1024,7 +1030,8 @@ class SyntheticPasswordManager { } return VerifyCredentialResponse.fromGateKeeperResponse(response); } else if (persistentData.type == PersistentData.TYPE_SP_WEAVER) { - if (!isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver == null) { Slog.e(TAG, "No weaver service to verify SP-based FRP credential"); return VerifyCredentialResponse.ERROR; } @@ -1032,7 +1039,8 @@ class SyntheticPasswordManager { byte[] stretchedLskf = stretchLskf(userCredential, pwd); int weaverSlot = persistentData.userId; - return weaverVerify(weaverSlot, stretchedLskfToWeaverKey(stretchedLskf)).stripPayload(); + return weaverVerify(weaver, weaverSlot, + stretchedLskfToWeaverKey(stretchedLskf)).stripPayload(); } else { Slog.e(TAG, "persistentData.type must be TYPE_SP_GATEKEEPER or TYPE_SP_WEAVER, but is " + persistentData.type); @@ -1134,7 +1142,7 @@ class SyntheticPasswordManager { TokenData tokenData = new TokenData(); tokenData.mType = type; final byte[] secdiscardable = SecureRandomUtils.randomBytes(SECDISCARDABLE_LENGTH); - if (isWeaverAvailable()) { + if (getWeaverService() != null) { tokenData.weaverSecret = SecureRandomUtils.randomBytes(mWeaverConfig.valueSize); tokenData.secdiscardableOnDisk = SyntheticPasswordCrypto.encrypt(tokenData.weaverSecret, PERSONALIZATION_WEAVER_TOKEN, secdiscardable); @@ -1177,10 +1185,11 @@ class SyntheticPasswordManager { return false; } Slogf.i(TAG, "Creating token-based protector %016x for user %d", tokenHandle, userId); - if (isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver != null) { int slot = getNextAvailableWeaverSlot(); Slogf.i(TAG, "Using Weaver slot %d for new token-based protector", slot); - if (weaverEnroll(slot, null, tokenData.weaverSecret) == null) { + if (weaverEnroll(weaver, slot, null, tokenData.weaverSecret) == null) { Slog.e(TAG, "Failed to enroll weaver secret when activating token"); return false; } @@ -1269,12 +1278,14 @@ class SyntheticPasswordManager { int weaverSlot = loadWeaverSlot(protectorId, userId); if (weaverSlot != INVALID_WEAVER_SLOT) { // Protector uses Weaver to verify the LSKF - if (!isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver == null) { Slog.e(TAG, "Protector uses Weaver, but Weaver is unavailable"); result.gkResponse = VerifyCredentialResponse.ERROR; return result; } - result.gkResponse = weaverVerify(weaverSlot, stretchedLskfToWeaverKey(stretchedLskf)); + result.gkResponse = weaverVerify(weaver, weaverSlot, + stretchedLskfToWeaverKey(stretchedLskf)); if (result.gkResponse.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) { return result; } @@ -1442,12 +1453,13 @@ class SyntheticPasswordManager { } int slotId = loadWeaverSlot(protectorId, userId); if (slotId != INVALID_WEAVER_SLOT) { - if (!isWeaverAvailable()) { + final IWeaver weaver = getWeaverService(); + if (weaver == null) { Slog.e(TAG, "Protector uses Weaver, but Weaver is unavailable"); result.gkResponse = VerifyCredentialResponse.ERROR; return result; } - VerifyCredentialResponse response = weaverVerify(slotId, null); + VerifyCredentialResponse response = weaverVerify(weaver, slotId, null); if (response.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK || response.getGatekeeperHAT() == null) { Slog.e(TAG, -- cgit v1.2.3-59-g8ed1b From 59a58494d92fff2bfcd061a1f9df623f625e53d2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 29 Jun 2023 18:17:02 +0000 Subject: Reconnect if weaver service connection dies For robustness reasons, reconnect to the Weaver service if it dies. This should not be relied on, but it's easy to do and is the same thing that LockSettingsService does for the Gatekeeper service. Bug: 287705522 Test: Treehugger Merged-In: Ic06c45353c6b7282e2f22eef13ea1c6e36fe5c58 Change-Id: Ic06c45353c6b7282e2f22eef13ea1c6e36fe5c58 (cherry picked from commit 5b21e30b66567881f40bd531d5d1750b1b987cc7) --- .../locksettings/SyntheticPasswordManager.java | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index f00ff8f2226e..a95b968b0bcf 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -31,6 +31,7 @@ import android.hardware.weaver.IWeaver; import android.hardware.weaver.WeaverConfig; import android.hardware.weaver.WeaverReadResponse; import android.hardware.weaver.WeaverReadStatus; +import android.os.IBinder; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.ServiceManager; @@ -500,7 +501,7 @@ class SyntheticPasswordManager { private final Context mContext; private LockSettingsStorage mStorage; - private IWeaver mWeaver; + private volatile IWeaver mWeaver; private WeaverConfig mWeaverConfig; private PasswordSlotManager mPasswordSlotManager; @@ -531,6 +532,21 @@ class SyntheticPasswordManager { } } + private class WeaverDiedRecipient implements IBinder.DeathRecipient { + // Not synchronized on the outer class, since setting the pointer to null is atomic, and we + // don't want to have to worry about any sort of deadlock here. + @Override + public void binderDied() { + // Weaver died. Try to recover by setting mWeaver to null, which makes + // getWeaverService() look up the service again. This is done only as a simple + // robustness measure; it should not be relied on. If this triggers, the root cause is + // almost certainly a bug in the device's Weaver implementation, which must be fixed. + Slog.wtf(TAG, "Weaver service has died"); + mWeaver.asBinder().unlinkToDeath(this, 0); + mWeaver = null; + } + } + private @Nullable IWeaver getWeaverServiceInternal() { // Try to get the AIDL service first try { @@ -538,6 +554,11 @@ class SyntheticPasswordManager { ServiceManager.waitForDeclaredService(IWeaver.DESCRIPTOR + "/default")); if (aidlWeaver != null) { Slog.i(TAG, "Using AIDL weaver service"); + try { + aidlWeaver.asBinder().linkToDeath(new WeaverDiedRecipient(), 0); + } catch (RemoteException e) { + Slog.w(TAG, "Unable to register Weaver death recipient", e); + } return aidlWeaver; } } catch (SecurityException e) { -- cgit v1.2.3-59-g8ed1b From 456cd05c7e547bf32ae0e327c1118f2b3d016653 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Tue, 22 Aug 2023 18:04:10 +0800 Subject: locksettings: Don't use AIDL V1 weaver V1 and V2 reports IWeaver.read() errors differently. V1 throws ServiceSpecificException wrapping an error code. V2 returns the error code in the output parcel. The current client code expects the weaver HAL to be V2 or newer, however it doesn't check if the actual service version is V2 or not. If the service is V1, then IWeaver.read() errors would become unhandled exception, crashing the system_server. Since Weaver AIDL V1 is broken and should never be used, don't use Weaver AIDL V1 services. This fixes a regression in Android 14 where AIDL V1 started being used when available. Bug: 296984182 Bug: 296512452 Test: Boot on old (tm) vendor Merged-In: I32306fb8473c655e68d89d63a1e4f00c8bb5d61f Change-Id: I32306fb8473c655e68d89d63a1e4f00c8bb5d61f (cherry picked from commit 048aaf42d7073d2c45377154badbe3732103047b) --- .../locksettings/SyntheticPasswordManager.java | 53 ++++++++++++++++------ 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index a95b968b0bcf..11c0f5183e21 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -72,7 +72,6 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; - /** * A class that manages a user's synthetic password (SP) ({@link #SyntheticPassword}), along with a * set of SP protectors that are independent ways that the SP is protected. @@ -547,22 +546,48 @@ class SyntheticPasswordManager { } } - private @Nullable IWeaver getWeaverServiceInternal() { - // Try to get the AIDL service first + private @Nullable IWeaver getWeaverAidlService() { + final IWeaver aidlWeaver; try { - IWeaver aidlWeaver = IWeaver.Stub.asInterface( - ServiceManager.waitForDeclaredService(IWeaver.DESCRIPTOR + "/default")); - if (aidlWeaver != null) { - Slog.i(TAG, "Using AIDL weaver service"); - try { - aidlWeaver.asBinder().linkToDeath(new WeaverDiedRecipient(), 0); - } catch (RemoteException e) { - Slog.w(TAG, "Unable to register Weaver death recipient", e); - } - return aidlWeaver; - } + aidlWeaver = + IWeaver.Stub.asInterface( + ServiceManager.waitForDeclaredService(IWeaver.DESCRIPTOR + "/default")); } catch (SecurityException e) { Slog.w(TAG, "Does not have permissions to get AIDL weaver service"); + return null; + } + if (aidlWeaver == null) { + return null; + } + final int aidlVersion; + try { + aidlVersion = aidlWeaver.getInterfaceVersion(); + } catch (RemoteException e) { + Slog.e(TAG, "Cannot get AIDL weaver service version", e); + return null; + } + if (aidlVersion < 2) { + Slog.w(TAG, + "Ignoring AIDL weaver service v" + + aidlVersion + + " because only v2 and later are supported"); + return null; + } + Slog.i(TAG, "Found AIDL weaver service v" + aidlVersion); + return aidlWeaver; + } + + private @Nullable IWeaver getWeaverServiceInternal() { + // Try to get the AIDL service first + IWeaver aidlWeaver = getWeaverAidlService(); + if (aidlWeaver != null) { + Slog.i(TAG, "Using AIDL weaver service"); + try { + aidlWeaver.asBinder().linkToDeath(new WeaverDiedRecipient(), 0); + } catch (RemoteException e) { + Slog.w(TAG, "Unable to register Weaver death recipient", e); + } + return aidlWeaver; } // If the AIDL service can't be found, look for the HIDL service -- cgit v1.2.3-59-g8ed1b