diff options
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; |