Avoid creating AttributionSource when possible

For some permission types (i.e., types which are not associated with an
AppOp), there is no need to create an AttributionSource. Delegate the
decision to the PermissionEnforcer. The variant that supports an
AttributionSource when passed as an explicit argument of the method is
kept.

This drastically improves the performance on the server side. Using
the EnforcePermissionPerfTests benchmark on a Pixel device:

Before:
------------------------------------
android.tests.enforcepermission.tests.ServicePerfTest#testManuallyProtectedLocal:
        manuallyProtectedLocal_mean (ns): 241
        manuallyProtectedLocal_median (ns): 240
        manuallyProtectedLocal_min (ns): 240
        manuallyProtectedLocal_standardDeviation: 3
android.tests.enforcepermission.tests.ServicePerfTest#testAnnotatedPermissionLocal:
        annotatedPermissionLocal_mean (ns): 890
        annotatedPermissionLocal_median (ns): 887
        annotatedPermissionLocal_min (ns): 857
        annotatedPermissionLocal_standardDeviation: 24
android.tests.enforcepermission.tests.ServicePerfTest#testManuallyProtected:
        manuallyProtected_mean (ns): 73641
        manuallyProtected_median (ns): 73270
        manuallyProtected_min (ns): 72602
        manuallyProtected_standardDeviation: 1036
android.tests.enforcepermission.tests.ServicePerfTest#testAnnotatedPermission:
        annotatedPermission_mean (ns): 85346
        annotatedPermission_median (ns): 85268
        annotatedPermission_min (ns): 84765
        annotatedPermission_standardDeviation: 593

After:
------------------------------------
android.tests.enforcepermission.tests.ServicePerfTest#testManuallyProtectedLocal:
        manuallyProtectedLocal_mean (ns): 227
        manuallyProtectedLocal_median (ns): 224
        manuallyProtectedLocal_min (ns): 223
        manuallyProtectedLocal_standardDeviation: 5
android.tests.enforcepermission.tests.ServicePerfTest#testAnnotatedPermissionLocal:
        annotatedPermissionLocal_mean (ns): 279
        annotatedPermissionLocal_median (ns): 278
        annotatedPermissionLocal_min (ns): 269
        annotatedPermissionLocal_standardDeviation: 9
android.tests.enforcepermission.tests.ServicePerfTest#testManuallyProtected:
        manuallyProtected_mean (ns): 73772
        manuallyProtected_median (ns): 73523
        manuallyProtected_min (ns): 73160
        manuallyProtected_standardDeviation: 690
android.tests.enforcepermission.tests.ServicePerfTest#testAnnotatedPermission:
        annotatedPermission_mean (ns): 78132
        annotatedPermission_median (ns): 78083
        annotatedPermission_min (ns): 77450
        annotatedPermission_standardDeviation: 548

Notes on the benchmark execution:
- The warm up time and test duration of
  android.perftests.utils.BenchmarkState were increased to observe more
  stable results (i.e., reduced standard deviation). Warm up (250ms ->
  5s), test iteration (500ms -> 2s). This ensures that any JIT
  optimization occurred before the measurement started.
- In the "after" section, the difference between manually implemented
  and generated is likely attributed to the extra hash map lookup to
  determine if a permission is relying on an AppOp.
- In the "after" section, while there is only a difference of ~50ns
  locally, one would expect no difference in the remote case as this is
  negligible compared to the cost of the binder call. From the perfetto
  trace, it appears that the CPU selection and clock was different and
  resulted in a larger difference (i.e., slower, smaller CPUs were
  selected by the OS). Overall, it is not possible to directly compare
  the remote vs local benchmarks.
- A similar experiment was run with the service embedded within
  system_server. Similar results were observed.

Bug: 269684922
Test: atest EnforcePermissionTests
Test: atest EnforcePermissionPerfTests
Change-Id: Ib372e619033775c442dd55e65df717a11ee16679

5 files changed, 63 insertions, 39 deletions

