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

Commit d05498b9 authored by Max Bires's avatar Max Bires
Browse files

Fixing the race condition in GenerateRkpKey

This file was written on the assumption that bindService was
synchronous, which it isn't. This change adds a CountDownLatch to force
the class to wait for the binding to finish.

Bug: 190222116
Test: atest RemoteProvisionerUnitTests
Change-Id: I917a61da612f21f9a0f783bea5d24270d4e1db42
parent ff3d2907
Loading
Loading
Loading
Loading
+49 −19
Original line number Original line Diff line number Diff line
@@ -22,6 +22,10 @@ import android.content.Intent;
import android.content.ServiceConnection;
import android.content.ServiceConnection;
import android.os.IBinder;
import android.os.IBinder;
import android.os.RemoteException;
import android.os.RemoteException;
import android.util.Log;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;


/**
/**
 * GenerateKey is a helper class to handle interactions between Keystore and the RemoteProvisioner
 * GenerateKey is a helper class to handle interactions between Keystore and the RemoteProvisioner
@@ -41,14 +45,25 @@ import android.os.RemoteException;
 * @hide
 * @hide
 */
 */
public class GenerateRkpKey {
public class GenerateRkpKey {
    private static final String TAG = "GenerateRkpKey";

    private static final int NOTIFY_EMPTY = 0;
    private static final int NOTIFY_KEY_GENERATED = 1;
    private static final int TIMEOUT_MS = 1000;


    private IGenerateRkpKeyService mBinder;
    private IGenerateRkpKeyService mBinder;
    private Context mContext;
    private Context mContext;
    private CountDownLatch mCountDownLatch;


    private ServiceConnection mConnection = new ServiceConnection() {
    private ServiceConnection mConnection = new ServiceConnection() {
        @Override
        @Override
        public void onServiceConnected(ComponentName className, IBinder service) {
        public void onServiceConnected(ComponentName className, IBinder service) {
            mBinder = IGenerateRkpKeyService.Stub.asInterface(service);
            mBinder = IGenerateRkpKeyService.Stub.asInterface(service);
            mCountDownLatch.countDown();
        }

        @Override public void onBindingDied(ComponentName className) {
            mCountDownLatch.countDown();
        }
        }


        @Override
        @Override
@@ -64,36 +79,51 @@ public class GenerateRkpKey {
        mContext = context;
        mContext = context;
    }
    }


    /**
    private void bindAndSendCommand(int command, int securityLevel) throws RemoteException {
     * Fulfills the use case of (2) described in the class documentation. Blocks until the
     * RemoteProvisioner application can get new attestation keys signed by the server.
     */
    public void notifyEmpty(int securityLevel) throws RemoteException {
        Intent intent = new Intent(IGenerateRkpKeyService.class.getName());
        Intent intent = new Intent(IGenerateRkpKeyService.class.getName());
        ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0);
        ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0);
        if (comp == null) {
            throw new RemoteException("Could not resolve GenerateRkpKeyService.");
        }
        intent.setComponent(comp);
        intent.setComponent(comp);
        if (comp == null || !mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
        mCountDownLatch = new CountDownLatch(1);
            throw new RemoteException("Failed to bind to GenerateKeyService");
        if (!mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
            throw new RemoteException("Failed to bind to GenerateRkpKeyService");
        }
        try {
            mCountDownLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS);
        } catch (InterruptedException e) {
            Log.e(TAG, "Interrupted: ", e);
        }
        }
        if (mBinder != null) {
        if (mBinder != null) {
            switch (command) {
                case NOTIFY_EMPTY:
                    mBinder.generateKey(securityLevel);
                    mBinder.generateKey(securityLevel);
                    break;
                case NOTIFY_KEY_GENERATED:
                    mBinder.notifyKeyGenerated(securityLevel);
                    break;
                default:
                    Log.e(TAG, "Invalid case for command");
            }
        } else {
            Log.e(TAG, "Binder object is null; failed to bind to GenerateRkpKeyService.");
        }
        }
        mContext.unbindService(mConnection);
        mContext.unbindService(mConnection);
    }
    }


    /**
    /**
     * FUlfills the use case of (1) described in the class documentation. Non blocking call.
     * Fulfills the use case of (2) described in the class documentation. Blocks until the
     * RemoteProvisioner application can get new attestation keys signed by the server.
     */
    public void notifyEmpty(int securityLevel) throws RemoteException {
        bindAndSendCommand(NOTIFY_EMPTY, securityLevel);
    }

    /**
     * Fulfills the use case of (1) described in the class documentation. Non blocking call.
     */
     */
    public void notifyKeyGenerated(int securityLevel) throws RemoteException {
    public void notifyKeyGenerated(int securityLevel) throws RemoteException {
        Intent intent = new Intent(IGenerateRkpKeyService.class.getName());
        bindAndSendCommand(NOTIFY_KEY_GENERATED, securityLevel);
        ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0);
        intent.setComponent(comp);
        if (comp == null || !mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
            throw new RemoteException("Failed to bind to GenerateKeyService");
        }
        if (mBinder != null) {
            mBinder.notifyKeyGenerated(securityLevel);
        }
        mContext.unbindService(mConnection);
    }
    }
}
}
+1 −1
Original line number Original line Diff line number Diff line
@@ -580,7 +580,7 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato
            } catch (RemoteException e) {
            } catch (RemoteException e) {
                // This is not really an error state, and necessarily does not apply to non RKP
                // This is not really an error state, and necessarily does not apply to non RKP
                // systems or hybrid systems where RKP is not currently turned on.
                // systems or hybrid systems where RKP is not currently turned on.
                Log.d(TAG, "Couldn't connect to the RemoteProvisioner backend.");
                Log.d(TAG, "Couldn't connect to the RemoteProvisioner backend.", e);
            }
            }
            success = true;
            success = true;
            return new KeyPair(publicKey, publicKey.getPrivateKey());
            return new KeyPair(publicKey, publicKey.getPrivateKey());