Merge "Validate input arguments to BlobStoreManager APIs."

3 files changed, 48 insertions, 28 deletions

diff --git a/apex/blobstore/framework/java/android/app/blob/BlobHandle.java b/apex/blobstore/framework/java/android/app/blob/BlobHandle.java
index f7e6a987ded3..ee0ee9894d4b 100644
--- a/apex/blobstore/framework/java/android/app/blob/BlobHandle.java
+++ b/apex/blobstore/framework/java/android/app/blob/BlobHandle.java
@@ -45,6 +45,10 @@ import java.util.Objects;
 public final class BlobHandle implements Parcelable {
     private static final String ALGO_SHA_256 = "SHA-256";
 
+    private static final String[] SUPPORTED_ALGOS = {
+            ALGO_SHA_256
+    };
+
     private static final int LIMIT_BLOB_TAG_LENGTH = 128; // characters
 
     /**
@@ -104,14 +108,9 @@ public final class BlobHandle implements Parcelable {
     public static @NonNull BlobHandle create(@NonNull String algorithm, @NonNull byte[] digest,
             @NonNull CharSequence label, @CurrentTimeMillisLong long expiryTimeMillis,
             @NonNull String tag) {
-        Preconditions.checkNotNull(algorithm, "algorithm must not be null");
-        Preconditions.checkNotNull(digest, "digest must not be null");
-        Preconditions.checkNotNull(label, "label must not be null");
-        Preconditions.checkArgumentNonnegative(expiryTimeMillis,
-                "expiryTimeMillis must not be negative");
-        Preconditions.checkNotNull(tag, "tag must not be null");
-        Preconditions.checkArgument(tag.length() <= LIMIT_BLOB_TAG_LENGTH, "tag too long");
-        return new BlobHandle(algorithm, digest, label, expiryTimeMillis, tag);
+        final BlobHandle handle = new BlobHandle(algorithm, digest, label, expiryTimeMillis, tag);
+        handle.assertIsValid();
+        return handle;
     }
 
     /**
@@ -223,6 +222,17 @@ public final class BlobHandle implements Parcelable {
         fout.println("tag: " + tag);
     }
 
+    /** @hide */
+    public void assertIsValid() {
+        Preconditions.checkArgumentIsSupported(SUPPORTED_ALGOS, algorithm);
+        Preconditions.checkByteArrayNotEmpty(digest, "digest");
+        Preconditions.checkStringNotEmpty(label, "label must not be null");
+        Preconditions.checkArgumentNonnegative(expiryTimeMillis,
+                "expiryTimeMillis must not be negative");
+        Preconditions.checkStringNotEmpty(tag, "tag must not be null");
+        Preconditions.checkArgument(tag.length() <= LIMIT_BLOB_TAG_LENGTH, "tag too long");
+    }
+
     public static final @NonNull Creator<BlobHandle> CREATOR = new Creator<BlobHandle>() {
         @Override
         public @NonNull BlobHandle createFromParcel(@NonNull Parcel source) {
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
index fcc30e30dfaa..dfe441077cab 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
@@ -49,6 +49,7 @@ import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManagerInternal;
+import android.content.res.ResourceId;
 import android.os.Binder;
 import android.os.Handler;
 import android.os.HandlerThread;
@@ -91,6 +92,7 @@ import java.io.PrintWriter;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * Service responsible for maintaining and facilitating access to data blobs published by apps.
@@ -658,10 +660,9 @@ public class BlobStoreManagerService extends SystemService {
         @IntRange(from = 1)
         public long createSession(@NonNull BlobHandle blobHandle,
                 @NonNull String packageName) {
-            Preconditions.checkNotNull(blobHandle, "blobHandle must not be null");
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
-            // TODO: verify blobHandle.algorithm is sha-256
-            // TODO: assert blobHandle is valid.
+            Objects.requireNonNull(blobHandle, "blobHandle must not be null");
+            blobHandle.assertIsValid();
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
@@ -682,7 +683,7 @@ public class BlobStoreManagerService extends SystemService {
                 @NonNull String packageName) {
             Preconditions.checkArgumentPositive(sessionId,
                     "sessionId must be positive: " + sessionId);
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
@@ -695,7 +696,7 @@ public class BlobStoreManagerService extends SystemService {
                 @NonNull String packageName) {
             Preconditions.checkArgumentPositive(sessionId,
                     "sessionId must be positive: " + sessionId);
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
@@ -706,8 +707,9 @@ public class BlobStoreManagerService extends SystemService {
         @Override
         public ParcelFileDescriptor openBlob(@NonNull BlobHandle blobHandle,
                 @NonNull String packageName) {
-            Preconditions.checkNotNull(blobHandle, "blobHandle must not be null");
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
+            Objects.requireNonNull(blobHandle, "blobHandle must not be null");
+            blobHandle.assertIsValid();
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
@@ -727,24 +729,27 @@ public class BlobStoreManagerService extends SystemService {
 
         @Override
         public void acquireLease(@NonNull BlobHandle blobHandle, @IdRes int descriptionResId,
-                @CurrentTimeSecondsLong long leaseTimeoutSecs, @NonNull String packageName) {
-            Preconditions.checkNotNull(blobHandle, "blobHandle must not be null");
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
-            Preconditions.checkArgumentPositive(descriptionResId,
-                    "descriptionResId must be positive; value=" + descriptionResId);
+                @CurrentTimeSecondsLong long leaseExpiryTimeMillis, @NonNull String packageName) {
+            Objects.requireNonNull(blobHandle, "blobHandle must not be null");
+            blobHandle.assertIsValid();
+            Preconditions.checkArgument(ResourceId.isValid(descriptionResId),
+                    "descriptionResId is not valid");
+            Preconditions.checkArgumentNonnegative(leaseExpiryTimeMillis,
+                    "leaseExpiryTimeMillis must not be negative");
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
 
-            acquireLeaseInternal(blobHandle, descriptionResId, leaseTimeoutSecs,
+            acquireLeaseInternal(blobHandle, descriptionResId, leaseExpiryTimeMillis,
                     callingUid, packageName);
         }
 
         @Override
         public void releaseLease(@NonNull BlobHandle blobHandle, @NonNull String packageName) {
-            Preconditions.checkNotNull(blobHandle, "blobHandle must not be null");
-            Preconditions.checkNotNull(packageName, "packageName must not be null");
-
+            Objects.requireNonNull(blobHandle, "blobHandle must not be null");
+            blobHandle.assertIsValid();
+            Objects.requireNonNull(packageName, "packageName must not be null");
 
             final int callingUid = Binder.getCallingUid();
             verifyCallingPackage(callingUid, packageName);
@@ -754,7 +759,7 @@ public class BlobStoreManagerService extends SystemService {
 
         @Override
         public void waitForIdle(@NonNull RemoteCallback remoteCallback) {
-            Preconditions.checkNotNull(remoteCallback, "remoteCallback must not be null");
+            Objects.requireNonNull(remoteCallback, "remoteCallback must not be null");
 
             mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP,
                     "Caller is not allowed to call this; caller=" + Binder.getCallingUid());
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
index 7d1c16653383..40f9f585a29f 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
@@ -62,6 +62,7 @@ import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Objects;
 
 /** TODO: add doc */
 public class BlobStoreSession extends IBlobStoreSession.Stub {
@@ -155,6 +156,8 @@ public class BlobStoreSession extends IBlobStoreSession.Stub {
     @NonNull
     public ParcelFileDescriptor openWrite(@BytesLong long offsetBytes,
             @BytesLong long lengthBytes) {
+        Preconditions.checkArgumentNonnegative(offsetBytes, "offsetBytes must not be negative");
+
         assertCallerIsOwner();
         synchronized (mSessionLock) {
             if (mState != STATE_OPENED) {
@@ -242,7 +245,7 @@ public class BlobStoreSession extends IBlobStoreSession.Stub {
     public void allowPackageAccess(@NonNull String packageName,
             @NonNull byte[] certificate) {
         assertCallerIsOwner();
-        Preconditions.checkNotNull(packageName, "packageName must not be null");
+        Objects.requireNonNull(packageName, "packageName must not be null");
         synchronized (mSessionLock) {
             if (mState != STATE_OPENED) {
                 throw new IllegalStateException("Not allowed to change access type in state: "
@@ -280,7 +283,9 @@ public class BlobStoreSession extends IBlobStoreSession.Stub {
     public boolean isPackageAccessAllowed(@NonNull String packageName,
             @NonNull byte[] certificate) {
         assertCallerIsOwner();
-        Preconditions.checkNotNull(packageName, "packageName must not be null");
+        Objects.requireNonNull(packageName, "packageName must not be null");
+        Preconditions.checkByteArrayNotEmpty(certificate, "certificate");
+
         synchronized (mSessionLock) {
             if (mState != STATE_OPENED) {
                 throw new IllegalStateException("Not allowed to get access type in state: "