diff --git a/generate_java_binder.cpp b/generate_java_binder.cpp
index c631e34a..04add061 100644
--- a/generate_java_binder.cpp
+++ b/generate_java_binder.cpp
@@ -472,7 +472,37 @@ static std::shared_ptr<Method> GenerateInterfaceMethod(const AidlInterface& ifac
 // Visitor for the permission declared in the @EnforcePermission annotation.
 class PermissionVisitor {
  public:
-  PermissionVisitor(CodeWriter* code) : code_(code) {}
+  PermissionVisitor(CodeWriter* code, const AidlMethod& method) : code_(code), method_(method) {
+    use_attribution_source_ = std::any_of(
+        method_.GetArguments().begin(), method_.GetArguments().end(), [](const auto& arg) {
+          return arg->GetType().GetName() == "android.content.AttributionSource";
+        });
+  }
+
+  ~PermissionVisitor() {
+    code_->Dedent();
+    *code_ << "}\n";
+  }
+
+  string Credentials() const {
+    if (use_attribution_source_) {
+      return "source";
+    }
+    return "getCallingPid(), getCallingUid()";
+  }
+
+  void Prologue() {
+    *code_ << "/** Helper method to enforce permissions for " << method_.GetName() << " */\n";
+    *code_ << "protected void " << method_.GetName() << "_enforcePermission("
+           << (use_attribution_source_ ? "android.content.AttributionSource source" : "")
+           << ") throws SecurityException {\n";
+    code_->Indent();
+  }
+
+  void AddStaticArrayPermissions(const std::vector<std::string>& permissions) {
+    *code_ << "static final String[] PERMISSIONS_" << method_.GetName() << " = {"
+           << Join(permissions, ", ") << "};\n";
+  }
 
   void operator()(const perm::AllOf& quantifier) {
     std::vector<std::string> permissions;
@@ -480,8 +510,10 @@ class PermissionVisitor {
     for (auto const& permission : quantifier.operands) {
       permissions.push_back(android::aidl::perm::JavaFullName(permission));
     }
-    *code_ << "mEnforcer.enforcePermissionAllOf(new String[]{" << Join(permissions, ", ")
-           << "}, source);\n";
+    AddStaticArrayPermissions(permissions);
+    Prologue();
+    *code_ << "mEnforcer.enforcePermissionAllOf(PERMISSIONS_" << method_.GetName() << ", "
+           << Credentials() << ");\n";
   }
 
   void operator()(const perm::AnyOf& quantifier) {
@@ -490,47 +522,34 @@ class PermissionVisitor {
     for (auto const& permission : quantifier.operands) {
       permissions.push_back(android::aidl::perm::JavaFullName(permission));
     }
-    *code_ << "mEnforcer.enforcePermissionAnyOf(new String[]{" << Join(permissions, ", ")
-           << "}, source);\n";
+    AddStaticArrayPermissions(permissions);
+    Prologue();
+    *code_ << "mEnforcer.enforcePermissionAnyOf(PERMISSIONS_" << method_.GetName() << ", "
+           << Credentials() << ");\n";
   }
 
   void operator()(const std::string& permission) {
     auto permissionName = android::aidl::perm::JavaFullName(permission);
-    *code_ << "mEnforcer.enforcePermission(" << permissionName << ", source);\n";
+    Prologue();
+    *code_ << "mEnforcer.enforcePermission(" << permissionName << ", " << Credentials() << ");\n";
   }
 
  private:
   CodeWriter* code_;
+  const AidlMethod& method_;
+  bool use_attribution_source_;
 };
 
 static void GeneratePermissionMethod(const AidlInterface& iface, const AidlMethod& method,
                                      const std::shared_ptr<Class>& addTo) {
   string code;
   CodeWriterPtr writer = CodeWriter::ForString(&code);
-  *writer << "/** Helper method to enforce permissions for " << method.GetName() << " */\n";
-
-  auto has_attribution_source =
-      std::any_of(method.GetArguments().begin(), method.GetArguments().end(), [](const auto& arg) {
-        return arg->GetType().GetName() == "android.content.AttributionSource";
-      });
-
-  *writer << "protected void " << method.GetName() << "_enforcePermission("
-          << (has_attribution_source ? "android.content.AttributionSource source" : "")
-          << ") throws SecurityException {\n";
-  writer->Indent();
-
-  if (!has_attribution_source) {
-    *writer << "android.content.AttributionSource source = "
-               "new android.content.AttributionSource(getCallingUid(), null, null);\n";
-  }
 
   if (auto ifacePermExpr = iface.EnforceExpression(); ifacePermExpr) {
-    std::visit(PermissionVisitor(writer.get()), *ifacePermExpr.get());
+    std::visit(PermissionVisitor(writer.get(), method), *ifacePermExpr.get());
   } else if (auto methodPermExpr = method.GetType().EnforceExpression(); methodPermExpr) {
-    std::visit(PermissionVisitor(writer.get()), *methodPermExpr.get());
+    std::visit(PermissionVisitor(writer.get(), method), *methodPermExpr.get());
   }
-  writer->Dedent();
-  *writer << "}\n";
   writer->Close();
   addTo->elements.push_back(std::make_shared<LiteralClassElement>(code));
 }
diff --git a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java
index 6c350285..1e10c089 100644
--- a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java
+++ b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java
@@ -249,26 +249,24 @@ public interface IProtected extends android.os.IInterface
     static final int TRANSACTION_PermissionProtected = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
     /** Helper method to enforce permissions for PermissionProtected */
     protected void PermissionProtected_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source);
+      mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, getCallingPid(), getCallingUid());
     }
     static final int TRANSACTION_MultiplePermissionsAll = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
