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

Commit 7bd32592 authored by Seth Moore's avatar Seth Moore
Browse files

Add support for storing upgraded RKP key blobs 

This requires a minor interface update to return an async result,
since failing open means we might leave keys vulnerable to system
software rollback attacks.

Bug: 262748535
Test: RemoteProvisioningRegistrationTest
Change-Id: If2b28dae285631b70d93910a8cc2196f808cb5be
parent ebf1f112

core/java/android/security/rkp/IRegistration.aidl

+7 −4
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.security.rkp;



import android.security.rkp.IGetKeyCallback;

import android.security.rkp.IStoreUpgradedKeyCallback;



/**

 * This interface is associated with the registration of an
@@ -70,16 +71,18 @@ oneway interface IRegistration { 
     * mechanism, see the documentation for IKeyMintDevice.upgradeKey.

     *

     * Once a key has been upgraded, the IRegistration where the key is stored

     * needs to be told about the new blob. After calling storeUpgradedKey,

     * needs to be told about the new blob. After calling storeUpgradedKeyAsync,

     * getKey will return the new key blob instead of the old one.

     *

     * Note that this function does NOT extend the lifetime of key blobs. The

     * certificate for the key is unchanged, and the key will still expire at

     * the same time it would have if storeUpgradedKey had never been called.

     * the same time it would have if storeUpgradedKeyAsync had never been called.

     *

     * @param oldKeyBlob The old key blob to be replaced by {@code newKeyBlob}.

     *

     * @param newKeyblob The new blob to replace {@code oldKeyBlob}.

     * @param callback Receives the result of the call. A callback must only

     * be used with one {@code storeUpgradedKeyAsync} call at a time.

     */

    void storeUpgradedKey(in byte[] oldKeyBlob, in byte[] newKeyBlob);

    void storeUpgradedKeyAsync(

            in byte[] oldKeyBlob, in byte[] newKeyBlob, IStoreUpgradedKeyCallback callback);

}

core/java/android/security/rkp/IStoreUpgradedKeyCallback.aidl

0 → 100644
+39 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package android.security.rkp;



/**

 * Callback interface for storing an upgraded remotely provisioned key blob.

 * {@link IRegistration}.

 *

 * @hide

 */

oneway interface IStoreUpgradedKeyCallback {

    /**

     * Called in response to {@link IRegistration.storeUpgradedKeyAsync}, indicating

     * a remotely-provisioned key is available.

     */

    void onSuccess();



    /**

     * Called when an error has occurred while trying to store an upgraded

     * remotely provisioned key.

     *

     * @param error A description of what failed, suitable for logging.

     */

    void onError(String error);

}

services/core/java/com/android/server/security/rkp/RemoteProvisioningRegistration.java

+39 −9
Original line number Diff line number Diff line
@@ -21,10 +21,12 @@ import android.os.OperationCanceledException; 
import android.os.OutcomeReceiver;

import android.security.rkp.IGetKeyCallback;

import android.security.rkp.IRegistration;

import android.security.rkp.IStoreUpgradedKeyCallback;

import android.security.rkp.service.RegistrationProxy;

import android.security.rkp.service.RemotelyProvisionedKey;

import android.util.Log;



import java.util.Set;

import java.util.concurrent.ConcurrentHashMap;

import java.util.concurrent.Executor;
@@ -36,8 +38,10 @@ import java.util.concurrent.Executor; 
 */

final class RemoteProvisioningRegistration extends IRegistration.Stub {

    static final String TAG = RemoteProvisioningService.TAG;

    private final ConcurrentHashMap<IGetKeyCallback, CancellationSignal> mOperations =

    private final ConcurrentHashMap<IGetKeyCallback, CancellationSignal> mGetKeyOperations =

            new ConcurrentHashMap<>();

    private final Set<IStoreUpgradedKeyCallback> mStoreUpgradedKeyOperations =

            ConcurrentHashMap.newKeySet();

    private final RegistrationProxy mRegistration;

