From 7df6694f8f5b9d40aa3c008887bdc158dbd7e2f7 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 21 Apr 2023 14:59:28 +0000 Subject: Revert "Avoid creating AttributionSource when possible" This reverts commit 6b6274290d1060146c6e598127dd287e7d1695d2. Reason for revert: b/279114919 Bug: 279114919 Change-Id: I2fe1cce566e01966722f51a58610d28fc6a69888 Merged-In: I2fe1cce566e01966722f51a58610d28fc6a69888 --- generate_java_binder.cpp | 71 ++++++++-------------- .../android/aidl/tests/permission/IProtected.java | 14 +++-- .../aidl/tests/permission/IProtectedInterface.java | 6 +- .../aidl/tests/permission/platform/IProtected.java | 3 +- .../permission/service/FakePermissionEnforcer.java | 8 --- 5 files changed, 39 insertions(+), 63 deletions(-) diff --git a/generate_java_binder.cpp b/generate_java_binder.cpp index 04add061..c631e34a 100644 --- a/generate_java_binder.cpp +++ b/generate_java_binder.cpp @@ -472,37 +472,7 @@ static std::shared_ptr GenerateInterfaceMethod(const AidlInterface& ifac // Visitor for the permission declared in the @EnforcePermission annotation. class PermissionVisitor { public: - 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& permissions) { - *code_ << "static final String[] PERMISSIONS_" << method_.GetName() << " = {" - << Join(permissions, ", ") << "};\n"; - } + PermissionVisitor(CodeWriter* code) : code_(code) {} void operator()(const perm::AllOf& quantifier) { std::vector permissions; @@ -510,10 +480,8 @@ class PermissionVisitor { for (auto const& permission : quantifier.operands) { permissions.push_back(android::aidl::perm::JavaFullName(permission)); } - AddStaticArrayPermissions(permissions); - Prologue(); - *code_ << "mEnforcer.enforcePermissionAllOf(PERMISSIONS_" << method_.GetName() << ", " - << Credentials() << ");\n"; + *code_ << "mEnforcer.enforcePermissionAllOf(new String[]{" << Join(permissions, ", ") + << "}, source);\n"; } void operator()(const perm::AnyOf& quantifier) { @@ -522,34 +490,47 @@ class PermissionVisitor { for (auto const& permission : quantifier.operands) { permissions.push_back(android::aidl::perm::JavaFullName(permission)); } - AddStaticArrayPermissions(permissions); - Prologue(); - *code_ << "mEnforcer.enforcePermissionAnyOf(PERMISSIONS_" << method_.GetName() << ", " - << Credentials() << ");\n"; + *code_ << "mEnforcer.enforcePermissionAnyOf(new String[]{" << Join(permissions, ", ") + << "}, source);\n"; } void operator()(const std::string& permission) { auto permissionName = android::aidl::perm::JavaFullName(permission); - Prologue(); - *code_ << "mEnforcer.enforcePermission(" << permissionName << ", " << Credentials() << ");\n"; + *code_ << "mEnforcer.enforcePermission(" << permissionName << ", source);\n"; } private: CodeWriter* code_; - const AidlMethod& method_; - bool use_attribution_source_; }; static void GeneratePermissionMethod(const AidlInterface& iface, const AidlMethod& method, const std::shared_ptr& 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(), method), *ifacePermExpr.get()); + std::visit(PermissionVisitor(writer.get()), *ifacePermExpr.get()); } else if (auto methodPermExpr = method.GetType().EnforceExpression(); methodPermExpr) { - std::visit(PermissionVisitor(writer.get(), method), *methodPermExpr.get()); + std::visit(PermissionVisitor(writer.get()), *methodPermExpr.get()); } + writer->Dedent(); + *writer << "}\n"; writer->Close(); addTo->elements.push_back(std::make_shared(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 1e10c089..6c350285 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,24 +249,26 @@ 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 { - mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source); } 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 { - mEnforcer.enforcePermissionAllOf(PERMISSIONS_MultiplePermissionsAll, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); } 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 { - mEnforcer.enforcePermissionAnyOf(PERMISSIONS_MultiplePermissionsAny, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); } 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 { - mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source); } 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 5a97ed0e..24f1f2d1 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,12 +162,14 @@ 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 { - mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); } 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 { - mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, getCallingPid(), getCallingUid()); + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); } /** @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 954527fe..d6fde910 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,10 +137,9 @@ 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(PERMISSIONS_ProtectedWithSourceAttribution, source); + mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, 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 f7b694a5..92ec8304 100644 --- a/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java +++ b/tests/java/src/android/aidl/permission/service/FakePermissionEnforcer.java @@ -30,14 +30,6 @@ public class FakePermissionEnforcer extends android.os.PermissionEnforcer { public void setGranted(List 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)) { -- cgit v1.2.3