Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 61ca99b4 authored by Eric Biggers's avatar Eric Biggers
Browse files

Use RESPONSE_CRED_INCORRECT for Weaver INCORRECT_KEY with nonzero timeout 

Translate Weaver statuses of INCORRECT_KEY with nonzero timeout into
RESPONSE_CRED_INCORRECT instead of RESPONSE_RETRY.  The timeout remains
the same and is still available in the VerifyCredentialResponse.

The result is that in this case
LockSettingsService#reportResultToSoftwareRateLimiter() will tell the
software rate-limiter the guess is certainly wrong.  This allows
duplicates of it to be detected, whereas previously they weren't
detected.  Note that given the current software rate-limiting schedule
combined with the 5 minute expiration time on the saved wrong guesses,
this makes a difference only for (unique) guess number 5.  I.e., when
making guess 6, duplicates of guess 5 are now detected.

Previously, VerifyCredentialResponse#hasTimeout() checked whether the
response code was RESPONSE_RETRY.  Make it instead check for nonzero
timeout, so that it also returns true for RESPONSE_CRED_INCORRECT
responses with nonzero timeout (which previously used RESPONSE_RETRY).

Technically, this does change the behavior of hasTimeout() in the edge
case where GateKeeper returns RETRY with zero timeout or Weaver returns
THROTTLE with zero timeout.  But that is not supposed to happen, and
even if it does, the new behavior seems correct anyway.  The purpose of
checking for RESPONSE_RETRY was to determine whether there is a timeout.
If the timeout is zero, then there is no timeout.

Bug: 423038471
Test: atest FrameworksServicesTests:com.android.server.locksettings
Test: manually tested detection of duplication with guess #5 as
      described above, on a device whose Weaver implementation uses
      INCORRECT_KEY with nonzero timeout so is affected by this change.
Flag: android.security.software_ratelimiter
Change-Id: I414481de5c314a1ac2f274f1910563396721271c
parent 25fbdd63

core/java/com/android/internal/widget/VerifyCredentialResponse.java

