Validate input arguments to BlobStoreManager APIs.

Bug: 148580257
Test: atest cts/tests/BlobStore/src/com/android/cts/blob/BlobStoreManagerTest.java
Change-Id: Id272c4a9be3ec9300c8cc6d8abdcd287b4d6dacb

4 files changed, 108 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: "
diff --git a/core/java/com/android/internal/util/Preconditions.java b/core/java/com/android/internal/util/Preconditions.java
index 408a7a8e139a..a87e080fa522 100644
--- a/core/java/com/android/internal/util/Preconditions.java
+++ b/core/java/com/android/internal/util/Preconditions.java
@@ -21,7 +21,9 @@ import android.annotation.NonNull;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.text.TextUtils;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Objects;
 
 /**
  * Simple static methods to be called at the start of your own methods to verify
@@ -497,6 +499,64 @@ public class Preconditions {
     }
 
     /**
+     * Ensures that the given byte array is not {@code null}, and contains at least one element.
+     *
+     * @param value an array of elements.
+     * @param valueName the name of the argument to use if the check fails.
+
+     * @return the validated array
+     *
+     * @throws NullPointerException if the {@code value} was {@code null}
+     * @throws IllegalArgumentException if the {@code value} was empty
+     */
+    @NonNull
+    public static byte[] checkByteArrayNotEmpty(final byte[] value, final String valueName) {
+        if (value == null) {
+            throw new NullPointerException(valueName + " must not be null");
+        }
+        if (value.length == 0) {
+            throw new IllegalArgumentException(valueName + " is empty");
+        }
+        return value;
+    }
+
+    /**
+     * Ensures that argument {@code value} is one of {@code supportedValues}.
+     *
+     * @param supportedValues an array of string values
+     * @param value a string value
+     *
+     * @return the validated value
+     *
+     * @throws NullPointerException if either {@code value} or {@code supportedValues} is null
+     * @throws IllegalArgumentException if the {@code value} is not in {@code supportedValues}
+     */
+    @NonNull
+    public static String checkArgumentIsSupported(final String[] supportedValues,
+            final String value) {
+        checkNotNull(value);
+        checkNotNull(supportedValues);
+
+        if (!contains(supportedValues, value)) {
+            throw new IllegalArgumentException(value + "is not supported "
+                    + Arrays.toString(supportedValues));
+        }
+        return value;
+    }
+
+    private static boolean contains(String[] values, String value) {
+        if (values == null) {
+            return false;
+        }
+        for (int i = 0; i < values.length; ++i) {
+            if (Objects.equals(value, values[i])) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
      * Ensures that all elements in the argument floating point array are within the inclusive range
      *
      * <p>While this can be used to range check against +/- infinity, note that all NaN numbers