    private final Executor mExecutor;
@@ -49,7 +53,7 @@ final class RemoteProvisioningRegistration extends IRegistration.Stub { 


        @Override

        public void onResult(RemotelyProvisionedKey result) {

            mOperations.remove(mCallback);

            mGetKeyOperations.remove(mCallback);

            Log.i(TAG, "Successfully fetched key for client " + mCallback.hashCode());

            android.security.rkp.RemotelyProvisionedKey parcelable =

                    new android.security.rkp.RemotelyProvisionedKey();
@@ -60,7 +64,7 @@ final class RemoteProvisioningRegistration extends IRegistration.Stub { 


        @Override

        public void onError(Exception e) {

            mOperations.remove(mCallback);

            mGetKeyOperations.remove(mCallback);

            if (e instanceof OperationCanceledException) {

                Log.i(TAG, "Operation cancelled for client " + mCallback.hashCode());

                wrapCallback(mCallback::onCancel);
@@ -79,7 +83,7 @@ final class RemoteProvisioningRegistration extends IRegistration.Stub { 
    @Override

    public void getKey(int keyId, IGetKeyCallback callback) {

        CancellationSignal cancellationSignal = new CancellationSignal();

        if (mOperations.putIfAbsent(callback, cancellationSignal) != null) {

        if (mGetKeyOperations.putIfAbsent(callback, cancellationSignal) != null) {

            Log.e(TAG, "Client can only request one call at a time " + callback.hashCode());

            throw new IllegalArgumentException(

                    "Callback is already associated with an existing operation: "
@@ -92,14 +96,14 @@ final class RemoteProvisioningRegistration extends IRegistration.Stub { 
                    new GetKeyReceiver(callback));

        } catch (Exception e) {

            Log.e(TAG, "getKeyAsync threw an exception for client " + callback.hashCode(), e);

            mOperations.remove(callback);

            mGetKeyOperations.remove(callback);

            wrapCallback(() -> callback.onError(e.getMessage()));

        }

    }



    @Override

    public void cancelGetKey(IGetKeyCallback callback) {

        CancellationSignal cancellationSignal = mOperations.remove(callback);

        CancellationSignal cancellationSignal = mGetKeyOperations.remove(callback);

        if (cancellationSignal == null) {

            throw new IllegalArgumentException(

                    "Invalid client in cancelGetKey: " + callback.hashCode());
@@ -110,9 +114,35 @@ final class RemoteProvisioningRegistration extends IRegistration.Stub { 
    }



    @Override

    public void storeUpgradedKey(byte[] oldKeyBlob, byte[] newKeyBlob) {

        // TODO(b/262748535)

        Log.e(TAG, "RegistrationBinder.storeUpgradedKey NOT YET IMPLEMENTED");

    public void storeUpgradedKeyAsync(byte[] oldKeyBlob, byte[] newKeyBlob,

            IStoreUpgradedKeyCallback callback) {

        if (!mStoreUpgradedKeyOperations.add(callback)) {

            throw new IllegalArgumentException(

                    "Callback is already associated with an existing operation: "

                            + callback.hashCode());

        }



        try {

            mRegistration.storeUpgradedKeyAsync(oldKeyBlob, newKeyBlob, mExecutor,

                    new OutcomeReceiver<>() {

                        @Override

                        public void onResult(Void result) {

                            mStoreUpgradedKeyOperations.remove(callback);

                            wrapCallback(callback::onSuccess);

                        }



                        @Override

                        public void onError(Exception e) {

                            mStoreUpgradedKeyOperations.remove(callback);

                            wrapCallback(() -> callback.onError(e.getMessage()));

                        }

                    });

        } catch (Exception e) {

            Log.e(TAG, "storeUpgradedKeyAsync threw an exception for client "

                    + callback.hashCode(), e);

            mStoreUpgradedKeyOperations.remove(callback);

            wrapCallback(() -> callback.onError(e.getMessage()));

        }

    }



    interface CallbackRunner {

services/tests/RemoteProvisioningServiceTests/src/com/android/server/security/rkp/RemoteProvisioningRegistrationTest.java

+59 −4
Original line number Diff line number Diff line
@@ -34,7 +34,9 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; 
import android.os.CancellationSignal;

import android.os.OperationCanceledException;

import android.os.OutcomeReceiver;

import android.os.RemoteException;

import android.security.rkp.IGetKeyCallback;

import android.security.rkp.IStoreUpgradedKeyCallback;

import android.security.rkp.service.RegistrationProxy;

import android.security.rkp.service.RemotelyProvisionedKey;
@@ -72,6 +74,12 @@ public class RemoteProvisioningRegistrationTest { 
        return answerVoid(answer);

    }



    // answerVoid wrapper for mocking storeUpgradeKeyAsync.

    static Answer<Void> answerStoreUpgradedKeyAsync(

            VoidAnswer4<byte[], byte[], Executor, OutcomeReceiver<Void, Exception>> answer) {

        return answerVoid(answer);

    }



    // matcher helper, making it easier to match the different key types

    private android.security.rkp.RemotelyProvisionedKey matches(

            RemotelyProvisionedKey expectedKey) {
@@ -178,16 +186,63 @@ public class RemoteProvisioningRegistrationTest { 


    @Test

    public void storeUpgradedKeySuccess() throws Exception {

        // TODO(b/262748535)

        doAnswer(

                answerStoreUpgradedKeyAsync((oldBlob, newBlob, executor, receiver) ->

                        executor.execute(() -> receiver.onResult(null))))

                .when(mRegistrationProxy)

                .storeUpgradedKeyAsync(any(), any(), any(), any());



        IStoreUpgradedKeyCallback callback = mock(IStoreUpgradedKeyCallback.class);

        mRegistration.storeUpgradedKeyAsync(new byte[0], new byte[0], callback);

        verify(callback).onSuccess();

        verifyNoMoreInteractions(callback);

    }



    @Test

    public void storeUpgradedKeyFails() throws Exception {

        // TODO(b/262748535)

        final String errorString = "this is a failure";

        doAnswer(

                answerStoreUpgradedKeyAsync((oldBlob, newBlob, executor, receiver) ->

                        executor.execute(() -> receiver.onError(new RemoteException(errorString)))))

                .when(mRegistrationProxy)

                .storeUpgradedKeyAsync(any(), any(), any(), any());



        IStoreUpgradedKeyCallback callback = mock(IStoreUpgradedKeyCallback.class);

        mRegistration.storeUpgradedKeyAsync(new byte[0], new byte[0], callback);

        verify(callback).onError(errorString);

        verifyNoMoreInteractions(callback);

    }



    @Test

    public void storeUpgradedKeyHandlesException() throws Exception {

        final String errorString = "all aboard the failboat, toot toot";

        doThrow(new IllegalArgumentException(errorString))

                .when(mRegistrationProxy)

                .storeUpgradedKeyAsync(any(), any(), any(), any());



        IStoreUpgradedKeyCallback callback = mock(IStoreUpgradedKeyCallback.class);

        mRegistration.storeUpgradedKeyAsync(new byte[0], new byte[0], callback);

        verify(callback).onError(errorString);

        verifyNoMoreInteractions(callback);

    }



    @Test

    public void storeUpgradedCatchesExceptionFromProxy() throws Exception {

        // TODO(b/262748535)

    public void storeUpgradedKeyDuplicateCallback() throws Exception {

        IStoreUpgradedKeyCallback callback = mock(IStoreUpgradedKeyCallback.class);



        doAnswer(

                answerStoreUpgradedKeyAsync((oldBlob, newBlob, executor, receiver) -> {

                    assertThrows(IllegalArgumentException.class,

                            () -> mRegistration.storeUpgradedKeyAsync(new byte[0], new byte[0],

                                    callback));

                    executor.execute(() -> receiver.onResult(null));

                }))

                .when(mRegistrationProxy)

                .storeUpgradedKeyAsync(any(), any(), any(), any());



        mRegistration.storeUpgradedKeyAsync(new byte[0], new byte[0], callback);

        verify(callback).onSuccess();

        verifyNoMoreInteractions(callback);

    }



}