+32 −8
Original line number Diff line number Diff line
@@ -45,9 +45,14 @@ public final class VerifyCredentialResponse implements Parcelable { 
    private static final int RESPONSE_OK = 0;



    /**

     * Either the credential could not be verified because a timeout is still active, or the

     * credential was incorrect and there is a timeout before the next attempt will be allowed.

     * {@link #getTimeout()} gives the timeout.

     * The credential could not be verified because a timeout is still active. {@link #getTimeout()}

     * gives the currently active timeout.

     *

     * <p>Alternatively, a timeout was not active, the credential was incorrect, and there is a

     * timeout before the <em>next</em> attempt will be allowed. {@link #getTimeout()} gives the

     * newly active timeout. The preferred response code in this case is {@link

     * #RESPONSE_CRED_INCORRECT}, but some devices use a rate-limiting HAL implementation that does

     * not differentiate this case from the "timeout is still active" case.

     */

    private static final int RESPONSE_RETRY = 1;
@@ -60,6 +65,8 @@ public final class VerifyCredentialResponse implements Parcelable { 
    /**

     * Credential was incorrect and none of {@link #RESPONSE_RETRY}, {@link

     * #RESPONSE_CRED_TOO_SHORT}, or {@link #RESPONSE_CRED_ALREADY_TRIED} applies.

     *

     * <p>{@link #getTimeout()} gives the newly active timeout, if any.

     */

    private static final int RESPONSE_CRED_INCORRECT = 4;
@@ -136,10 +143,8 @@ public final class VerifyCredentialResponse implements Parcelable { 
    }



    /**

     * 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}.

     * Builds a {@link VerifyCredentialResponse} with {@link #RESPONSE_RETRY} and the given timeout

     * in milliseconds.

     */

    public static VerifyCredentialResponse fromTimeout(int timeout) {

        return new VerifyCredentialResponse(RESPONSE_RETRY,
@@ -149,7 +154,7 @@ public final class VerifyCredentialResponse implements Parcelable { 
    }



    /**

     * Like {@link #fromTimeout(int)}, but takes a Duration instead of a raw milliseconds value.

     * Builds a {@link VerifyCredentialResponse} with {@link #RESPONSE_RETRY} and the given timeout.

     *

     * <p>The timeout is clamped to fit in an int. See {@link #timeoutToClampedMillis(Duration)}.

     */
@@ -157,6 +162,20 @@ public final class VerifyCredentialResponse implements Parcelable { 
        return fromTimeout(timeoutToClampedMillis(timeout));

    }



    /**

     * Builds a {@link VerifyCredentialResponse} with {@link #RESPONSE_CRED_INCORRECT} and the given

     * timeout.

     *

     * <p>The timeout is clamped to fit in an int. See {@link #timeoutToClampedMillis(Duration)}.

     */

    public static VerifyCredentialResponse credIncorrect(Duration timeout) {

        return new VerifyCredentialResponse(

                VerifyCredentialResponse.RESPONSE_CRED_INCORRECT,

                timeoutToClampedMillis(timeout),

                /* gatekeeperHAT= */ null,

                /* gatekeeperPasswordHandle= */ 0L);

    }



    /**

     * Clamps the given timeout to fit in an int that holds a non-negative milliseconds value.

     *
@@ -260,6 +279,11 @@ public final class VerifyCredentialResponse implements Parcelable { 
     * will be allowed.

     */

    public boolean hasTimeout() {

        if (android.security.Flags.softwareRatelimiter()) {

            // Check mTimeout directly. It can be nonzero for either RESPONSE_RETRY or

            // RESPONSE_CRED_INCORRECT.

            return mTimeout != 0;

        }

        return mResponseCode == RESPONSE_RETRY;

    }

services/core/java/com/android/server/locksettings/LockSettingsService.java

+4 −1
Original line number Diff line number Diff line
@@ -2552,7 +2552,10 @@ public class LockSettingsService extends ILockSettings.Stub { 
                // lock screen doesn't allow another attempt until both rate-limiters allow it.

                Duration hwTimeout = response.getTimeoutAsDuration();

                if (swTimeout.compareTo(hwTimeout) > 0) {

                    response = VerifyCredentialResponse.fromTimeout(swTimeout);

                    response =

                            isCertainlyWrongGuess

                                    ? VerifyCredentialResponse.credIncorrect(swTimeout)

                                    : VerifyCredentialResponse.fromTimeout(swTimeout);

                }

            }

        }

services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java

+7 −9
Original line number Diff line number Diff line
@@ -719,19 +719,17 @@ class SyntheticPasswordManager { 
                    // THROTTLE for both cases.

                    VerifyCredentialResponse.fromTimeout(Duration.ofMillis(weaverResponse.timeout));

            case WeaverReadStatus.INCORRECT_KEY -> {

                // The credential was incorrect. There may be a timeout until the next attempt is

                // allowed; that occurs when the Weaver implementation returns INCORRECT_KEY in this

                // case instead of THROTTLE.

                if (android.security.Flags.softwareRatelimiter()) {

                    yield VerifyCredentialResponse.credIncorrect(

                            Duration.ofMillis(weaverResponse.timeout));

                }

                if (weaverResponse.timeout != 0) {

                    // The credential was incorrect, and there is a timeout until the next attempt

                    // will be allowed. This is reached if the Weaver implementation returns

                    // INCORRECT_KEY in this case instead of THROTTLE.

                    //

                    // TODO(b/395976735): use RESPONSE_CRED_INCORRECT in this case, and update users

                    // of VerifyCredentialResponse to be compatible with that.

                    yield VerifyCredentialResponse.fromTimeout(

                            Duration.ofMillis(weaverResponse.timeout));

                }

                if (android.security.Flags.softwareRatelimiter()) {

                    yield VerifyCredentialResponse.credIncorrect();

                }

                yield VerifyCredentialResponse.OTHER_ERROR;

            }

            default -> VerifyCredentialResponse.OTHER_ERROR;

services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java

+3 −5
Original line number Diff line number Diff line
@@ -58,7 +58,6 @@ import com.android.internal.widget.LockscreenCredential; 
import com.android.internal.widget.VerifyCredentialResponse;



import org.junit.Before;

import org.junit.Ignore;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -733,6 +732,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
                mService.verifyCredential(wrongGuess, userId, /* flags= */ 0);

        assertTrue(response.isCredCertainlyIncorrect());

        assertFalse(response.isCredAlreadyTried());

        assertFalse(response.hasTimeout());

        assertEquals(Duration.ZERO, response.getTimeoutAsDuration());



        response = mService.verifyCredential(wrongGuess, userId, /* flags= */ 0);
@@ -741,10 +741,6 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
    }



    // Same as preceding test case, but uses a nonzero timeout.

    //

    // TODO(b/395976735): currently the behavior in this scenario is wrong, so currently this test

    // case is ignored. Fix the behavior and remove @Ignore.

    @Ignore

    @Test

    @EnableFlags(android.security.Flags.FLAG_SOFTWARE_RATELIMITER)

    public void testRepeatOfWrongGuessRejectedAsDuplicate_afterWeaverIncorrectKeyWithTimeout()
@@ -760,6 +756,8 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
        mSpManager.injectWeaverReadResponse(WeaverReadStatus.INCORRECT_KEY, timeout);

        VerifyCredentialResponse response =

                mService.verifyCredential(wrongGuess, userId, /* flags= */ 0);

        assertTrue(response.isCredCertainlyIncorrect());

        assertFalse(response.isCredAlreadyTried());

        assertTrue(response.hasTimeout());

        assertEquals(timeout, response.getTimeoutAsDuration());