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

Commit e9959faa authored by Eric Biggers's avatar Eric Biggers
Browse files

Consolidate timeout clamping into VerifyCredentialResponse 

In preparation for supporting RESPONSE_CRED_INCORRECT with nonzero
timeout, consolidate the timeout clamping into VerifyCredentialResponse.

Specifically, change the timeout clamping logic in
VerifyCredentialResponse#fromTimeout(Duration) (which previously was
called only for SoftwareRateLimiter responses) to be identical to the
clamping logic in SyntheticPasswordManager#responseFromTimeout() (which
was used for Weaver responses).  Then replace calls to the latter method
with the former, and remove the latter method.

This only change in behavior is how negative timeouts from the
SoftwareRateLimiter are handled.  This does not matter, since
SoftwareRateLimiter never returns negative timeouts.

Bug: 423038471
Test: atest FrameworksServicesTests:com.android.server.locksettings
Flag: EXEMPT refactor
Change-Id: Ie67513e3bd2888ab92cb63ac393c217e356a1382
parent 449a634c

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

+23 −3
Original line number Diff line number Diff line
@@ -74,6 +74,8 @@ public final class VerifyCredentialResponse implements Parcelable { 
    @Retention(RetentionPolicy.SOURCE)

    @interface ResponseCode {}



    private static final Duration MAX_TIMEOUT = Duration.ofMillis(Integer.MAX_VALUE);



    public static final VerifyCredentialResponse OK = new VerifyCredentialResponse.Builder()

            .build();

    public static final VerifyCredentialResponse OTHER_ERROR = fromError(RESPONSE_OTHER_ERROR);
@@ -146,12 +148,30 @@ public final class VerifyCredentialResponse implements Parcelable { 
                0L /* gatekeeperPasswordHandle */);

    }



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

    /**

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

     *

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

     */

    public static VerifyCredentialResponse fromTimeout(Duration timeout) {

        return fromTimeout((int) Math.min(timeout.toMillis(), (long) Integer.MAX_VALUE));

        return fromTimeout(timeoutToClampedMillis(timeout));

    }



    /**

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

     *

     * <p>A negative timeout should never occur here, since the rate-limiters do not report negative

     * timeouts. If a negative timeout is seen anyway, fail secure and treat it as possibly intended

     * to be an unsigned value, i.e. clamp it to MAX_VALUE rather than MIN_VALUE.

     */

    private static int timeoutToClampedMillis(Duration timeout) {

        if (timeout.isNegative() || timeout.compareTo(MAX_TIMEOUT) > 0) {

            return Integer.MAX_VALUE;

        }

        return (int) timeout.toMillis();

    }



    /** Builds a {@link VerifyCredentialResponse} with {@link RESPONSE_OTHER_ERROR}. */

    /** Builds a {@link VerifyCredentialResponse} with {@link #RESPONSE_OTHER_ERROR}. */

    public static VerifyCredentialResponse fromError() {

        return fromError(RESPONSE_OTHER_ERROR);

    }

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

+5 −15
Original line number Diff line number Diff line
@@ -68,6 +68,7 @@ import libcore.util.HexEncoding; 
import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.nio.ByteBuffer;

import java.time.Duration;

import java.util.Arrays;

import java.util.Collections;

import java.util.HashSet;
@@ -700,19 +701,6 @@ class SyntheticPasswordManager { 
        return value;

    }



    /**

     * Create a VerifyCredentialResponse from a timeout base on the WeaverReadResponse.

     * This checks the received timeout(long) to make sure it sure it fits in an int before

     * using it. If it doesn't fit, we use Integer.MAX_VALUE.

     */

    private static VerifyCredentialResponse responseFromTimeout(WeaverReadResponse response) {

        int timeout =

                response.timeout > Integer.MAX_VALUE || response.timeout < 0

                ? Integer.MAX_VALUE

                : (int) response.timeout;

        return VerifyCredentialResponse.fromTimeout(timeout);

    }



    /**

     * Translate a {@link WeaverReadResponse} to a {@link VerifyCredentialResponse}.

     *
@@ -729,7 +717,8 @@ class SyntheticPasswordManager { 
                // the credential was incorrect and there is a timeout before the next attempt will

                // be allowed. INCORRECT_KEY is preferred in the latter case to avoid the ambiguity,

                // but we still have to support implementations that use THROTTLE for both cases.

                return responseFromTimeout(weaverResponse);

                return VerifyCredentialResponse.fromTimeout(

                        Duration.ofMillis(weaverResponse.timeout));

            case WeaverReadStatus.INCORRECT_KEY:

                if (weaverResponse.timeout != 0) {

                    // The credential was incorrect, and there is a timeout until the next attempt
@@ -738,7 +727,8 @@ class SyntheticPasswordManager { 
                    //

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

                    // of VerifyCredentialResponse to be compatible with that.

                    return responseFromTimeout(weaverResponse);

                    return VerifyCredentialResponse.fromTimeout(

                            Duration.ofMillis(weaverResponse.timeout));

                }

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

                    return VerifyCredentialResponse.fromError(

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

+22 −0
Original line number Diff line number Diff line
@@ -825,6 +825,28 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
        assertTrue(response.isMatched());

    }



    @Test

    public void testVerifyCredentialResponseTimeoutClamping() {

        testTimeoutClamping(Duration.ofMillis(Long.MIN_VALUE), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofMillis(Integer.MIN_VALUE), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofMillis(-100), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofMillis(-1), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofNanos(-1), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ZERO, 0);

        testTimeoutClamping(Duration.ofNanos(1), 0);

        testTimeoutClamping(Duration.ofMillis(1), 1);

        testTimeoutClamping(Duration.ofSeconds(1), 1000);

        testTimeoutClamping(Duration.ofSeconds(1000000), 1000000000);

        testTimeoutClamping(Duration.ofMillis(Integer.MAX_VALUE), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofMillis((long) Integer.MAX_VALUE + 1), Integer.MAX_VALUE);

        testTimeoutClamping(Duration.ofMillis(Long.MAX_VALUE), Integer.MAX_VALUE);

    }



    private void testTimeoutClamping(Duration originalTimeout, int expectedClampedTimeout) {

        VerifyCredentialResponse response = VerifyCredentialResponse.fromTimeout(originalTimeout);

        assertEquals(expectedClampedTimeout, response.getTimeout());

    }



    private void checkRecordedFrpNotificationIntent() {

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

            Intent savedNotificationIntent = mService.getSavedFrpNotificationIntent();