From 9510ef1aa2248b2a5ed3c24b7a7d885e1618b619 Mon Sep 17 00:00:00 2001 From: Max Bires Date: Wed, 30 Mar 2022 00:17:28 -0700 Subject: [PATCH] Make generateKey() return a status This change adds some integers to the AIDL interface in order to convey status back to the caller of generateKey(). This will inform the caller as to whether or not the errors that may occur during provisioning are permanent, and if not, what to do with the transient error. Bug: 227306369 Test: RemoteProvisionerUnitTests Change-Id: I9202358a102b0fb0a104525632a005acb7355840 --- .../java/android/security/GenerateRkpKey.java | 36 ++++++++-- .../security/IGenerateRkpKeyService.aidl | 28 +++++++- keystore/java/android/security/KeyStore2.java | 6 ++ .../AndroidKeyStoreKeyPairGeneratorSpi.java | 68 ++++++++++++++----- 4 files changed, 113 insertions(+), 25 deletions(-) diff --git a/keystore/java/android/security/GenerateRkpKey.java b/keystore/java/android/security/GenerateRkpKey.java index 2e54e63a5b7aa..698133287f633 100644 --- a/keystore/java/android/security/GenerateRkpKey.java +++ b/keystore/java/android/security/GenerateRkpKey.java @@ -16,6 +16,8 @@ package android.security; +import android.annotation.CheckResult; +import android.annotation.IntDef; import android.content.ComponentName; import android.content.Context; import android.content.Intent; @@ -24,6 +26,8 @@ import android.os.IBinder; import android.os.RemoteException; import android.util.Log; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.Executors; @@ -57,6 +61,21 @@ public class GenerateRkpKey { private Context mContext; private CountDownLatch mCountDownLatch; + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(flag = true, value = { + IGenerateRkpKeyService.Status.OK, + IGenerateRkpKeyService.Status.NO_NETWORK_CONNECTIVITY, + IGenerateRkpKeyService.Status.NETWORK_COMMUNICATION_ERROR, + IGenerateRkpKeyService.Status.DEVICE_NOT_REGISTERED, + IGenerateRkpKeyService.Status.HTTP_CLIENT_ERROR, + IGenerateRkpKeyService.Status.HTTP_SERVER_ERROR, + IGenerateRkpKeyService.Status.HTTP_UNKNOWN_ERROR, + IGenerateRkpKeyService.Status.INTERNAL_ERROR, + }) + public @interface Status { + } + private ServiceConnection mConnection = new ServiceConnection() { @Override public void onServiceConnected(ComponentName className, IBinder service) { @@ -81,12 +100,14 @@ public class GenerateRkpKey { mContext = context; } - private void bindAndSendCommand(int command, int securityLevel) throws RemoteException { + @Status + private int bindAndSendCommand(int command, int securityLevel) throws RemoteException { Intent intent = new Intent(IGenerateRkpKeyService.class.getName()); ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + int returnCode = IGenerateRkpKeyService.Status.OK; if (comp == null) { // On a system that does not use RKP, the RemoteProvisioner app won't be installed. - return; + return returnCode; } intent.setComponent(comp); mCountDownLatch = new CountDownLatch(1); @@ -102,7 +123,7 @@ public class GenerateRkpKey { if (mBinder != null) { switch (command) { case NOTIFY_EMPTY: - mBinder.generateKey(securityLevel); + returnCode = mBinder.generateKey(securityLevel); break; case NOTIFY_KEY_GENERATED: mBinder.notifyKeyGenerated(securityLevel); @@ -112,16 +133,21 @@ public class GenerateRkpKey { } } else { Log.e(TAG, "Binder object is null; failed to bind to GenerateRkpKeyService."); + returnCode = IGenerateRkpKeyService.Status.INTERNAL_ERROR; } mContext.unbindService(mConnection); + return returnCode; } /** * 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. + * @return the status of the key generation */ - public void notifyEmpty(int securityLevel) throws RemoteException { - bindAndSendCommand(NOTIFY_EMPTY, securityLevel); + @CheckResult + @Status + public int notifyEmpty(int securityLevel) throws RemoteException { + return bindAndSendCommand(NOTIFY_EMPTY, securityLevel); } /** diff --git a/keystore/java/android/security/IGenerateRkpKeyService.aidl b/keystore/java/android/security/IGenerateRkpKeyService.aidl index 5f1d6693c23a8..eeaeb27a7c77c 100644 --- a/keystore/java/android/security/IGenerateRkpKeyService.aidl +++ b/keystore/java/android/security/IGenerateRkpKeyService.aidl @@ -26,11 +26,35 @@ package android.security; * @hide */ interface IGenerateRkpKeyService { + @JavaDerive(toString=true) + @Backing(type="int") + enum Status { + /** No error(s) occurred */ + OK = 0, + /** Unable to provision keys due to a lack of internet connectivity. */ + NO_NETWORK_CONNECTIVITY = 1, + /** An error occurred while communicating with the RKP server. */ + NETWORK_COMMUNICATION_ERROR = 2, + /** The given device was not registered with the RKP backend. */ + DEVICE_NOT_REGISTERED = 4, + /** The RKP server returned an HTTP client error, indicating a misbehaving client. */ + HTTP_CLIENT_ERROR = 5, + /** The RKP server returned an HTTP server error, indicating something went wrong on the server. */ + HTTP_SERVER_ERROR = 6, + /** The RKP server returned an HTTP status that is unknown. This should never happen. */ + HTTP_UNKNOWN_ERROR = 7, + /** An unexpected internal error occurred. This should never happen. */ + INTERNAL_ERROR = 8, + } + /** * Ping the provisioner service to let it know an app generated a key. This may or may not have * consumed a remotely provisioned attestation key, so the RemoteProvisioner app should check. */ oneway void notifyKeyGenerated(in int securityLevel); - /** Ping the provisioner service to indicate there are no remaining attestation keys left. */ - void generateKey(in int securityLevel); + + /** + * Ping the provisioner service to indicate there are no remaining attestation keys left. + */ + Status generateKey(in int securityLevel); } diff --git a/keystore/java/android/security/KeyStore2.java b/keystore/java/android/security/KeyStore2.java index 3d53cfb388e12..c2cd6ffc622cf 100644 --- a/keystore/java/android/security/KeyStore2.java +++ b/keystore/java/android/security/KeyStore2.java @@ -345,6 +345,12 @@ public class KeyStore2 { case ResponseCode.KEY_PERMANENTLY_INVALIDATED: return new KeyStoreException(errorCode, "Key permanently invalidated", serviceErrorMessage); + case ResponseCode.OUT_OF_KEYS: + // Getting a more specific RKP status requires the security level, which we + // don't have here. Higher layers of the stack can interpret this exception + // and add more flavor. + return new KeyStoreException(errorCode, serviceErrorMessage, + KeyStoreException.RKP_TEMPORARILY_UNAVAILABLE); default: return new KeyStoreException(errorCode, String.valueOf(errorCode), serviceErrorMessage); diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java index 5950b5bc72317..40659f5dbfb0c 100644 --- a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java +++ b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java @@ -28,6 +28,7 @@ import android.hardware.security.keymint.Tag; import android.os.Build; import android.os.RemoteException; import android.security.GenerateRkpKey; +import android.security.IGenerateRkpKeyService; import android.security.KeyPairGeneratorSpec; import android.security.KeyStore2; import android.security.KeyStoreException; @@ -624,7 +625,7 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato * GenerateRkpKey.notifyEmpty() will delay for a while before returning. */ result = generateKeyPairHelper(); - if (result.rkpStatus == KeyStoreException.RKP_SUCCESS) { + if (result.rkpStatus == KeyStoreException.RKP_SUCCESS && result.keyPair != null) { return result.keyPair; } } @@ -706,27 +707,12 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato success = true; KeyPair kp = new KeyPair(publicKey, publicKey.getPrivateKey()); return new GenerateKeyPairHelperResult(0, kp); - } catch (android.security.KeyStoreException e) { + } catch (KeyStoreException e) { switch (e.getErrorCode()) { case KeymasterDefs.KM_ERROR_HARDWARE_TYPE_UNAVAILABLE: throw new StrongBoxUnavailableException("Failed to generated key pair.", e); case ResponseCode.OUT_OF_KEYS: - GenerateRkpKey keyGen = new GenerateRkpKey(ActivityThread - .currentApplication()); - try { - //TODO: When detailed error information is available from the remote - //provisioner, propagate it up. - keyGen.notifyEmpty(securityLevel); - } catch (RemoteException f) { - KeyStoreException ksException = new KeyStoreException( - ResponseCode.OUT_OF_KEYS, - "Remote exception: " + f.getMessage(), - KeyStoreException.RKP_TEMPORARILY_UNAVAILABLE); - throw new ProviderException("Failed to talk to RemoteProvisioner", - ksException); - } - return new GenerateKeyPairHelperResult( - KeyStoreException.RKP_TEMPORARILY_UNAVAILABLE, null); + throw makeOutOfKeysException(e, securityLevel); default: ProviderException p = new ProviderException("Failed to generate key pair.", e); if ((mSpec.getPurposes() & KeyProperties.PURPOSE_WRAP_KEY) != 0) { @@ -752,6 +738,52 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato } } + // In case keystore reports OUT_OF_KEYS, call this handler in an attempt to remotely provision + // some keys. + private ProviderException makeOutOfKeysException(KeyStoreException e, int securityLevel) { + GenerateRkpKey keyGen = new GenerateRkpKey(ActivityThread + .currentApplication()); + KeyStoreException ksException; + try { + final int keyGenStatus = keyGen.notifyEmpty(securityLevel); + // Default stance: temporary error. This is a hint to the caller to try again with + // exponential back-off. + int rkpStatus; + switch (keyGenStatus) { + case IGenerateRkpKeyService.Status.NO_NETWORK_CONNECTIVITY: + rkpStatus = KeyStoreException.RKP_FETCHING_PENDING_CONNECTIVITY; + break; + case IGenerateRkpKeyService.Status.DEVICE_NOT_REGISTERED: + rkpStatus = KeyStoreException.RKP_SERVER_REFUSED_ISSUANCE; + break; + case IGenerateRkpKeyService.Status.OK: + // This will actually retry once immediately, so on "OK" go ahead and return + // "temporarily unavailable". @see generateKeyPair + case IGenerateRkpKeyService.Status.NETWORK_COMMUNICATION_ERROR: + case IGenerateRkpKeyService.Status.HTTP_CLIENT_ERROR: + case IGenerateRkpKeyService.Status.HTTP_SERVER_ERROR: + case IGenerateRkpKeyService.Status.HTTP_UNKNOWN_ERROR: + case IGenerateRkpKeyService.Status.INTERNAL_ERROR: + default: + // These errors really should never happen. The best we can do is assume they + // are transient and hint to the caller to retry with back-off. + rkpStatus = KeyStoreException.RKP_TEMPORARILY_UNAVAILABLE; + break; + } + ksException = new KeyStoreException( + ResponseCode.OUT_OF_KEYS, + "Out of RKP keys due to IGenerateRkpKeyService status: " + keyGenStatus, + rkpStatus); + } catch (RemoteException f) { + ksException = new KeyStoreException( + ResponseCode.OUT_OF_KEYS, + "Remote exception: " + f.getMessage(), + KeyStoreException.RKP_TEMPORARILY_UNAVAILABLE); + } + ksException.initCause(e); + return new ProviderException("Failed to talk to RemoteProvisioner", ksException); + } + private void addAttestationParameters(@NonNull List params) throws ProviderException, IllegalArgumentException, DeviceIdAttestationException { byte[] challenge = mSpec.getAttestationChallenge();