binder: SecurityPolicy updates (take 2). (#8637)

The previous attempt at this CL relied on guava's Hashing class which
is still in beta. This update compares Signature objects directly instead
of SHA256 hashs, removing the need for the Hashing class.

Add additional comments to the security policy class, to mention that
implementing new policies requires significant care.

With that in mind, add security policies to check the peer app's
signature, so people can create cross-app communication without
having to implement their own policy.

Finally, add the UntrustedSecurityPolicies class, since that's
inevitably a policy which is sometimes needed.

binder/src/main/java/io/grpc/binder/SecurityPolicies.java

@@ -16,9 +16,20 @@
 
 package io.grpc.binder;
 
+import android.annotation.SuppressLint;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.content.pm.PackageManager.NameNotFoundException;
+import android.content.pm.Signature;
+import android.os.Build;
 import android.os.Process;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import io.grpc.ExperimentalApi;
 import io.grpc.Status;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
 import javax.annotation.CheckReturnValue;
 
 /** Static factory methods for creating standard security policies. */
@@ -55,4 +66,125 @@ public final class SecurityPolicies {
       }
     };
   }
+
+  /**
+   * Creates a {@link SecurityPolicy} which checks if the package signature
+   * matches {@code requiredSignature}.
+   *
+   * @param packageName the package name of the allowed package.
+   * @param requiredSignature the allowed signature of the allowed package.
+   * @throws NullPointerException if any of the inputs are {@code null}.
+   */
+  public static SecurityPolicy hasSignature(
+      PackageManager packageManager, String packageName, Signature requiredSignature) {
+    return oneOfSignatures(
+        packageManager, packageName, ImmutableList.of(requiredSignature));
+  }
+
+  /**
+   * Creates a {@link SecurityPolicy} which checks if the package signature
+   * matches any of {@code requiredSignatures}.
+   *
+   * @param packageName the package name of the allowed package.
+   * @param requiredSignatures the allowed signatures of the allowed package.
+   * @throws NullPointerException if any of the inputs are {@code null}.
+   * @throws IllegalArgumentException if {@code requiredSignatures} is empty.
+   */
+  public static SecurityPolicy oneOfSignatures(
+      PackageManager packageManager,
+      String packageName,
+      Collection<Signature> requiredSignatures) {
+    Preconditions.checkNotNull(packageManager, "packageManager");
+    Preconditions.checkNotNull(packageName, "packageName");
+    Preconditions.checkNotNull(requiredSignatures, "requiredSignatures");
+    Preconditions.checkArgument(!requiredSignatures.isEmpty(),
+        "requiredSignatures");
+    ImmutableList<Signature> requiredSignaturesImmutable = ImmutableList.copyOf(requiredSignatures);
+
+    for (Signature requiredSignature : requiredSignaturesImmutable) {
+      Preconditions.checkNotNull(requiredSignature);
+    }
+
+    return new SecurityPolicy() {
+      @Override
+      public Status checkAuthorization(int uid) {
+        return checkUidSignature(
+            packageManager, uid, packageName, requiredSignaturesImmutable);
+      }
+    };
+  }
+
+  private static Status checkUidSignature(
+      PackageManager packageManager,
+      int uid,
+      String packageName,
+      ImmutableList<Signature> requiredSignatures) {
+    String[] packages = packageManager.getPackagesForUid(uid);
+    if (packages == null) {
+      return Status.UNAUTHENTICATED.withDescription(
+          "Rejected by signature check security policy");
+    }
+    boolean packageNameMatched = false;
+    for (String pkg : packages) {
+      if (!packageName.equals(pkg)) {
+        continue;
+      }
+      packageNameMatched = true;
+      if (checkPackageSignature(packageManager, pkg, requiredSignatures)) {
+        return Status.OK;
+      }
+    }
+    return Status.PERMISSION_DENIED.withDescription(
+        "Rejected by signature check security policy. Package name matched: "
+            + packageNameMatched);
+  }
+
+  /**
+   * Checks if the signature of {@code packageName} matches one of the given signatures.
+   *
+   * @param packageName the package to be checked
+   * @param requiredSignatures list of signatures.
+   * @return {@code true} if {@code packageName} has a matching signature.
+   */
+  @SuppressWarnings("deprecation") // For PackageInfo.signatures
+  @SuppressLint("PackageManagerGetSignatures") // We only allow 1 signature.
+  private static boolean checkPackageSignature(
+      PackageManager packageManager,
+      String packageName,
+      ImmutableList<Signature> requiredSignatures) {
+    PackageInfo packageInfo;
+    try {
+      if (Build.VERSION.SDK_INT >= 28) {
+        packageInfo =
+            packageManager.getPackageInfo(packageName, PackageManager.GET_SIGNING_CERTIFICATES);
+        if (packageInfo.signingInfo == null) {
+          return false;
+        }
+        Signature[] signatures =
+            packageInfo.signingInfo.hasMultipleSigners()
+                ? packageInfo.signingInfo.getApkContentsSigners()
+                : packageInfo.signingInfo.getSigningCertificateHistory();
+
+        for (Signature signature : signatures) {
+          if (requiredSignatures.contains(signature)) {
+            return true;
+          }
+        }
+      } else {
+        packageInfo = packageManager.getPackageInfo(packageName, PackageManager.GET_SIGNATURES);
+        if (packageInfo.signatures == null || packageInfo.signatures.length != 1) {
+          // Reject multiply-signed apks because of b/13678484
+          // (See PackageManagerGetSignatures supression above).
+          return false;
+        }
+
+        if (requiredSignatures.contains(packageInfo.signatures[0])) {
+          return true;
+        }
+      }
+    } catch (NameNotFoundException nnfe) {
+      return false;
+    }
+    return false;
+  }
 }