+    static final String[] PERMISSIONS_MultiplePermissionsAll = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE};
     /** Helper method to enforce permissions for MultiplePermissionsAll */
     protected void MultiplePermissionsAll_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+      mEnforcer.enforcePermissionAllOf(PERMISSIONS_MultiplePermissionsAll, getCallingPid(), getCallingUid());
     }
     static final int TRANSACTION_MultiplePermissionsAny = (android.os.IBinder.FIRST_CALL_TRANSACTION + 2);
+    static final String[] PERMISSIONS_MultiplePermissionsAny = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE};
     /** Helper method to enforce permissions for MultiplePermissionsAny */
     protected void MultiplePermissionsAny_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+      mEnforcer.enforcePermissionAnyOf(PERMISSIONS_MultiplePermissionsAny, getCallingPid(), getCallingUid());
     }
     static final int TRANSACTION_NonManifestPermission = (android.os.IBinder.FIRST_CALL_TRANSACTION + 3);
     /** Helper method to enforce permissions for NonManifestPermission */
     protected void NonManifestPermission_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source);
+      mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, getCallingPid(), getCallingUid());
     }
     static final int TRANSACTION_SetGranted = (android.os.IBinder.FIRST_CALL_TRANSACTION + 4);
     /** @hide */
diff --git a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java
index 24f1f2d1..5a97ed0e 100644
--- a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java
+++ b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java
@@ -162,14 +162,12 @@ public interface IProtectedInterface extends android.os.IInterface
     static final int TRANSACTION_Method1 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
     /** Helper method to enforce permissions for Method1 */
     protected void Method1_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+      mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, getCallingPid(), getCallingUid());
     }
     static final int TRANSACTION_Method2 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
     /** Helper method to enforce permissions for Method2 */
     protected void Method2_enforcePermission() throws SecurityException {
-      android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
-      mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+      mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, getCallingPid(), getCallingUid());
     }
     /** @hide */
     public int getMaxTransactionId()
diff --git a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/platform/IProtected.java b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/platform/IProtected.java
index d6fde910..954527fe 100644
--- a/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/platform/IProtected.java
+++ b/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/platform/IProtected.java
@@ -137,9 +137,10 @@ public interface IProtected extends android.os.IInterface
       }
     }
     static final int TRANSACTION_ProtectedWithSourceAttribution = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
+    static final String[] PERMISSIONS_ProtectedWithSourceAttribution = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE};
     /** Helper method to enforce permissions for ProtectedWithSourceAttribution */
     protected void ProtectedWithSourceAttribution_enforcePermission(android.content.AttributionSource source) throws SecurityException {
-      mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+      mEnforcer.enforcePermissionAllOf(PERMISSIONS_ProtectedWithSourceAttribution, source);
     }
     /** @hide */
     public int getMaxTransactionId()
diff --git a/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java b/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java
index 92ec8304..f7b694a5 100644
--- a/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java
+++ b/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java
@@ -31,6 +31,14 @@ public class FakePermissionEnforcer extends android.os.PermissionEnforcer {
   public void setGranted(List<String> granted) { mGranted = granted; }
 
   @Override
+  protected int checkPermission(@NonNull String permission, int pid, int uid) {
+    if (mGranted != null && mGranted.contains(permission)) {
+      return PERMISSION_GRANTED;
+    }
+    return PERMISSION_HARD_DENIED;
+  }
+
+  @Override
   protected int checkPermission(@NonNull String permission, @NonNull AttributionSource source) {
     if (mGranted != null && mGranted.contains(permission)) {
       return PERMISSION_GRANTED;