diff options
5 files changed, 72 insertions, 34 deletions
diff --git a/services/companion/java/com/android/server/companion/securechannel/SecureChannel.java b/services/companion/java/com/android/server/companion/securechannel/SecureChannel.java index 5a3db4b18a1a..3cb9ac823bac 100644 --- a/services/companion/java/com/android/server/companion/securechannel/SecureChannel.java +++ b/services/companion/java/com/android/server/companion/securechannel/SecureChannel.java @@ -130,6 +130,7 @@ public class SecureChannel { if (DEBUG) { Slog.d(TAG, "Starting secure channel."); } + mStopped = false; new Thread(() -> { try { // 1. Wait for the next handshake message and process it. @@ -185,6 +186,17 @@ public class SecureChannel { } /** + * Return true if the channel is currently inactive. + * The channel could have been stopped by either {@link SecureChannel#stop()} or by + * encountering a fatal error. + * + * @return true if the channel is currently inactive. + */ + public boolean isStopped() { + return mStopped; + } + + /** * Start exchanging handshakes to create a secure layer asynchronously. When the handshake is * completed successfully, then the {@link Callback#onSecureConnection()} will trigger. Any * error that occurs during the handshake will be passed by {@link Callback#onError(Throwable)}. @@ -290,6 +302,7 @@ public class SecureChannel { try { data = new byte[length]; } catch (OutOfMemoryError error) { + Streams.skipByReading(mInput, Long.MAX_VALUE); throw new SecureChannelException("Payload is too large.", error); } diff --git a/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java b/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java index 0f00f5f1c3a5..9498108b35dc 100644 --- a/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java +++ b/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java @@ -137,13 +137,7 @@ public class CompanionTransportManager { synchronized (mTransports) { for (int i = 0; i < associationIds.length; i++) { if (mTransports.contains(associationIds[i])) { - try { - mTransports.get(associationIds[i]).sendMessage(message, data); - } catch (IOException e) { - Slog.e(TAG, "Failed to send message 0x" + Integer.toHexString(message) - + " data length " + data.length + " to association " - + associationIds[i]); - } + mTransports.get(associationIds[i]).requestForResponse(message, data); } } } @@ -244,6 +238,7 @@ public class CompanionTransportManager { } addMessageListenersToTransport(transport); + transport.setOnTransportClosedListener(this::detachSystemDataTransport); transport.start(); synchronized (mTransports) { mTransports.put(associationId, transport); @@ -321,4 +316,14 @@ public class CompanionTransportManager { transport.addListener(mMessageListeners.keyAt(i), mMessageListeners.valueAt(i)); } } + + void detachSystemDataTransport(Transport transport) { + int associationId = transport.mAssociationId; + AssociationInfo association = mAssociationStore.getAssociationById(associationId); + if (association != null) { + detachSystemDataTransport(association.getPackageName(), + association.getUserId(), + association.getId()); + } + } } diff --git a/services/companion/java/com/android/server/companion/transport/RawTransport.java b/services/companion/java/com/android/server/companion/transport/RawTransport.java index e64509facbb4..ca169aac6a37 100644 --- a/services/companion/java/com/android/server/companion/transport/RawTransport.java +++ b/services/companion/java/com/android/server/companion/transport/RawTransport.java @@ -70,6 +70,8 @@ class RawTransport extends Transport { } IoUtils.closeQuietly(mRemoteIn); IoUtils.closeQuietly(mRemoteOut); + + super.close(); } @Override diff --git a/services/companion/java/com/android/server/companion/transport/SecureTransport.java b/services/companion/java/com/android/server/companion/transport/SecureTransport.java index 2d856b9614cb..a0301a920d96 100644 --- a/services/companion/java/com/android/server/companion/transport/SecureTransport.java +++ b/services/companion/java/com/android/server/companion/transport/SecureTransport.java @@ -21,7 +21,6 @@ import android.content.Context; import android.os.ParcelFileDescriptor; import android.util.Slog; -import com.android.internal.annotations.GuardedBy; import com.android.server.companion.securechannel.AttestationVerifier; import com.android.server.companion.securechannel.SecureChannel; @@ -35,7 +34,6 @@ class SecureTransport extends Transport implements SecureChannel.Callback { private volatile boolean mShouldProcessRequests = false; - @GuardedBy("mRequestQueue") private final BlockingQueue<byte[]> mRequestQueue = new ArrayBlockingQueue<>(100); SecureTransport(int associationId, ParcelFileDescriptor fd, Context context) { @@ -64,6 +62,8 @@ class SecureTransport extends Transport implements SecureChannel.Callback { void close() { mSecureChannel.close(); mShouldProcessRequests = false; + + super.close(); } @Override @@ -81,13 +81,19 @@ class SecureTransport extends Transport implements SecureChannel.Callback { } // Queue up a message to send - synchronized (mRequestQueue) { + try { mRequestQueue.add(ByteBuffer.allocate(HEADER_LENGTH + data.length) .putInt(message) .putInt(sequence) .putInt(data.length) .put(data) .array()); + } catch (IllegalStateException e) { + // Request buffer can only be full if too many requests are being added or + // the request processing thread is dead. Assume latter and detach the transport. + Slog.w(TAG, "Failed to queue message 0x" + Integer.toHexString(message) + + " . Request buffer is full; detaching transport.", e); + close(); } } @@ -96,8 +102,8 @@ class SecureTransport extends Transport implements SecureChannel.Callback { try { mSecureChannel.establishSecureConnection(); } catch (Exception e) { - Slog.w(TAG, "Failed to initiate secure channel handshake.", e); - onError(e); + Slog.e(TAG, "Failed to initiate secure channel handshake.", e); + close(); } } @@ -108,17 +114,14 @@ class SecureTransport extends Transport implements SecureChannel.Callback { // TODO: find a better way to handle incoming requests than a dedicated thread. new Thread(() -> { - try { - while (mShouldProcessRequests) { - synchronized (mRequestQueue) { - byte[] request = mRequestQueue.poll(); - if (request != null) { - mSecureChannel.sendSecureMessage(request); - } - } + while (mShouldProcessRequests) { + try { + byte[] request = mRequestQueue.take(); + mSecureChannel.sendSecureMessage(request); + } catch (Exception e) { + Slog.e(TAG, "Failed to send secure message.", e); + close(); } - } catch (IOException e) { - onError(e); } }).start(); } @@ -135,13 +138,18 @@ class SecureTransport extends Transport implements SecureChannel.Callback { try { handleMessage(message, sequence, content); } catch (IOException error) { - onError(error); + // IOException won't be thrown here because a separate thread is handling + // the write operations inside onSecureConnection(). } } @Override public void onError(Throwable error) { - mShouldProcessRequests = false; - Slog.e(TAG, error.getMessage(), error); + Slog.e(TAG, "Secure transport encountered an error.", error); + + // If the channel was stopped as a result of the error, then detach itself. + if (mSecureChannel.isStopped()) { + close(); + } } } diff --git a/services/companion/java/com/android/server/companion/transport/Transport.java b/services/companion/java/com/android/server/companion/transport/Transport.java index 6ad6d3a4aa72..bc9c8694ece5 100644 --- a/services/companion/java/com/android/server/companion/transport/Transport.java +++ b/services/companion/java/com/android/server/companion/transport/Transport.java @@ -70,6 +70,8 @@ public abstract class Transport { */ private final Map<Integer, IOnMessageReceivedListener> mListeners; + private OnTransportClosedListener mOnTransportClosed; + private static boolean isRequest(int message) { return (message & 0xFF000000) == 0x63000000; } @@ -120,20 +122,18 @@ public abstract class Transport { abstract void stop(); /** - * Stop listening to the incoming data and close the streams. + * Stop listening to the incoming data and close the streams. If a listener for closed event + * is set, then trigger it to assist with its clean-up. */ - abstract void close(); + void close() { + if (mOnTransportClosed != null) { + mOnTransportClosed.onClosed(this); + } + } protected abstract void sendMessage(int message, int sequence, @NonNull byte[] data) throws IOException; - /** - * Send a message. - */ - public void sendMessage(int message, @NonNull byte[] data) throws IOException { - sendMessage(message, mNextSequence.incrementAndGet(), data); - } - public Future<byte[]> requestForResponse(int message, byte[] data) { if (DEBUG) Slog.d(TAG, "Requesting for response"); final int sequence = mNextSequence.incrementAndGet(); @@ -247,4 +247,14 @@ public abstract class Transport { } } } + + void setOnTransportClosedListener(OnTransportClosedListener callback) { + this.mOnTransportClosed = callback; + } + + // Interface to pass transport to the transport manager to assist with detachment. + @FunctionalInterface + interface OnTransportClosedListener { + void onClosed(Transport transport); + } } |