binder/src/main/java/io/grpc/binder/SecurityPolicy.java

@@ -23,6 +23,11 @@ import javax.annotation.CheckReturnValue;
 /**
  * Decides whether a given Android UID is authorized to access some resource.
  *
+ * While it's possible to extend this class to define your own policy, it's strongly
+ * recommended that you only use the policies provided by the {@link SecurityPolicies} or
+ * {@link UntrustedSecurityPolicies} classes. Implementing your own security policy requires
+ * significant care, and an understanding of the details and pitfalls of Android security.
+ *
  * <p><b>IMPORTANT</b> For any concrete extensions of this class, it's assumed that the
  * authorization status of a given UID will <b>not</b> change as long as a process with that UID is
  * alive.

binder/src/main/java/io/grpc/binder/UntrustedSecurityPolicies.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2021 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.binder;
+
+import io.grpc.ExperimentalApi;
+import io.grpc.Status;
+import javax.annotation.CheckReturnValue;
+
+/**
+ * Static factory methods for creating untrusted security policies.
+ */
+@CheckReturnValue
+@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8022")
+public final class UntrustedSecurityPolicies {
+
+  private UntrustedSecurityPolicies() {}
+
+  /**
+   * Return a security policy which allows any peer on device.
+   * Servers should only use this policy if they intend to expose
+   * a service to all applications on device.
+   * Clients should only use this policy if they don't need to trust the
+   * application they're connecting to.
+   */
+  public static SecurityPolicy untrustedPublic() {
+    return new SecurityPolicy() {
+      @Override
+      public Status checkAuthorization(int uid) {
+        return Status.OK;
+      }
+    };
+  }
+}

binder/src/test/java/io/grpc/binder/SecurityPoliciesTest.java

@@ -17,22 +17,64 @@
 package io.grpc.binder;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.robolectric.Shadows.shadowOf;
 
+import android.content.Context;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.content.pm.Signature;
 import android.os.Process;
+import androidx.test.core.app.ApplicationProvider;
+import com.google.common.collect.ImmutableList;
 import io.grpc.Status;
