From 7f794c42a581b649126a6c1008354f8fde1233e9 Mon Sep 17 00:00:00 2001 From: Reema Bajwa Date: Wed, 5 Apr 2023 08:11:53 +0000 Subject: [PATCH] Fix cancellation on the provider side This change makes the aidl interface for CredentialProviderService oneway as per the guidance for transactions going out of system service. Test: built & deployed locally Bug: 270439131 Change-Id: I70a30fe0f1073e418f8046f7e40d58e3bd3e2c0f --- .../CredentialProviderService.java | 25 +++-- .../IBeginCreateCredentialCallback.aidl | 2 + .../IBeginGetCredentialCallback.aidl | 3 + .../IClearCredentialStateCallback.aidl | 10 +- .../ICredentialProviderService.aidl | 8 +- .../credentials/ProviderClearSession.java | 9 +- .../credentials/ProviderCreateSession.java | 9 +- .../credentials/ProviderGetSession.java | 10 +- .../ProviderRegistryGetSession.java | 14 ++- .../credentials/RemoteCredentialService.java | 96 +++++++++++++------ 10 files changed, 133 insertions(+), 53 deletions(-) diff --git a/core/java/android/service/credentials/CredentialProviderService.java b/core/java/android/service/credentials/CredentialProviderService.java index b977606560596..e87333fe49418 100644 --- a/core/java/android/service/credentials/CredentialProviderService.java +++ b/core/java/android/service/credentials/CredentialProviderService.java @@ -231,12 +231,18 @@ public abstract class CredentialProviderService extends Service { } private final ICredentialProviderService mInterface = new ICredentialProviderService.Stub() { - public ICancellationSignal onBeginGetCredential(BeginGetCredentialRequest request, + @Override + public void onBeginGetCredential(BeginGetCredentialRequest request, IBeginGetCredentialCallback callback) { Objects.requireNonNull(request); Objects.requireNonNull(callback); ICancellationSignal transport = CancellationSignal.createTransport(); + try { + callback.onCancellable(transport); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } mHandler.sendMessage(obtainMessage( CredentialProviderService::onBeginGetCredential, @@ -267,7 +273,6 @@ public abstract class CredentialProviderService extends Service { } } )); - return transport; } private void enforceRemoteEntryPermission() { String permission = @@ -280,12 +285,17 @@ public abstract class CredentialProviderService extends Service { } @Override - public ICancellationSignal onBeginCreateCredential(BeginCreateCredentialRequest request, + public void onBeginCreateCredential(BeginCreateCredentialRequest request, IBeginCreateCredentialCallback callback) { Objects.requireNonNull(request); Objects.requireNonNull(callback); ICancellationSignal transport = CancellationSignal.createTransport(); + try { + callback.onCancellable(transport); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } mHandler.sendMessage(obtainMessage( CredentialProviderService::onBeginCreateCredential, @@ -316,16 +326,20 @@ public abstract class CredentialProviderService extends Service { } } )); - return transport; } @Override - public ICancellationSignal onClearCredentialState(ClearCredentialStateRequest request, + public void onClearCredentialState(ClearCredentialStateRequest request, IClearCredentialStateCallback callback) { Objects.requireNonNull(request); Objects.requireNonNull(callback); ICancellationSignal transport = CancellationSignal.createTransport(); + try { + callback.onCancellable(transport); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } mHandler.sendMessage(obtainMessage( CredentialProviderService::onClearCredentialState, @@ -350,7 +364,6 @@ public abstract class CredentialProviderService extends Service { } } )); - return transport; } }; diff --git a/core/java/android/service/credentials/IBeginCreateCredentialCallback.aidl b/core/java/android/service/credentials/IBeginCreateCredentialCallback.aidl index ab855ef0b13fe..4b73cbc77ee32 100644 --- a/core/java/android/service/credentials/IBeginCreateCredentialCallback.aidl +++ b/core/java/android/service/credentials/IBeginCreateCredentialCallback.aidl @@ -1,6 +1,7 @@ package android.service.credentials; import android.service.credentials.BeginCreateCredentialResponse; +import android.os.ICancellationSignal; /** * Interface from the system to a credential provider service. @@ -10,4 +11,5 @@ import android.service.credentials.BeginCreateCredentialResponse; oneway interface IBeginCreateCredentialCallback { void onSuccess(in BeginCreateCredentialResponse request); void onFailure(String errorType, in CharSequence message); + void onCancellable(in ICancellationSignal cancellation); } \ No newline at end of file diff --git a/core/java/android/service/credentials/IBeginGetCredentialCallback.aidl b/core/java/android/service/credentials/IBeginGetCredentialCallback.aidl index 73e98707d15e7..0710fb3a97eae 100644 --- a/core/java/android/service/credentials/IBeginGetCredentialCallback.aidl +++ b/core/java/android/service/credentials/IBeginGetCredentialCallback.aidl @@ -1,6 +1,8 @@ package android.service.credentials; import android.service.credentials.BeginGetCredentialResponse; +import android.os.ICancellationSignal; + /** * Interface from the system to a credential provider service. @@ -10,4 +12,5 @@ import android.service.credentials.BeginGetCredentialResponse; oneway interface IBeginGetCredentialCallback { void onSuccess(in BeginGetCredentialResponse response); void onFailure(String errorType, in CharSequence message); + void onCancellable(in ICancellationSignal cancellation); } \ No newline at end of file diff --git a/core/java/android/service/credentials/IClearCredentialStateCallback.aidl b/core/java/android/service/credentials/IClearCredentialStateCallback.aidl index ec805d0a1799b..57513186fe3d9 100644 --- a/core/java/android/service/credentials/IClearCredentialStateCallback.aidl +++ b/core/java/android/service/credentials/IClearCredentialStateCallback.aidl @@ -16,12 +16,16 @@ package android.service.credentials; +import android.os.ICancellationSignal; + + /** * Callback for onClearCredentialState request. * * @hide */ -interface IClearCredentialStateCallback { - oneway void onSuccess(); - oneway void onFailure(String errorType, CharSequence message); +oneway interface IClearCredentialStateCallback { + void onSuccess(); + void onFailure(String errorType, CharSequence message); + void onCancellable(in ICancellationSignal cancellation); } \ No newline at end of file diff --git a/core/java/android/service/credentials/ICredentialProviderService.aidl b/core/java/android/service/credentials/ICredentialProviderService.aidl index 626dd78586264..ebb4a4ef6cca1 100644 --- a/core/java/android/service/credentials/ICredentialProviderService.aidl +++ b/core/java/android/service/credentials/ICredentialProviderService.aidl @@ -30,8 +30,8 @@ import android.os.ICancellationSignal; * * @hide */ -interface ICredentialProviderService { - ICancellationSignal onBeginGetCredential(in BeginGetCredentialRequest request, in IBeginGetCredentialCallback callback); - ICancellationSignal onBeginCreateCredential(in BeginCreateCredentialRequest request, in IBeginCreateCredentialCallback callback); - ICancellationSignal onClearCredentialState(in ClearCredentialStateRequest request, in IClearCredentialStateCallback callback); +oneway interface ICredentialProviderService { + void onBeginGetCredential(in BeginGetCredentialRequest request, in IBeginGetCredentialCallback callback); + void onBeginCreateCredential(in BeginCreateCredentialRequest request, in IBeginCreateCredentialCallback callback); + void onClearCredentialState(in ClearCredentialStateRequest request, in IClearCredentialStateCallback callback); } diff --git a/services/credentials/java/com/android/server/credentials/ProviderClearSession.java b/services/credentials/java/com/android/server/credentials/ProviderClearSession.java index eaf58f13e2fa3..0c3d3f4ed6d24 100644 --- a/services/credentials/java/com/android/server/credentials/ProviderClearSession.java +++ b/services/credentials/java/com/android/server/credentials/ProviderClearSession.java @@ -23,6 +23,7 @@ import android.credentials.ClearCredentialStateException; import android.credentials.CredentialProviderInfo; import android.credentials.ui.ProviderData; import android.credentials.ui.ProviderPendingIntentResponse; +import android.os.ICancellationSignal; import android.service.credentials.CallingAppInfo; import android.service.credentials.ClearCredentialStateRequest; import android.util.Log; @@ -109,6 +110,11 @@ public final class ProviderClearSession extends ProviderSession>) filterResult + (Function>) filterResult -> filterResult.mCredentialEntries.stream()) .collect(Collectors.toList()); updateStatusAndInvokeCallback(Status.CREDENTIALS_RECEIVED, diff --git a/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java b/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java index ff4e3b6801318..c1e9bc6a5c3ca 100644 --- a/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java +++ b/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java @@ -82,6 +82,9 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { Log.i(TAG, "In onGetCredentials in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); + AtomicReference> futureRef = + new AtomicReference<>(); + CompletableFuture connectThenExecute = postAsync(service -> { CompletableFuture getCredentials = new CompletableFuture<>(); final long originalCallingUidToken = Binder.clearCallingIdentity(); try { - ICancellationSignal cancellationSignal = - service.onBeginGetCredential(request, - new IBeginGetCredentialCallback.Stub() { - @Override - public void onSuccess(BeginGetCredentialResponse response) { - getCredentials.complete(response); - } + service.onBeginGetCredential(request, + new IBeginGetCredentialCallback.Stub() { + @Override + public void onSuccess(BeginGetCredentialResponse response) { + getCredentials.complete(response); + } - @Override - public void onFailure(String errorType, CharSequence message) { - Log.i(TAG, "In onFailure in RemoteCredentialService"); - String errorMsg = message == null ? "" : String.valueOf( - message); - getCredentials.completeExceptionally( - new GetCredentialException(errorType, errorMsg)); - } - }); - cancellationSink.set(cancellationSignal); + @Override + public void onFailure(String errorType, CharSequence message) { + Log.i(TAG, "In onFailure in RemoteCredentialService"); + String errorMsg = message == null ? "" : String.valueOf( + message); + getCredentials.completeExceptionally( + new GetCredentialException(errorType, errorMsg)); + } + + @Override + public void onCancellable(ICancellationSignal cancellation) { + CompletableFuture future = + futureRef.get(); + if (future != null && future.isCancelled()) { + dispatchCancellationSignal(cancellation); + } else { + cancellationSink.set(cancellation); + callback.onProviderCancellable(cancellation); + } + } + }); return getCredentials; } finally { Binder.restoreCallingIdentity(originalCallingUidToken); } }).orTimeout(TIMEOUT_REQUEST_MILLIS, TimeUnit.MILLISECONDS); + futureRef.set(connectThenExecute); connectThenExecute.whenComplete((result, error) -> Handler.getMain().post(() -> handleExecutionResponse(result, error, cancellationSink, callback))); - return cancellationSink.get(); } /** @@ -164,10 +180,12 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { Log.i(TAG, "In onCreateCredential in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); + AtomicReference> futureRef = + new AtomicReference<>(); CompletableFuture connectThenExecute = postAsync(service -> { @@ -175,7 +193,7 @@ public class RemoteCredentialService extends ServiceConnector.Impl(); final long originalCallingUidToken = Binder.clearCallingIdentity(); try { - ICancellationSignal cancellationSignal = service.onBeginCreateCredential( + service.onBeginCreateCredential( request, new IBeginCreateCredentialCallback.Stub() { @Override public void onSuccess(BeginCreateCredentialResponse response) { @@ -192,18 +210,28 @@ public class RemoteCredentialService extends ServiceConnector.Impl future = + futureRef.get(); + if (future != null && future.isCancelled()) { + dispatchCancellationSignal(cancellation); + } else { + cancellationSink.set(cancellation); + callback.onProviderCancellable(cancellation); + } + } }); - cancellationSink.set(cancellationSignal); return createCredentialFuture; } finally { Binder.restoreCallingIdentity(originalCallingUidToken); } }).orTimeout(TIMEOUT_REQUEST_MILLIS, TimeUnit.MILLISECONDS); + futureRef.set(connectThenExecute); connectThenExecute.whenComplete((result, error) -> Handler.getMain().post(() -> handleExecutionResponse(result, error, cancellationSink, callback))); - - return cancellationSink.get(); } /** @@ -214,10 +242,11 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { Log.i(TAG, "In onClearCredentialState in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); + AtomicReference> futureRef = new AtomicReference<>(); CompletableFuture connectThenExecute = postAsync(service -> { @@ -225,7 +254,7 @@ public class RemoteCredentialService extends ServiceConnector.Impl(); final long originalCallingUidToken = Binder.clearCallingIdentity(); try { - ICancellationSignal cancellationSignal = service.onClearCredentialState( + service.onClearCredentialState( request, new IClearCredentialStateCallback.Stub() { @Override public void onSuccess() { @@ -243,18 +272,27 @@ public class RemoteCredentialService extends ServiceConnector.Impl future = futureRef.get(); + if (future != null && future.isCancelled()) { + dispatchCancellationSignal(cancellation); + } else { + cancellationSink.set(cancellation); + callback.onProviderCancellable(cancellation); + } + } }); - cancellationSink.set(cancellationSignal); return clearCredentialFuture; } finally { Binder.restoreCallingIdentity(originalCallingUidToken); } }).orTimeout(TIMEOUT_REQUEST_MILLIS, TimeUnit.MILLISECONDS); + futureRef.set(connectThenExecute); connectThenExecute.whenComplete((result, error) -> Handler.getMain().post(() -> handleExecutionResponse(result, error, cancellationSink, callback))); - - return cancellationSink.get(); } private void handleExecutionResponse(T result,