From e7694cc54cd20c8d385efc639e0f658896a4beb6 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 22 Jul 2020 15:16:20 -0700 Subject: [PATCH 1/5] 1/n: Allow LockSettingsService to return Gatekeeper Password For certain scenarios, it's ideal if a single prompt for the user's credential could generate multiple Gatekeeper HATs, each containing a distinct challenge. To do so, we expose the gatekeeper password to the caller, which can then be sent to LockSettingsService to mint a Gatekeeper HAT with a challenge specified by the caller. Functionally, this is split into two pieces: 1) ILockSettings#verifyCredential* has a new flags parameter, which if contains VERIFY_FLAG_RETURN_GK_PW, returns the gatekeeper password 2) ILockSettings introduces a new method, verifyGatekeeperPassword, which takes the Gatekeeper Password and challenge, from which Gatekeeper creates a HardwareAuthToken. This is different than the rest of spBasedDoverifyCredential and __only__ requests Gatekeeper to create the HardwareAuthToken. It does not proceed to do other things such as unlocking keystore keys, unlocking managed profiles, etc. Slightly cleaned up VerifyCredentialResponse: moved to builder pattern, cleaned up serialization/deserialization Returning a VerifyCredentialResponse object (instead of a byte[]) also makes it easier to debug failure cases (e.g. credential was verified but HAT was null, vs originally we have no idea). Similarly, this allows us to remove RequestThrottledException, which can help make it easier to reason about code flow (less unexpected nullness) Test: Clients with VERIFY_FLAG_RETURN_GK_PW have correct "accept, reject, timeout" behavior Test: Current biometric enrollment works and not affected Test: PIN/Pattern/Password verifyGatekeeperPassword works (see ag/12222644) Bug: 161765592 Change-Id: I6e2a7ea234aac1a278b35cdaff62b1c7e3e9f205 --- .../internal/widget/ILockSettings.aidl | 5 +- .../internal/widget/LockPatternChecker.java | 46 ++--- .../internal/widget/LockPatternUtils.java | 85 +++++++-- .../internal/widget/LockscreenCredential.java | 2 +- .../widget/VerifyCredentialResponse.java | 164 +++++++++++------- .../AuthCredentialPasswordView.java | 14 +- .../biometrics/AuthCredentialPatternView.java | 9 +- .../biometrics/AuthCredentialView.java | 16 +- .../locksettings/LockSettingsService.java | 88 +++++++--- .../SyntheticPasswordManager.java | 37 ++-- 10 files changed, 311 insertions(+), 155 deletions(-) diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index e35fda1ee76d8..a5a42e1afcdc4 100644 --- a/core/java/com/android/internal/widget/ILockSettings.aidl +++ b/core/java/com/android/internal/widget/ILockSettings.aidl @@ -47,8 +47,9 @@ interface ILockSettings { void resetKeyStore(int userId); VerifyCredentialResponse checkCredential(in LockscreenCredential credential, int userId, in ICheckCredentialProgressCallback progressCallback); - VerifyCredentialResponse verifyCredential(in LockscreenCredential credential, long challenge, int userId); - VerifyCredentialResponse verifyTiedProfileChallenge(in LockscreenCredential credential, long challenge, int userId); + VerifyCredentialResponse verifyCredential(in LockscreenCredential credential, long challenge, int userId, int flags); + VerifyCredentialResponse verifyTiedProfileChallenge(in LockscreenCredential credential, long challenge, int userId, int flags); + VerifyCredentialResponse verifyGatekeeperPassword(in byte[] gatekeeperPassword, long challenge, int userId); boolean checkVoldPassword(int userId); int getCredentialType(int userId); byte[] getHashFactor(in LockscreenCredential currentCredential, int userId); diff --git a/core/java/com/android/internal/widget/LockPatternChecker.java b/core/java/com/android/internal/widget/LockPatternChecker.java index 85a45fd8e0c05..eaa29312b7590 100644 --- a/core/java/com/android/internal/widget/LockPatternChecker.java +++ b/core/java/com/android/internal/widget/LockPatternChecker.java @@ -1,5 +1,6 @@ package com.android.internal.widget; +import android.annotation.NonNull; import android.os.AsyncTask; import com.android.internal.widget.LockPatternUtils.RequestThrottledException; @@ -41,11 +42,11 @@ public final class LockPatternChecker { /** * Invoked when a security verification is finished. * - * @param attestation The attestation that the challenge was verified, or null. + * @param response The response, optionally containing Gatekeeper HAT or Gatekeeper Password * @param throttleTimeoutMs The amount of time in ms to wait before reattempting - * the call. Only non-0 if attestation is null. + * the call. Only non-0 if the response is {@link VerifyCredentialResponse#RESPONSE_RETRY}. */ - void onVerified(byte[] attestation, int throttleTimeoutMs); + void onVerified(@NonNull VerifyCredentialResponse response, int throttleTimeoutMs); } /** @@ -55,31 +56,31 @@ public final class LockPatternChecker { * @param credential The credential to check. * @param challenge The challenge to verify against the credential. * @param userId The user to check against the credential. + * @param flags See {@link LockPatternUtils.VerifyFlag} * @param callback The callback to be invoked with the verification result. */ public static AsyncTask verifyCredential(final LockPatternUtils utils, final LockscreenCredential credential, final long challenge, final int userId, + final @LockPatternUtils.VerifyFlag int flags, final OnVerifyCallback callback) { // Create a copy of the credential since checking credential is asynchrounous. final LockscreenCredential credentialCopy = credential.duplicate(); - AsyncTask task = new AsyncTask() { - private int mThrottleTimeout; - + AsyncTask task = + new AsyncTask() { @Override - protected byte[] doInBackground(Void... args) { + protected VerifyCredentialResponse doInBackground(Void... args) { try { - return utils.verifyCredential(credentialCopy, challenge, userId); + return utils.verifyCredential(credentialCopy, challenge, userId, flags); } catch (RequestThrottledException ex) { - mThrottleTimeout = ex.getTimeoutMs(); - return null; + return VerifyCredentialResponse.fromTimeout(ex.getTimeoutMs()); } } @Override - protected void onPostExecute(byte[] result) { - callback.onVerified(result, mThrottleTimeout); + protected void onPostExecute(@NonNull VerifyCredentialResponse result) { + callback.onVerified(result, result.getTimeout()); credentialCopy.zeroize(); } @@ -143,31 +144,32 @@ public final class LockPatternChecker { * @param credential The credential to check. * @param challenge The challenge to verify against the credential. * @param userId The user to check against the credential. + * @param flags See {@link LockPatternUtils.VerifyFlag} * @param callback The callback to be invoked with the verification result. */ public static AsyncTask verifyTiedProfileChallenge(final LockPatternUtils utils, final LockscreenCredential credential, final long challenge, final int userId, + final @LockPatternUtils.VerifyFlag int flags, final OnVerifyCallback callback) { - // Create a copy of the credential since checking credential is asynchrounous. + // Create a copy of the credential since checking credential is asynchronous. final LockscreenCredential credentialCopy = credential.duplicate(); - AsyncTask task = new AsyncTask() { - private int mThrottleTimeout; - + AsyncTask task = + new AsyncTask() { @Override - protected byte[] doInBackground(Void... args) { + protected VerifyCredentialResponse doInBackground(Void... args) { try { - return utils.verifyTiedProfileChallenge(credentialCopy, challenge, userId); + return utils.verifyTiedProfileChallenge(credentialCopy, challenge, userId, + flags); } catch (RequestThrottledException ex) { - mThrottleTimeout = ex.getTimeoutMs(); - return null; + return VerifyCredentialResponse.fromTimeout(ex.getTimeoutMs()); } } @Override - protected void onPostExecute(byte[] result) { - callback.onVerified(result, mThrottleTimeout); + protected void onPostExecute(@NonNull VerifyCredentialResponse response) { + callback.onVerified(response, response.getTimeout()); credentialCopy.zeroize(); } diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 93690cdfc811d..2379e7a550251 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -129,6 +129,18 @@ public class LockPatternUtils { }) public @interface CredentialType {} + /** + * Flag provided to {@link #verifyCredential(LockscreenCredential, long, int, int)} . If set, + * the method will return the Gatekeeper Password in the {@link VerifyCredentialResponse}. + */ + public static final int VERIFY_FLAG_RETURN_GK_PW = 1 << 0; + + @Retention(RetentionPolicy.SOURCE) + @IntDef(flag = true, value = { + VERIFY_FLAG_RETURN_GK_PW + }) + public @interface VerifyFlag {} + /** * Special user id for triggering the FRP verification flow. */ @@ -376,27 +388,56 @@ public class LockPatternUtils { * @param credential The credential to check. * @param challenge The challenge to verify against the credential * @param userId The user whose credential is being verified - * @return the attestation that the challenge was verified, or null + * @param flags See {@link VerifyFlag} * @throws RequestThrottledException if credential verification is being throttled due to * to many incorrect attempts. * @throws IllegalStateException if called on the main thread. + * + * TODO: This probably doesn't need to throw RequestThrottledException anymore, since the + * method now returns a VerifyCredentialResponse object, which contains the timeout */ - public byte[] verifyCredential(@NonNull LockscreenCredential credential, long challenge, - int userId) throws RequestThrottledException { + @NonNull + public VerifyCredentialResponse verifyCredential(@NonNull LockscreenCredential credential, + long challenge, int userId, @VerifyFlag int flags) throws RequestThrottledException { throwIfCalledOnMainThread(); try { - VerifyCredentialResponse response = getLockSettings().verifyCredential( - credential, challenge, userId); + final VerifyCredentialResponse response = getLockSettings().verifyCredential( + credential, challenge, userId, flags); + if (response == null) { + return VerifyCredentialResponse.ERROR; + } + if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { - return response.getPayload(); + return response; } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { throw new RequestThrottledException(response.getTimeout()); } else { - return null; + return VerifyCredentialResponse.ERROR; } } catch (RemoteException re) { Log.e(TAG, "failed to verify credential", re); - return null; + return VerifyCredentialResponse.ERROR; + } + } + + /** + * With the Gatekeeper Password returned via {@link #verifyCredential(LockscreenCredential, + * long, int, boolean)}, request Gatekeeper to create a HardwareAuthToken wrapping the + * given challenge. + */ + @NonNull + public VerifyCredentialResponse verifyGatekeeperPassword(@NonNull byte[] gatekeeperPassword, + long challenge, int userId) { + try { + final VerifyCredentialResponse response = getLockSettings().verifyGatekeeperPassword( + gatekeeperPassword, challenge, userId); + if (response == null) { + return VerifyCredentialResponse.ERROR; + } + return response; + } catch (RemoteException e) { + Log.e(TAG, "failed to verify gatekeeper password", e); + return VerifyCredentialResponse.ERROR; } } @@ -418,8 +459,9 @@ public class LockPatternUtils { try { VerifyCredentialResponse response = getLockSettings().checkCredential( credential, userId, wrapCallback(progressCallback)); - - if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { + if (response == null) { + return false; + } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { return true; } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { throw new RequestThrottledException(response.getTimeout()); @@ -442,27 +484,36 @@ public class LockPatternUtils { * @param challenge The challenge to verify against the credential * @return the attestation that the challenge was verified, or null * @param userId The managed profile user id + * @param flags See {@link VerifyFlag} * @throws RequestThrottledException if credential verification is being throttled due to * to many incorrect attempts. * @throws IllegalStateException if called on the main thread. + * + * TODO: This probably doesn't need to throw RequestThrottledException anymore, since the + * method now returns a VerifyCredentialResponse object, which contains the timeout */ - public byte[] verifyTiedProfileChallenge(@NonNull LockscreenCredential credential, - long challenge, int userId) throws RequestThrottledException { + @NonNull + public VerifyCredentialResponse verifyTiedProfileChallenge( + @NonNull LockscreenCredential credential, + long challenge, int userId, @VerifyFlag int flags) throws RequestThrottledException { throwIfCalledOnMainThread(); try { - VerifyCredentialResponse response = - getLockSettings().verifyTiedProfileChallenge(credential, challenge, userId); + final VerifyCredentialResponse response = getLockSettings() + .verifyTiedProfileChallenge(credential, challenge, userId, flags); + if (response == null) { + return VerifyCredentialResponse.ERROR; + } if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { - return response.getPayload(); + return response; } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { throw new RequestThrottledException(response.getTimeout()); } else { - return null; + return VerifyCredentialResponse.ERROR; } } catch (RemoteException re) { Log.e(TAG, "failed to verify tied profile credential", re); - return null; + return VerifyCredentialResponse.ERROR; } } diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 55f30fb892531..a488449db0199 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -48,7 +48,7 @@ import java.util.Objects; * // Process the credential in some way * } * - * With this construct, we can garantee that there will be no copies of the password left in + * With this construct, we can guarantee that there will be no copies of the password left in * memory when the credential goes out of scope. This should help mitigate certain class of * attacks where the attcker gains read-only access to full device memory (cold boot attack, * unsecured software/hardware memory dumping interfaces such as JTAG). diff --git a/core/java/com/android/internal/widget/VerifyCredentialResponse.java b/core/java/com/android/internal/widget/VerifyCredentialResponse.java index 7d1c706470921..e09eb42282190 100644 --- a/core/java/com/android/internal/widget/VerifyCredentialResponse.java +++ b/core/java/com/android/internal/widget/VerifyCredentialResponse.java @@ -16,11 +16,16 @@ package com.android.internal.widget; +import android.annotation.IntDef; +import android.annotation.Nullable; import android.os.Parcel; import android.os.Parcelable; import android.service.gatekeeper.GateKeeperResponse; import android.util.Slog; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + /** * Response object for a ILockSettings credential verification request. * @hide @@ -30,78 +35,114 @@ public final class VerifyCredentialResponse implements Parcelable { public static final int RESPONSE_ERROR = -1; public static final int RESPONSE_OK = 0; public static final int RESPONSE_RETRY = 1; + @IntDef({RESPONSE_ERROR, + RESPONSE_OK, + RESPONSE_RETRY}) + @Retention(RetentionPolicy.SOURCE) + @interface ResponseCode {} - public static final VerifyCredentialResponse OK = new VerifyCredentialResponse(); - public static final VerifyCredentialResponse ERROR - = new VerifyCredentialResponse(RESPONSE_ERROR, 0, null); + public static final VerifyCredentialResponse OK = new VerifyCredentialResponse.Builder() + .build(); + public static final VerifyCredentialResponse ERROR = fromError(); private static final String TAG = "VerifyCredentialResponse"; - private int mResponseCode; - private byte[] mPayload; - private int mTimeout; + private final @ResponseCode int mResponseCode; + private final int mTimeout; + @Nullable private final byte[] mGatekeeperHAT; + @Nullable private final byte[] mGatekeeperPw; public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { @Override public VerifyCredentialResponse createFromParcel(Parcel source) { - int responseCode = source.readInt(); - VerifyCredentialResponse response = new VerifyCredentialResponse(responseCode, 0, null); - if (responseCode == RESPONSE_RETRY) { - response.setTimeout(source.readInt()); - } else if (responseCode == RESPONSE_OK) { - int size = source.readInt(); - if (size > 0) { - byte[] payload = new byte[size]; - source.readByteArray(payload); - response.setPayload(payload); - } - } - return response; + final @ResponseCode int responseCode = source.readInt(); + final int timeout = source.readInt(); + final byte[] gatekeeperHAT = source.createByteArray(); + final byte[] gatekeeperPassword = source.createByteArray(); + + return new VerifyCredentialResponse(responseCode, timeout, gatekeeperHAT, + gatekeeperPassword); } @Override public VerifyCredentialResponse[] newArray(int size) { return new VerifyCredentialResponse[size]; } - }; - public VerifyCredentialResponse() { - mResponseCode = RESPONSE_OK; - mPayload = null; + public static class Builder { + @Nullable private byte[] mGatekeeperHAT; + @Nullable private byte[] mGatekeeperPassword; + + /** + * @param gatekeeperHAT Gatekeeper HardwareAuthToken, minted upon successful authentication. + */ + public Builder setGatekeeperHAT(byte[] gatekeeperHAT) { + mGatekeeperHAT = gatekeeperHAT; + return this; + } + + public Builder setGatekeeperPassword(byte[] gatekeeperPassword) { + mGatekeeperPassword = gatekeeperPassword; + return this; + } + + /** + * Builds a VerifyCredentialResponse with {@link #RESPONSE_OK} and any other parameters + * that were preveiously set. + * @return + */ + public VerifyCredentialResponse build() { + return new VerifyCredentialResponse(RESPONSE_OK, + 0 /* timeout */, + mGatekeeperHAT, + mGatekeeperPassword); + } } - - public VerifyCredentialResponse(byte[] payload) { - mPayload = payload; - mResponseCode = RESPONSE_OK; + /** + * Since timeouts are always an error, provide a way to create the VerifyCredentialResponse + * object directly. None of the other fields (Gatekeeper HAT, Gatekeeper Password, etc) + * are valid in this case. Similarly, the response code will always be + * {@link #RESPONSE_RETRY}. + */ + public static VerifyCredentialResponse fromTimeout(int timeout) { + return new VerifyCredentialResponse(RESPONSE_RETRY, + timeout, + null /* gatekeeperHAT */, + null /* gatekeeperPassword */); } - public VerifyCredentialResponse(int timeout) { - mTimeout = timeout; - mResponseCode = RESPONSE_RETRY; - mPayload = null; + /** + * Since error (incorrect password) should never result in any of the other fields from + * being populated, provide a default method to return a VerifyCredentialResponse. + */ + public static VerifyCredentialResponse fromError() { + return new VerifyCredentialResponse(RESPONSE_ERROR, + 0 /* timeout */, + null /* gatekeeperHAT */, + null /* gatekeeperPassword */); } - private VerifyCredentialResponse(int responseCode, int timeout, byte[] payload) { + private VerifyCredentialResponse(@ResponseCode int responseCode, int timeout, + @Nullable byte[] gatekeeperHAT, @Nullable byte[] gatekeeperPassword) { mResponseCode = responseCode; mTimeout = timeout; - mPayload = payload; + mGatekeeperHAT = gatekeeperHAT; + mGatekeeperPw = gatekeeperPassword; + } + + public VerifyCredentialResponse stripPayload() { + return new VerifyCredentialResponse(mResponseCode, mTimeout, + null /* gatekeeperHAT */, null /* gatekeeperPassword */); } @Override public void writeToParcel(Parcel dest, int flags) { dest.writeInt(mResponseCode); - if (mResponseCode == RESPONSE_RETRY) { - dest.writeInt(mTimeout); - } else if (mResponseCode == RESPONSE_OK) { - if (mPayload != null) { - dest.writeInt(mPayload.length); - dest.writeByteArray(mPayload); - } else { - dest.writeInt(0); - } - } + dest.writeInt(mTimeout); + dest.writeByteArray(mGatekeeperHAT); + dest.writeByteArray(mGatekeeperPw); } @Override @@ -109,48 +150,51 @@ public final class VerifyCredentialResponse implements Parcelable { return 0; } - public byte[] getPayload() { - return mPayload; + @Nullable + public byte[] getGatekeeperHAT() { + return mGatekeeperHAT; + } + + @Nullable + public byte[] getGatekeeperPw() { + return mGatekeeperPw; } public int getTimeout() { return mTimeout; } - public int getResponseCode() { + public @ResponseCode int getResponseCode() { return mResponseCode; } - private void setTimeout(int timeout) { - mTimeout = timeout; + public boolean isMatched() { + return mResponseCode == RESPONSE_OK; } - private void setPayload(byte[] payload) { - mPayload = payload; - } - - public VerifyCredentialResponse stripPayload() { - return new VerifyCredentialResponse(mResponseCode, mTimeout, new byte[0]); + @Override + public String toString() { + return "Response: " + mResponseCode + + ", GK HAT: " + (mGatekeeperHAT != null) + + ", GK PW: " + (mGatekeeperPw != null); } public static VerifyCredentialResponse fromGateKeeperResponse( GateKeeperResponse gateKeeperResponse) { - VerifyCredentialResponse response; int responseCode = gateKeeperResponse.getResponseCode(); if (responseCode == GateKeeperResponse.RESPONSE_RETRY) { - response = new VerifyCredentialResponse(gateKeeperResponse.getTimeout()); + return fromTimeout(gateKeeperResponse.getTimeout()); } else if (responseCode == GateKeeperResponse.RESPONSE_OK) { byte[] token = gateKeeperResponse.getPayload(); if (token == null) { // something's wrong if there's no payload with a challenge Slog.e(TAG, "verifyChallenge response had no associated payload"); - response = VerifyCredentialResponse.ERROR; + return fromError(); } else { - response = new VerifyCredentialResponse(token); + return new VerifyCredentialResponse.Builder().setGatekeeperHAT(token).build(); } } else { - response = VerifyCredentialResponse.ERROR; + return fromError(); } - return response; } } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java index 95bbea15a88c8..fe20d894274f2 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java @@ -16,6 +16,7 @@ package com.android.systemui.biometrics; +import android.annotation.NonNull; import android.content.Context; import android.os.UserHandle; import android.text.InputType; @@ -28,6 +29,7 @@ import android.widget.TextView; import com.android.internal.widget.LockPatternChecker; import com.android.internal.widget.LockscreenCredential; +import com.android.internal.widget.VerifyCredentialResponse; import com.android.systemui.R; /** @@ -105,17 +107,17 @@ public class AuthCredentialPasswordView extends AuthCredentialView } mPendingLockCheck = LockPatternChecker.verifyCredential(mLockPatternUtils, - password, mOperationId, mEffectiveUserId, this::onCredentialVerified); + password, mOperationId, mEffectiveUserId, 0 /* flags */, + this::onCredentialVerified); } } @Override - protected void onCredentialVerified(byte[] attestation, int timeoutMs) { - super.onCredentialVerified(attestation, timeoutMs); + protected void onCredentialVerified(@NonNull VerifyCredentialResponse response, + int timeoutMs) { + super.onCredentialVerified(response, timeoutMs); - final boolean matched = attestation != null; - - if (matched) { + if (response.isMatched()) { mImm.hideSoftInputFromWindow(getWindowToken(), 0 /* flags */); } else { mPasswordField.setText(""); diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java index 6d16f4397115c..0573ef0067dbe 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java @@ -16,6 +16,7 @@ package com.android.systemui.biometrics; +import android.annotation.NonNull; import android.content.Context; import android.util.AttributeSet; @@ -23,6 +24,7 @@ import com.android.internal.widget.LockPatternChecker; import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockPatternView; import com.android.internal.widget.LockscreenCredential; +import com.android.internal.widget.VerifyCredentialResponse; import com.android.systemui.R; import java.util.List; @@ -61,7 +63,7 @@ public class AuthCredentialPatternView extends AuthCredentialView { if (pattern.size() < LockPatternUtils.MIN_PATTERN_REGISTER_FAIL) { // Pattern size is less than the minimum, do not count it as a failed attempt. - onPatternVerified(null /* attestation */, 0 /* timeoutMs */); + onPatternVerified(VerifyCredentialResponse.ERROR, 0 /* timeoutMs */); return; } @@ -71,12 +73,13 @@ public class AuthCredentialPatternView extends AuthCredentialView { credential, mOperationId, mEffectiveUserId, + 0 /* flags */, this::onPatternVerified); } } - private void onPatternVerified(byte[] attestation, int timeoutMs) { - AuthCredentialPatternView.this.onCredentialVerified(attestation, timeoutMs); + private void onPatternVerified(@NonNull VerifyCredentialResponse response, int timeoutMs) { + AuthCredentialPatternView.this.onCredentialVerified(response, timeoutMs); if (timeoutMs > 0) { mLockPatternView.setEnabled(false); } else { diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java index 2695c69056b14..101474be0ea8a 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java @@ -16,6 +16,9 @@ package com.android.systemui.biometrics; +import android.annotation.IntDef; +import android.annotation.NonNull; +import android.annotation.Nullable; import android.app.AlertDialog; import android.app.admin.DevicePolicyManager; import android.content.Context; @@ -38,12 +41,10 @@ import android.widget.ImageView; import android.widget.LinearLayout; import android.widget.TextView; -import androidx.annotation.IntDef; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.annotation.StringRes; import com.android.internal.widget.LockPatternUtils; +import com.android.internal.widget.VerifyCredentialResponse; import com.android.systemui.Interpolators; import com.android.systemui.R; @@ -283,14 +284,11 @@ public abstract class AuthCredentialView extends LinearLayout { protected void onErrorTimeoutFinish() {} - protected void onCredentialVerified(byte[] attestation, int timeoutMs) { - - final boolean matched = attestation != null; - - if (matched) { + protected void onCredentialVerified(@NonNull VerifyCredentialResponse response, int timeoutMs) { + if (response.isMatched()) { mClearErrorRunnable.run(); mLockPatternUtils.userPresent(mEffectiveUserId); - mCallback.onCredentialMatched(attestation); + mCallback.onCredentialMatched(response.getGatekeeperHAT()); } else { if (timeoutMs > 0) { mHandler.removeCallbacks(mClearErrorRunnable); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index f1b89c7a433c1..127d5cc390370 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -33,6 +33,7 @@ import static com.android.internal.widget.LockPatternUtils.SYNTHETIC_PASSWORD_HA import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_LOCKOUT; import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_FOR_UNATTENDED_UPDATE; import static com.android.internal.widget.LockPatternUtils.USER_FRP; +import static com.android.internal.widget.LockPatternUtils.VERIFY_FLAG_RETURN_GK_PW; import static com.android.internal.widget.LockPatternUtils.frpCredentialEnabled; import static com.android.internal.widget.LockPatternUtils.userOwnsFrpCredential; @@ -1308,7 +1309,7 @@ public class LockSettingsService extends ILockSettings.Stub { try { doVerifyCredential(getDecryptedPasswordForTiedProfile(profileHandle), challengeType, challenge, profileHandle, null /* progressCallback */, - resetLockouts); + resetLockouts, 0 /* flags */); } catch (UnrecoverableKeyException | InvalidKeyException | KeyStoreException | NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException | IllegalBlockSizeException @@ -1609,7 +1610,7 @@ public class LockSettingsService extends ILockSettings.Stub { // auth token such that getDecryptedPasswordForTiedProfile() inside // setLockCredentialInternal() can function correctly. verifyCredential(savedCredential, /* challenge */ 0, - mUserManager.getProfileParent(userId).id); + mUserManager.getProfileParent(userId).id, 0 /* flags */); savedCredential.zeroize(); savedCredential = LockscreenCredential.createNone(); } @@ -1724,7 +1725,7 @@ public class LockSettingsService extends ILockSettings.Stub { fixateNewestUserKeyAuth(userId); // Refresh the auth token doVerifyCredential(credential, CHALLENGE_FROM_CALLER, 0, userId, - null /* progressCallback */); + null /* progressCallback */, 0 /* flags */); synchronizeUnifiedWorkChallengeForProfiles(userId, null); sendCredentialsOnChangeIfRequired(credential, userId, isLockTiedToParent); return true; @@ -1835,7 +1836,7 @@ public class LockSettingsService extends ILockSettings.Stub { throw new IllegalArgumentException("Non-OK response verifying a credential we just set " + vcr.getResponseCode()); } - byte[] token = vcr.getPayload(); + byte[] token = vcr.getGatekeeperHAT(); if (token == null) { throw new IllegalArgumentException("Empty payload verifying a credential we just set"); } @@ -1968,35 +1969,66 @@ public class LockSettingsService extends ILockSettings.Stub { ICheckCredentialProgressCallback progressCallback) { checkPasswordReadPermission(userId); try { - return doVerifyCredential(credential, CHALLENGE_NONE, 0, userId, progressCallback); + return doVerifyCredential(credential, CHALLENGE_NONE, 0, userId, progressCallback, + 0 /* flags */); } finally { scheduleGc(); } } @Override + @Nullable public VerifyCredentialResponse verifyCredential(LockscreenCredential credential, - long challenge, int userId) { + long challenge, int userId, @LockPatternUtils.VerifyFlag int flags) { checkPasswordReadPermission(userId); @ChallengeType int challengeType = CHALLENGE_FROM_CALLER; if (challenge == 0) { Slog.w(TAG, "VerifyCredential called with challenge=0"); challengeType = CHALLENGE_NONE; - } + if (challenge != 0 && ((flags & VERIFY_FLAG_RETURN_GK_PW) != 0)) { + // Caller requests Gatekeeper Password to be returned. Challenge is a no-op here, since + // its only used when Gatekeeper verifies the SP. + Slog.w(TAG, "Challenge will not be used"); + } + try { return doVerifyCredential(credential, challengeType, challenge, userId, - null /* progressCallback */); + null /* progressCallback */, flags); } finally { scheduleGc(); } } + @Override + public VerifyCredentialResponse verifyGatekeeperPassword(byte[] gatekeeperPassword, + long challenge, int userId) { + checkPasswordReadPermission(userId); + + VerifyCredentialResponse response; + synchronized (mSpManager) { + response = mSpManager.verifyChallengeInternal(getGateKeeperService(), + gatekeeperPassword, challenge, userId); + } + return response; + } + + /** + * @param credential User's lockscreen credential + * @param challengeType Owner of the challenge + * @param challenge Challenge to be wrapped within Gatekeeper's HAT, if the credential is + * verified + * @param userId User to verify the credential for + * @param progressCallback Receive progress callbacks + * @param flags See {@link LockPatternUtils.VerifyFlag} + * @return See {@link VerifyCredentialResponse} + */ private VerifyCredentialResponse doVerifyCredential(LockscreenCredential credential, @ChallengeType int challengeType, long challenge, int userId, - ICheckCredentialProgressCallback progressCallback) { + ICheckCredentialProgressCallback progressCallback, + @LockPatternUtils.VerifyFlag int flags) { return doVerifyCredential(credential, challengeType, challenge, userId, - progressCallback, null /* resetLockouts */); + progressCallback, null /* resetLockouts */, flags); } /** @@ -2006,7 +2038,8 @@ public class LockSettingsService extends ILockSettings.Stub { private VerifyCredentialResponse doVerifyCredential(LockscreenCredential credential, @ChallengeType int challengeType, long challenge, int userId, ICheckCredentialProgressCallback progressCallback, - @Nullable ArrayList resetLockouts) { + @Nullable ArrayList resetLockouts, + @LockPatternUtils.VerifyFlag int flags) { if (credential == null || credential.isNone()) { throw new IllegalArgumentException("Credential can't be null or empty"); } @@ -2017,7 +2050,7 @@ public class LockSettingsService extends ILockSettings.Stub { } VerifyCredentialResponse response = null; response = spBasedDoVerifyCredential(credential, challengeType, challenge, - userId, progressCallback, resetLockouts); + userId, progressCallback, resetLockouts, flags); // The user employs synthetic password based credential. if (response != null) { if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { @@ -2050,7 +2083,7 @@ public class LockSettingsService extends ILockSettings.Stub { @Override public VerifyCredentialResponse verifyTiedProfileChallenge(LockscreenCredential credential, - long challenge, int userId) { + long challenge, int userId, @LockPatternUtils.VerifyFlag int flags) { checkPasswordReadPermission(userId); if (!isManagedProfileWithUnifiedLock(userId)) { throw new IllegalArgumentException("User id must be managed profile with unified lock"); @@ -2062,7 +2095,8 @@ public class LockSettingsService extends ILockSettings.Stub { CHALLENGE_FROM_CALLER, challenge, parentProfileId, - null /* progressCallback */); + null /* progressCallback */, + flags); if (parentResponse.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) { // Failed, just return parent's response return parentResponse; @@ -2073,7 +2107,8 @@ public class LockSettingsService extends ILockSettings.Stub { return doVerifyCredential(getDecryptedPasswordForTiedProfile(userId), CHALLENGE_FROM_CALLER, challenge, - userId, null /* progressCallback */); + userId, null /* progressCallback */, + flags); } catch (UnrecoverableKeyException | InvalidKeyException | KeyStoreException | NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException | IllegalBlockSizeException @@ -2132,8 +2167,8 @@ public class LockSettingsService extends ILockSettings.Stub { unlockKeystore(credential.getCredential(), userId); Slog.i(TAG, "Unlocking user " + userId + " with token length " - + response.getPayload().length); - unlockUser(userId, response.getPayload(), secretFromCredential(credential)); + + response.getGatekeeperHAT().length); + unlockUser(userId, response.getGatekeeperHAT(), secretFromCredential(credential)); if (isManagedProfileWithSeparatedLock(userId)) { setDeviceUnlockedForUser(userId); @@ -2684,7 +2719,8 @@ public class LockSettingsService extends ILockSettings.Stub { private VerifyCredentialResponse spBasedDoVerifyCredential(LockscreenCredential userCredential, @ChallengeType int challengeType, long challenge, int userId, ICheckCredentialProgressCallback progressCallback, - @Nullable ArrayList resetLockouts) { + @Nullable ArrayList resetLockouts, + @LockPatternUtils.VerifyFlag int flags) { final boolean hasEnrolledBiometrics = mInjector.hasEnrolledBiometrics(userId); @@ -2705,6 +2741,8 @@ public class LockSettingsService extends ILockSettings.Stub { final AuthenticationResult authResult; VerifyCredentialResponse response; + final boolean returnGkPw = (flags & VERIFY_FLAG_RETURN_GK_PW) != 0; + synchronized (mSpManager) { if (!isSyntheticPasswordBasedCredentialLocked(userId)) { return null; @@ -2717,8 +2755,8 @@ public class LockSettingsService extends ILockSettings.Stub { long handle = getSyntheticPasswordHandleLocked(userId); authResult = mSpManager.unwrapPasswordBasedSyntheticPassword( getGateKeeperService(), handle, userCredential, userId, progressCallback); - response = authResult.gkResponse; + // credential has matched if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { // perform verifyChallenge with synthetic password which generates the real GK auth @@ -2739,7 +2777,7 @@ public class LockSettingsService extends ILockSettings.Stub { if (resetLockouts == null) { resetLockouts = new ArrayList<>(); } - resetLockouts.add(new PendingResetLockout(userId, response.getPayload())); + resetLockouts.add(new PendingResetLockout(userId, response.getGatekeeperHAT())); } onCredentialVerified(authResult.authToken, challengeType, challenge, resetLockouts, @@ -2750,7 +2788,12 @@ public class LockSettingsService extends ILockSettings.Stub { } } - return response; + if (response.isMatched() && returnGkPw) { + return new VerifyCredentialResponse.Builder() + .setGatekeeperPassword(authResult.authToken.deriveGkPassword()).build(); + } else { + return response; + } } private void onCredentialVerified(AuthenticationToken authToken, @@ -3172,7 +3215,8 @@ public class LockSettingsService extends ILockSettings.Stub { if (cred == null) { return false; } - return doVerifyCredential(cred, CHALLENGE_NONE, 0, userId, null /* progressCallback */) + return doVerifyCredential(cred, CHALLENGE_NONE, 0, userId, + null /* progressCallback */, 0 /* flags */) .getResponseCode() == VerifyCredentialResponse.RESPONSE_OK; } } diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index d644b1dc6ca08..e31bf3d514ed9 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -482,11 +482,12 @@ public class SyntheticPasswordManager { (int status, WeaverReadResponse readResponse) -> { switch (status) { case WeaverReadStatus.OK: - response[0] = new VerifyCredentialResponse( - fromByteArrayList(readResponse.value)); + response[0] = new VerifyCredentialResponse.Builder().setGatekeeperHAT( + fromByteArrayList(readResponse.value)).build(); break; case WeaverReadStatus.THROTTLE: - response[0] = new VerifyCredentialResponse(readResponse.timeout); + response[0] = VerifyCredentialResponse + .fromTimeout(readResponse.timeout); Slog.e(TAG, "weaver read failed (THROTTLE), slot: " + slot); break; case WeaverReadStatus.INCORRECT_KEY: @@ -494,7 +495,8 @@ public class SyntheticPasswordManager { response[0] = VerifyCredentialResponse.ERROR; Slog.e(TAG, "weaver read failed (INCORRECT_KEY), slot: " + slot); } else { - response[0] = new VerifyCredentialResponse(readResponse.timeout); + response[0] = VerifyCredentialResponse + .fromTimeout(readResponse.timeout); Slog.e(TAG, "weaver read failed (INCORRECT_KEY/THROTTLE), slot: " + slot); } @@ -1007,7 +1009,8 @@ public class SyntheticPasswordManager { return result; } sid = GateKeeper.INVALID_SECURE_USER_ID; - applicationId = transformUnderWeaverSecret(pwdToken, result.gkResponse.getPayload()); + applicationId = transformUnderWeaverSecret(pwdToken, + result.gkResponse.getGatekeeperHAT()); } else { byte[] gkPwdToken = passwordTokenToGkInput(pwdToken); GateKeeperResponse response; @@ -1045,7 +1048,7 @@ public class SyntheticPasswordManager { } } } else if (responseCode == GateKeeperResponse.RESPONSE_RETRY) { - result.gkResponse = new VerifyCredentialResponse(response.getTimeout()); + result.gkResponse = VerifyCredentialResponse.fromTimeout(response.getTimeout()); return result; } else { result.gkResponse = VerifyCredentialResponse.ERROR; @@ -1096,12 +1099,12 @@ public class SyntheticPasswordManager { } VerifyCredentialResponse response = weaverVerify(slotId, null); if (response.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK || - response.getPayload() == null) { + response.getGatekeeperHAT() == null) { Slog.e(TAG, "Failed to retrieve weaver secret when unwrapping token"); result.gkResponse = VerifyCredentialResponse.ERROR; return result; } - secdiscardable = SyntheticPasswordCrypto.decrypt(response.getPayload(), + secdiscardable = SyntheticPasswordCrypto.decrypt(response.getGatekeeperHAT(), PERSONALISATION_WEAVER_TOKEN, secdiscardable); } byte[] applicationId = transformUnderSecdiscardable(token, secdiscardable); @@ -1174,6 +1177,12 @@ public class SyntheticPasswordManager { */ public @Nullable VerifyCredentialResponse verifyChallenge(IGateKeeperService gatekeeper, @NonNull AuthenticationToken auth, long challenge, int userId) { + return verifyChallengeInternal(gatekeeper, auth.deriveGkPassword(), challenge, userId); + } + + protected @Nullable VerifyCredentialResponse verifyChallengeInternal( + IGateKeeperService gatekeeper, @NonNull byte[] gatekeeperPassword, long challenge, + int userId) { byte[] spHandle = loadSyntheticPasswordHandle(userId); if (spHandle == null) { // There is no password handle associated with the given user, i.e. the user is not @@ -1183,18 +1192,19 @@ public class SyntheticPasswordManager { GateKeeperResponse response; try { response = gatekeeper.verifyChallenge(userId, challenge, - spHandle, auth.deriveGkPassword()); + spHandle, gatekeeperPassword); } catch (RemoteException e) { Slog.e(TAG, "Fail to verify with gatekeeper " + userId, e); return VerifyCredentialResponse.ERROR; } int responseCode = response.getResponseCode(); if (responseCode == GateKeeperResponse.RESPONSE_OK) { - VerifyCredentialResponse result = new VerifyCredentialResponse(response.getPayload()); + VerifyCredentialResponse result = new VerifyCredentialResponse.Builder() + .setGatekeeperHAT(response.getPayload()).build(); if (response.getShouldReEnroll()) { try { response = gatekeeper.enroll(userId, spHandle, spHandle, - auth.deriveGkPassword()); + gatekeeperPassword); } catch (RemoteException e) { Slog.e(TAG, "Failed to invoke gatekeeper.enroll", e); response = GateKeeperResponse.ERROR; @@ -1203,7 +1213,8 @@ public class SyntheticPasswordManager { spHandle = response.getPayload(); saveSyntheticPasswordHandle(spHandle, userId); // Call self again to re-verify with updated handle - return verifyChallenge(gatekeeper, auth, challenge, userId); + return verifyChallengeInternal(gatekeeper, gatekeeperPassword, challenge, + userId); } else { // Fall through, return result from the previous verification attempt. Slog.w(TAG, "Fail to re-enroll SP handle for user " + userId); @@ -1211,7 +1222,7 @@ public class SyntheticPasswordManager { } return result; } else if (responseCode == GateKeeperResponse.RESPONSE_RETRY) { - return new VerifyCredentialResponse(response.getTimeout()); + return VerifyCredentialResponse.fromTimeout(response.getTimeout()); } else { return VerifyCredentialResponse.ERROR; } From 6e6a735e65020a02064a9b6b913fb00de71b46d1 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 23 Jul 2020 17:31:22 -0700 Subject: [PATCH 2/5] 2/n: Remove unnecessary RequestThrottledException for verify paths verifyCredential and verifyTiedProfileChallenge return a VerifyCredentialResponse object instead of a byte[] now. Thus, any/all information regarding the authentication attempt is received by callers. We no longer need to use exceptions as a method of returning an alternate non-byte[] result. The RequestThrottledException cannot be completely removed yet because checkCredential returns a boolean and has no way of returning a timeout yet. See b/161956762 Bug: 161765592 Test: Accept/Reject/Lockout the following 1) Owner profile 2) Managed profile with separate challenge 3) Managed profile with unified challenge Change-Id: Ia61c71bdde42dec34d8f2eac86d7ea964b1485c2 --- .../internal/widget/LockPatternChecker.java | 13 ++------ .../internal/widget/LockPatternUtils.java | 30 +++---------------- 2 files changed, 6 insertions(+), 37 deletions(-) diff --git a/core/java/com/android/internal/widget/LockPatternChecker.java b/core/java/com/android/internal/widget/LockPatternChecker.java index eaa29312b7590..a71ca8734e75d 100644 --- a/core/java/com/android/internal/widget/LockPatternChecker.java +++ b/core/java/com/android/internal/widget/LockPatternChecker.java @@ -71,11 +71,7 @@ public final class LockPatternChecker { new AsyncTask() { @Override protected VerifyCredentialResponse doInBackground(Void... args) { - try { - return utils.verifyCredential(credentialCopy, challenge, userId, flags); - } catch (RequestThrottledException ex) { - return VerifyCredentialResponse.fromTimeout(ex.getTimeoutMs()); - } + return utils.verifyCredential(credentialCopy, challenge, userId, flags); } @Override @@ -159,12 +155,7 @@ public final class LockPatternChecker { new AsyncTask() { @Override protected VerifyCredentialResponse doInBackground(Void... args) { - try { - return utils.verifyTiedProfileChallenge(credentialCopy, challenge, userId, - flags); - } catch (RequestThrottledException ex) { - return VerifyCredentialResponse.fromTimeout(ex.getTimeoutMs()); - } + return utils.verifyTiedProfileChallenge(credentialCopy, challenge, userId, flags); } @Override diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 2379e7a550251..206227344b963 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -389,30 +389,19 @@ public class LockPatternUtils { * @param challenge The challenge to verify against the credential * @param userId The user whose credential is being verified * @param flags See {@link VerifyFlag} - * @throws RequestThrottledException if credential verification is being throttled due to - * to many incorrect attempts. * @throws IllegalStateException if called on the main thread. - * - * TODO: This probably doesn't need to throw RequestThrottledException anymore, since the - * method now returns a VerifyCredentialResponse object, which contains the timeout */ @NonNull public VerifyCredentialResponse verifyCredential(@NonNull LockscreenCredential credential, - long challenge, int userId, @VerifyFlag int flags) throws RequestThrottledException { + long challenge, int userId, @VerifyFlag int flags) { throwIfCalledOnMainThread(); try { final VerifyCredentialResponse response = getLockSettings().verifyCredential( credential, challenge, userId, flags); if (response == null) { return VerifyCredentialResponse.ERROR; - } - - if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { - return response; - } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { - throw new RequestThrottledException(response.getTimeout()); } else { - return VerifyCredentialResponse.ERROR; + return response; } } catch (RemoteException re) { Log.e(TAG, "failed to verify credential", re); @@ -485,31 +474,20 @@ public class LockPatternUtils { * @return the attestation that the challenge was verified, or null * @param userId The managed profile user id * @param flags See {@link VerifyFlag} - * @throws RequestThrottledException if credential verification is being throttled due to - * to many incorrect attempts. * @throws IllegalStateException if called on the main thread. - * - * TODO: This probably doesn't need to throw RequestThrottledException anymore, since the - * method now returns a VerifyCredentialResponse object, which contains the timeout */ @NonNull public VerifyCredentialResponse verifyTiedProfileChallenge( @NonNull LockscreenCredential credential, - long challenge, int userId, @VerifyFlag int flags) throws RequestThrottledException { + long challenge, int userId, @VerifyFlag int flags) { throwIfCalledOnMainThread(); try { final VerifyCredentialResponse response = getLockSettings() .verifyTiedProfileChallenge(credential, challenge, userId, flags); if (response == null) { return VerifyCredentialResponse.ERROR; - } - - if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { - return response; - } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { - throw new RequestThrottledException(response.getTimeout()); } else { - return VerifyCredentialResponse.ERROR; + return response; } } catch (RemoteException re) { Log.e(TAG, "failed to verify tied profile credential", re); From cdf4b2868a23edc18792ed2bf689aca38ee6ec82 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Fri, 24 Jul 2020 00:24:11 -0700 Subject: [PATCH 3/5] 3/n: Remove challenge from verifyCredential Decouples the remainder of challenges from LockSettingsService. Clients that require Gatekeeper HATs that wrap challenges should request the Gatekeeper Password, then request LockSettingsService to verify(GatekeeperPassword, Challenge). If the challenge is biometric-related, it must be generated after LockSettingsService completes verifyCredential, since LockSettingsService internally does generateChallenge/resetLockout/revokeChallenge. Bug: 161765592 Test: CtsVerifier biometric portion Test: Reset fingerprint/face lockout Test: atest com.android.server.locksettings Change-Id: Icb384194ce5007b264068e697113d55cbf94945b --- .../android/hardware/face/FaceManager.java | 6 +-- .../internal/widget/ILockSettings.aidl | 4 +- .../internal/widget/LockPatternChecker.java | 8 +--- .../internal/widget/LockPatternUtils.java | 15 +++--- .../AuthCredentialPasswordView.java | 6 ++- .../biometrics/AuthCredentialPatternView.java | 6 ++- .../biometrics/AuthCredentialView.java | 9 +++- .../locksettings/LockSettingsService.java | 33 +++++-------- .../LockSettingsServiceTests.java | 21 ++++---- .../locksettings/LockscreenFrpTest.java | 21 +++++--- .../locksettings/SyntheticPasswordTests.java | 48 ++++++++----------- 11 files changed, 87 insertions(+), 90 deletions(-) diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 885d137dd2fe3..19cb13c7e1148 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -1073,12 +1073,12 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan /** * @hide */ - public abstract static class GenerateChallengeCallback { - public abstract void onGenerateChallengeResult(long challenge); + public interface GenerateChallengeCallback { + void onGenerateChallengeResult(long challenge); } private abstract static class InternalGenerateChallengeCallback - extends GenerateChallengeCallback {} + implements GenerateChallengeCallback {} private class OnEnrollCancelListener implements OnCancelListener { @Override diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index a5a42e1afcdc4..d5d635de81d84 100644 --- a/core/java/com/android/internal/widget/ILockSettings.aidl +++ b/core/java/com/android/internal/widget/ILockSettings.aidl @@ -47,8 +47,8 @@ interface ILockSettings { void resetKeyStore(int userId); VerifyCredentialResponse checkCredential(in LockscreenCredential credential, int userId, in ICheckCredentialProgressCallback progressCallback); - VerifyCredentialResponse verifyCredential(in LockscreenCredential credential, long challenge, int userId, int flags); - VerifyCredentialResponse verifyTiedProfileChallenge(in LockscreenCredential credential, long challenge, int userId, int flags); + VerifyCredentialResponse verifyCredential(in LockscreenCredential credential, int userId, int flags); + VerifyCredentialResponse verifyTiedProfileChallenge(in LockscreenCredential credential, int userId, int flags); VerifyCredentialResponse verifyGatekeeperPassword(in byte[] gatekeeperPassword, long challenge, int userId); boolean checkVoldPassword(int userId); int getCredentialType(int userId); diff --git a/core/java/com/android/internal/widget/LockPatternChecker.java b/core/java/com/android/internal/widget/LockPatternChecker.java index a71ca8734e75d..5adbc583140fe 100644 --- a/core/java/com/android/internal/widget/LockPatternChecker.java +++ b/core/java/com/android/internal/widget/LockPatternChecker.java @@ -54,14 +54,12 @@ public final class LockPatternChecker { * * @param utils The LockPatternUtils instance to use. * @param credential The credential to check. - * @param challenge The challenge to verify against the credential. * @param userId The user to check against the credential. * @param flags See {@link LockPatternUtils.VerifyFlag} * @param callback The callback to be invoked with the verification result. */ public static AsyncTask verifyCredential(final LockPatternUtils utils, final LockscreenCredential credential, - final long challenge, final int userId, final @LockPatternUtils.VerifyFlag int flags, final OnVerifyCallback callback) { @@ -71,7 +69,7 @@ public final class LockPatternChecker { new AsyncTask() { @Override protected VerifyCredentialResponse doInBackground(Void... args) { - return utils.verifyCredential(credentialCopy, challenge, userId, flags); + return utils.verifyCredential(credentialCopy, userId, flags); } @Override @@ -138,14 +136,12 @@ public final class LockPatternChecker { * * @param utils The LockPatternUtils instance to use. * @param credential The credential to check. - * @param challenge The challenge to verify against the credential. * @param userId The user to check against the credential. * @param flags See {@link LockPatternUtils.VerifyFlag} * @param callback The callback to be invoked with the verification result. */ public static AsyncTask verifyTiedProfileChallenge(final LockPatternUtils utils, final LockscreenCredential credential, - final long challenge, final int userId, final @LockPatternUtils.VerifyFlag int flags, final OnVerifyCallback callback) { @@ -155,7 +151,7 @@ public final class LockPatternChecker { new AsyncTask() { @Override protected VerifyCredentialResponse doInBackground(Void... args) { - return utils.verifyTiedProfileChallenge(credentialCopy, challenge, userId, flags); + return utils.verifyTiedProfileChallenge(credentialCopy, userId, flags); } @Override diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 206227344b963..f7370d6a22f97 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -386,18 +386,17 @@ public class LockPatternUtils { * If credential matches, return an opaque attestation that the challenge was verified. * * @param credential The credential to check. - * @param challenge The challenge to verify against the credential * @param userId The user whose credential is being verified * @param flags See {@link VerifyFlag} * @throws IllegalStateException if called on the main thread. */ @NonNull public VerifyCredentialResponse verifyCredential(@NonNull LockscreenCredential credential, - long challenge, int userId, @VerifyFlag int flags) { + int userId, @VerifyFlag int flags) { throwIfCalledOnMainThread(); try { final VerifyCredentialResponse response = getLockSettings().verifyCredential( - credential, challenge, userId, flags); + credential, userId, flags); if (response == null) { return VerifyCredentialResponse.ERROR; } else { @@ -411,8 +410,8 @@ public class LockPatternUtils { /** * With the Gatekeeper Password returned via {@link #verifyCredential(LockscreenCredential, - * long, int, boolean)}, request Gatekeeper to create a HardwareAuthToken wrapping the - * given challenge. + * int, int)}, request Gatekeeper to create a HardwareAuthToken wrapping the given + * challenge. */ @NonNull public VerifyCredentialResponse verifyGatekeeperPassword(@NonNull byte[] gatekeeperPassword, @@ -470,7 +469,6 @@ public class LockPatternUtils { * verified. * * @param credential The parent user's credential to check. - * @param challenge The challenge to verify against the credential * @return the attestation that the challenge was verified, or null * @param userId The managed profile user id * @param flags See {@link VerifyFlag} @@ -478,12 +476,11 @@ public class LockPatternUtils { */ @NonNull public VerifyCredentialResponse verifyTiedProfileChallenge( - @NonNull LockscreenCredential credential, - long challenge, int userId, @VerifyFlag int flags) { + @NonNull LockscreenCredential credential, int userId, @VerifyFlag int flags) { throwIfCalledOnMainThread(); try { final VerifyCredentialResponse response = getLockSettings() - .verifyTiedProfileChallenge(credential, challenge, userId, flags); + .verifyTiedProfileChallenge(credential, userId, flags); if (response == null) { return VerifyCredentialResponse.ERROR; } else { diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java index fe20d894274f2..d4e6506bd9a0d 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPasswordView.java @@ -28,6 +28,7 @@ import android.widget.ImeAwareEditText; import android.widget.TextView; import com.android.internal.widget.LockPatternChecker; +import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockscreenCredential; import com.android.internal.widget.VerifyCredentialResponse; import com.android.systemui.R; @@ -106,8 +107,11 @@ public class AuthCredentialPasswordView extends AuthCredentialView return; } + // Request LockSettingsService to return the Gatekeeper Password in the + // VerifyCredentialResponse so that we can request a Gatekeeper HAT with the + // Gatekeeper Password and operationId. mPendingLockCheck = LockPatternChecker.verifyCredential(mLockPatternUtils, - password, mOperationId, mEffectiveUserId, 0 /* flags */, + password, mEffectiveUserId, LockPatternUtils.VERIFY_FLAG_RETURN_GK_PW, this::onCredentialVerified); } } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java index 0573ef0067dbe..ad89ae82f6376 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialPatternView.java @@ -68,12 +68,14 @@ public class AuthCredentialPatternView extends AuthCredentialView { } try (LockscreenCredential credential = LockscreenCredential.createPattern(pattern)) { + // Request LockSettingsService to return the Gatekeeper Password in the + // VerifyCredentialResponse so that we can request a Gatekeeper HAT with the + // Gatekeeper Password and operationId. mPendingLockCheck = LockPatternChecker.verifyCredential( mLockPatternUtils, credential, - mOperationId, mEffectiveUserId, - 0 /* flags */, + LockPatternUtils.VERIFY_FLAG_RETURN_GK_PW, this::onPatternVerified); } } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java index 101474be0ea8a..a8f6f85201f6f 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthCredentialView.java @@ -288,7 +288,14 @@ public abstract class AuthCredentialView extends LinearLayout { if (response.isMatched()) { mClearErrorRunnable.run(); mLockPatternUtils.userPresent(mEffectiveUserId); - mCallback.onCredentialMatched(response.getGatekeeperHAT()); + + // The response passed into this method contains the Gatekeeper Password. We still + // have to request Gatekeeper to create a Hardware Auth Token with the + // Gatekeeper Password and Challenge (keystore operationId in this case) + final VerifyCredentialResponse gkResponse = mLockPatternUtils.verifyGatekeeperPassword( + response.getGatekeeperPw(), mOperationId, mEffectiveUserId); + + mCallback.onCredentialMatched(gkResponse.getGatekeeperHAT()); } else { if (timeoutMs > 0) { mHandler.removeCallbacks(mClearErrorRunnable); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 127d5cc390370..d6e37bacdba8a 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -187,6 +187,9 @@ public class LockSettingsService extends ILockSettings.Stub { private static final String SYNTHETIC_PASSWORD_UPDATE_TIME_KEY = "sp-handle-ts"; private static final String USER_SERIAL_NUMBER_KEY = "serial-number"; + // TODO (b/145978626) LockSettingsService no longer accepts challenges in the verifyCredential + // paths. These are temporarily left around to ensure that resetLockout works. It will be + // removed once resetLockout is compartmentalized. // No challenge provided private static final int CHALLENGE_NONE = 0; // Challenge was provided from the external caller (non-LockSettingsService) @@ -1609,8 +1612,8 @@ public class LockSettingsService extends ILockSettings.Stub { // Verify the parent credential again, to make sure we have a fresh enough // auth token such that getDecryptedPasswordForTiedProfile() inside // setLockCredentialInternal() can function correctly. - verifyCredential(savedCredential, /* challenge */ 0, - mUserManager.getProfileParent(userId).id, 0 /* flags */); + verifyCredential(savedCredential, mUserManager.getProfileParent(userId).id, + 0 /* flags */); savedCredential.zeroize(); savedCredential = LockscreenCredential.createNone(); } @@ -1969,7 +1972,7 @@ public class LockSettingsService extends ILockSettings.Stub { ICheckCredentialProgressCallback progressCallback) { checkPasswordReadPermission(userId); try { - return doVerifyCredential(credential, CHALLENGE_NONE, 0, userId, progressCallback, + return doVerifyCredential(credential, CHALLENGE_NONE, 0L, userId, progressCallback, 0 /* flags */); } finally { scheduleGc(); @@ -1979,21 +1982,11 @@ public class LockSettingsService extends ILockSettings.Stub { @Override @Nullable public VerifyCredentialResponse verifyCredential(LockscreenCredential credential, - long challenge, int userId, @LockPatternUtils.VerifyFlag int flags) { + int userId, int flags) { checkPasswordReadPermission(userId); - @ChallengeType int challengeType = CHALLENGE_FROM_CALLER; - if (challenge == 0) { - Slog.w(TAG, "VerifyCredential called with challenge=0"); - challengeType = CHALLENGE_NONE; - } - if (challenge != 0 && ((flags & VERIFY_FLAG_RETURN_GK_PW) != 0)) { - // Caller requests Gatekeeper Password to be returned. Challenge is a no-op here, since - // its only used when Gatekeeper verifies the SP. - Slog.w(TAG, "Challenge will not be used"); - } try { - return doVerifyCredential(credential, challengeType, challenge, userId, + return doVerifyCredential(credential, CHALLENGE_NONE, 0L, userId, null /* progressCallback */, flags); } finally { scheduleGc(); @@ -2083,7 +2076,7 @@ public class LockSettingsService extends ILockSettings.Stub { @Override public VerifyCredentialResponse verifyTiedProfileChallenge(LockscreenCredential credential, - long challenge, int userId, @LockPatternUtils.VerifyFlag int flags) { + int userId, @LockPatternUtils.VerifyFlag int flags) { checkPasswordReadPermission(userId); if (!isManagedProfileWithUnifiedLock(userId)) { throw new IllegalArgumentException("User id must be managed profile with unified lock"); @@ -2092,8 +2085,8 @@ public class LockSettingsService extends ILockSettings.Stub { // Unlock parent by using parent's challenge final VerifyCredentialResponse parentResponse = doVerifyCredential( credential, - CHALLENGE_FROM_CALLER, - challenge, + CHALLENGE_NONE, + 0L, parentProfileId, null /* progressCallback */, flags); @@ -2105,8 +2098,8 @@ public class LockSettingsService extends ILockSettings.Stub { try { // Unlock work profile, and work profile with unified lock must use password only return doVerifyCredential(getDecryptedPasswordForTiedProfile(userId), - CHALLENGE_FROM_CALLER, - challenge, + CHALLENGE_NONE, + 0L, userId, null /* progressCallback */, flags); } catch (UnrecoverableKeyException | InvalidKeyException | KeyStoreException diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 12b144f2b7789..1f66c7c02658b 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -129,7 +129,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mGateKeeperService.clearAuthToken(TURNED_OFF_PROFILE_USER_ID); // verify credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - firstUnifiedPassword, 0, PRIMARY_USER_ID) + firstUnifiedPassword, PRIMARY_USER_ID, 0 /* flags */) .getResponseCode()); // Verify that we have a new auth token for the profile @@ -186,13 +186,13 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mGateKeeperService.clearAuthToken(MANAGED_PROFILE_USER_ID); // verify primary credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - primaryPassword, 0, PRIMARY_USER_ID) + primaryPassword, PRIMARY_USER_ID, 0 /* flags */) .getResponseCode()); assertNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); // verify profile credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - profilePassword, 0, MANAGED_PROFILE_USER_ID) + profilePassword, MANAGED_PROFILE_USER_ID, 0 /* flags */) .getResponseCode()); assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); @@ -203,7 +203,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { newPassword("pwd"), primaryPassword, PRIMARY_USER_ID)); mStorageManager.setIgnoreBadUnlock(false); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - profilePassword, 0, MANAGED_PROFILE_USER_ID) + profilePassword, MANAGED_PROFILE_USER_ID, 0 /* flags */) .getResponseCode()); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); } @@ -389,7 +389,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { initializeStorageWithCredential(PRIMARY_USER_ID, password, 1234); reset(mRecoverableKeyStoreManager); - mService.verifyCredential(password, 1, PRIMARY_USER_ID); + mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */); verify(mRecoverableKeyStoreManager) .lockScreenSecretAvailable( @@ -406,7 +406,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { MANAGED_PROFILE_USER_ID)); reset(mRecoverableKeyStoreManager); - mService.verifyCredential(pattern, 1, MANAGED_PROFILE_USER_ID); + mService.verifyCredential(pattern, MANAGED_PROFILE_USER_ID, 0 /* flags */); verify(mRecoverableKeyStoreManager) .lockScreenSecretAvailable( @@ -421,7 +421,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null); reset(mRecoverableKeyStoreManager); - mService.verifyCredential(pattern, 1, PRIMARY_USER_ID); + mService.verifyCredential(pattern, PRIMARY_USER_ID, 0 /* flags */); // Parent sends its credentials for both the parent and profile. verify(mRecoverableKeyStoreManager) @@ -484,9 +484,8 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { private void assertVerifyCredentials(int userId, LockscreenCredential credential, long sid) throws RemoteException{ - final long challenge = 54321; - VerifyCredentialResponse response = mService.verifyCredential(credential, - challenge, userId); + VerifyCredentialResponse response = mService.verifyCredential(credential, userId, + 0 /* flags */); assertEquals(GateKeeperResponse.RESPONSE_OK, response.getResponseCode()); if (sid != -1) assertEquals(sid, mGateKeeperService.getSecureUserId(userId)); @@ -508,7 +507,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { badCredential = LockscreenCredential.createPin("0"); } assertEquals(GateKeeperResponse.RESPONSE_ERROR, mService.verifyCredential( - badCredential, challenge, userId).getResponseCode()); + badCredential, userId, 0 /* flags */).getResponseCode()); } private void initializeStorageWithCredential(int userId, LockscreenCredential credential, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java index 4b3f7b5d08ef6..9c0239b2b6a6e 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java @@ -52,7 +52,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PIN, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPin("1234"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPin("1234"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @Test @@ -61,7 +62,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PATTERN, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPattern("4321"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPattern("4321"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @Test @@ -70,7 +72,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PASSWORD, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPassword("4321"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPassword("4321"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @Test @@ -80,7 +83,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PATTERN, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPattern("5678"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPattern("5678"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @Test @@ -98,7 +102,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { mSettings.setDeviceProvisioned(true); assertEquals(VerifyCredentialResponse.RESPONSE_ERROR, - mService.verifyCredential(newPin("1234"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPin("1234"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @Test @@ -113,7 +118,8 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PIN, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPin("1234"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPin("1234"), USER_FRP, 0 /* flags */) + .getResponseCode()); } @@ -129,6 +135,7 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { assertEquals(CREDENTIAL_TYPE_PASSWORD, mService.getCredentialType(USER_FRP)); assertEquals(VerifyCredentialResponse.RESPONSE_OK, - mService.verifyCredential(newPin("1234"), 0, USER_FRP).getResponseCode()); + mService.verifyCredential(newPin("1234"), USER_FRP, 0 /* flags */) + .getResponseCode()); } } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index ba851992cbad9..2bd0e55f7d21b 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -119,8 +119,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); mService.setLockCredential(newPassword, password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - newPassword, 0, PRIMARY_USER_ID) - .getResponseCode()); + newPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); } @@ -131,12 +130,10 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { initializeCredentialUnderSP(password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, 0, PRIMARY_USER_ID) - .getResponseCode()); + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertEquals(VerifyCredentialResponse.RESPONSE_ERROR, mService.verifyCredential( - badPassword, 0, PRIMARY_USER_ID) - .getResponseCode()); + badPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); } @Test @@ -153,8 +150,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { // set a new password mService.setLockCredential(badPassword, nonePassword(), PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, 0, PRIMARY_USER_ID) - .getResponseCode()); + badPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertNotEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); } @@ -166,8 +162,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { initializeCredentialUnderSP(password, PRIMARY_USER_ID); mService.setLockCredential(badPassword, password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, 0, PRIMARY_USER_ID) - .getResponseCode()); + badPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); // Check the same secret was passed each time ArgumentCaptor> secret = ArgumentCaptor.forClass(ArrayList.class); @@ -183,8 +178,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { initializeCredentialUnderSP(password, PRIMARY_USER_ID); reset(mAuthSecretService); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, 0, PRIMARY_USER_ID) - .getResponseCode()); + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); verify(mAuthSecretService).primaryUserCredential(any(ArrayList.class)); } @@ -194,8 +188,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { initializeCredentialUnderSP(password, SECONDARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, 0, SECONDARY_USER_ID) - .getResponseCode()); + password, SECONDARY_USER_ID, 0 /* flags */).getResponseCode()); verify(mAuthSecretService, never()).primaryUserCredential(any(ArrayList.class)); } @@ -246,7 +239,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertTrue(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) + .getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertFalse(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); @@ -259,8 +253,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { verify(mDevicePolicyManager).reportPasswordChanged(PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - pattern, 0, PRIMARY_USER_ID) - .getResponseCode()); + pattern, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } @@ -275,7 +268,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) + .getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mLocalService.setLockCredentialWithToken(nonePassword(), handle, token, PRIMARY_USER_ID); @@ -284,8 +278,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - pattern, 0, PRIMARY_USER_ID) - .getResponseCode()); + pattern, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } @@ -301,7 +294,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) + .getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.setLockCredential(pattern, password, PRIMARY_USER_ID); @@ -309,8 +303,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { mLocalService.setLockCredentialWithToken(newPassword, handle, token, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - newPassword, 0, PRIMARY_USER_ID) - .getResponseCode()); + newPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } @@ -357,8 +350,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); // Activate token (password gets migrated to SP at the same time) assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, 0, PRIMARY_USER_ID) - .getResponseCode()); + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); // Verify token is activated assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); } @@ -488,8 +480,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { initializeCredentialUnderSP(password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, 0, PRIMARY_USER_ID) - .getResponseCode()); + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); verify(mAuthSecretService, never()).primaryUserCredential(any(ArrayList.class)); } @@ -503,7 +494,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { reset(mDevicePolicyManager); long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) + .getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.onCleanupUser(PRIMARY_USER_ID); From cd52980d92dbb2502ae5c03f82798e63cad69d65 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 30 Jul 2020 14:05:07 -0700 Subject: [PATCH 4/5] Change GenerateChallengeCallback from abstract class to interface This allows us to use lamdas Test: Builds Bug: 161325267 Change-Id: I54e75d363969bbe74e346f2685b331d8743bb2c5 --- .../android/hardware/fingerprint/FingerprintManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index e384da7574ada..b92051b24dfaf 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -377,12 +377,12 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing /** * @hide */ - public abstract static class GenerateChallengeCallback { - public abstract void onChallengeGenerated(long challenge); + public interface GenerateChallengeCallback { + void onChallengeGenerated(long challenge); } private abstract static class InternalGenerateChallengeCallback - extends GenerateChallengeCallback {} + implements GenerateChallengeCallback {} /** * Request authentication of a crypto object. This call warms up the fingerprint hardware From 0bca835ebedc764592f265da5ae2dfc258af42cd Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 30 Jul 2020 18:31:31 -0700 Subject: [PATCH 5/5] Remove GenerateChallengeBlocking from FingerprintManager Test: Builds Bug: 162533680 Change-Id: I28c01bf977d32a5decd941c0449081577069d174 --- .../fingerprint/FingerprintManager.java | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index b92051b24dfaf..71598eb6394f5 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -580,37 +580,6 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } } - /** - * Same as {@link #generateChallenge(GenerateChallengeCallback)}, except blocks until the - * TEE/hardware operation is complete. - * @return challenge generated in the TEE/hardware - * @hide - */ - @RequiresPermission(MANAGE_FINGERPRINT) - public long generateChallengeBlocking() { - final AtomicReference result = new AtomicReference<>(); - final CountDownLatch latch = new CountDownLatch(1); - final GenerateChallengeCallback callback = new InternalGenerateChallengeCallback() { - @Override - public void onChallengeGenerated(long challenge) { - result.set(challenge); - latch.countDown(); - } - }; - - generateChallenge(callback); - - try { - latch.await(1, TimeUnit.SECONDS); - } catch (InterruptedException e) { - Slog.e(TAG, "Interrupted while generatingChallenge", e); - e.printStackTrace(); - } - - return result.get(); - } - - /** * Generates a unique random challenge in the TEE. A typical use case is to have it wrapped in a * HardwareAuthenticationToken, minted by Gatekeeper upon PIN/Pattern/Password verification.