+import io.grpc.binder.SecurityPolicy;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 
 @RunWith(RobolectricTestRunner.class)
 public final class SecurityPoliciesTest {
+
   private static final int MY_UID = Process.myUid();
   private static final int OTHER_UID = MY_UID + 1;
+  private static final int OTHER_UID_SAME_SIGNATURE = MY_UID + 2;
+  private static final int OTHER_UID_NO_SIGNATURE = MY_UID + 3;
+  private static final int OTHER_UID_UNKNOWN = MY_UID + 4;
 
   private static final String PERMISSION_DENIED_REASONS = "some reasons";
 
+  private static final Signature SIG1 = new Signature("1234");
+  private static final Signature SIG2 = new Signature("4321");
+
+  private static final String OTHER_UID_PACKAGE_NAME = "other.package";
+  private static final String OTHER_UID_SAME_SIGNATURE_PACKAGE_NAME = "other.package.samesignature";
+  private static final String OTHER_UID_NO_SIGNATURE_PACKAGE_NAME = "other.package.nosignature";
+
+  private Context appContext;
+  private PackageManager packageManager;
+
   private SecurityPolicy policy;
 
+  @Before
+  public void setUp() {
+    appContext = ApplicationProvider.getApplicationContext();
+    packageManager = appContext.getPackageManager();
+    installPackage(MY_UID, appContext.getPackageName(), SIG1);
+    installPackage(OTHER_UID, OTHER_UID_PACKAGE_NAME, SIG2);
+    installPackage(OTHER_UID_SAME_SIGNATURE, OTHER_UID_SAME_SIGNATURE_PACKAGE_NAME, SIG1);
+    installPackage(OTHER_UID_NO_SIGNATURE, OTHER_UID_NO_SIGNATURE_PACKAGE_NAME);
+  }
+
+  @SuppressWarnings("deprecation")
+  private void installPackage(int uid, String packageName, Signature... signatures) {
+    PackageInfo info = new PackageInfo();
+    info.packageName = packageName;
+    info.signatures = signatures;
+    shadowOf(packageManager).installPackage(info);
+    shadowOf(packageManager).setPackagesForUid(uid, packageName);
+  }
+
   @Test
   public void testInternalOnly() throws Exception {
     policy = SecurityPolicies.internalOnly();
@@ -53,4 +95,80 @@ public final class SecurityPoliciesTest {
     assertThat(policy.checkAuthorization(OTHER_UID).getDescription())
         .isEqualTo(PERMISSION_DENIED_REASONS);
   }
+
+  @Test
+  public void testHasSignature_succeedsIfPackageNameAndSignaturesMatch()
+      throws Exception {
+    policy = SecurityPolicies.hasSignature(packageManager, OTHER_UID_PACKAGE_NAME, SIG2);
+
+    // THEN UID for package that has SIG2 will be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID).getCode()).isEqualTo(Status.OK.getCode());
+  }
+
+  @Test
+  public void testHasSignature_failsIfPackageNameDoesNotMatch() throws Exception {
+    policy = SecurityPolicies.hasSignature(packageManager, appContext.getPackageName(), SIG1);
+
+    // THEN UID for package that has SIG1 but different package name will not be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID_SAME_SIGNATURE).getCode())
+        .isEqualTo(Status.PERMISSION_DENIED.getCode());
+  }
+
+  @Test
+  public void testHasSignature_failsIfSignatureDoesNotMatch() throws Exception {
+    policy = SecurityPolicies.hasSignature(packageManager, OTHER_UID_PACKAGE_NAME, SIG1);
+
+    // THEN UID for package that doesn't have SIG1 will not be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID).getCode())
+        .isEqualTo(Status.PERMISSION_DENIED.getCode());
+  }
+
+  @Test
+  public void testOneOfSignatures_succeedsIfPackageNameAndSignaturesMatch()
+      throws Exception {
+    policy =
+        SecurityPolicies.oneOfSignatures(
+            packageManager, OTHER_UID_PACKAGE_NAME, ImmutableList.of(SIG2));
+
+    // THEN UID for package that has SIG2 will be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID).getCode()).isEqualTo(Status.OK.getCode());
+  }
+
+  @Test
+  public void testOneOfSignature_failsIfAllSignaturesDoNotMatch() throws Exception {
+    policy =
+        SecurityPolicies.oneOfSignatures(
+            packageManager,
+            appContext.getPackageName(),
+            ImmutableList.of(SIG1, new Signature("1314")));
+
+    // THEN UID for package that has SIG1 but different package name will not be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID_SAME_SIGNATURE).getCode())
+        .isEqualTo(Status.PERMISSION_DENIED.getCode());
+  }
+
+  @Test
+  public void testOneOfSignature_succeedsIfPackageNameAndOneOfSignaturesMatch()
+      throws Exception {
+    policy =
+        SecurityPolicies.oneOfSignatures(
+            packageManager,
+            OTHER_UID_PACKAGE_NAME,
+            ImmutableList.of(SIG1, SIG2));
+
+    // THEN UID for package that has SIG2 will be authorized
+    assertThat(policy.checkAuthorization(OTHER_UID).getCode()).isEqualTo(Status.OK.getCode());
+  }
+
+  @Test
+  public void testHasSignature_failsIfUidUnknown() throws Exception {
+    policy =
+        SecurityPolicies.hasSignature(
+          packageManager,
+          appContext.getPackageName(),
+          SIG1);
+
+    assertThat(policy.checkAuthorization(OTHER_UID_UNKNOWN).getCode())
+      .isEqualTo(Status.UNAUTHENTICATED.getCode());
+  }
 }