Add further permission annotations
Introduce the @NoPermissionRequired annotation to explicitly declare
that an interface or method does not require any kind of permission when
called.
Add the @PermissionManuallyEnforced annotation to indicate that
permissions are manually verified within the implementation.
Ensure that these new annotations are not defined in a conflicted manner
with the @Enforce annotation.
Bug: 197828948
Test: atest --host aidl_unittests
Change-Id: Iff3b4c9c5d2a5b4c310d7a737b8d7628cc39ba20
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 3aa8fbc..25a1bdb 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -179,10 +179,18 @@
"SuppressWarnings",
CONTEXT_TYPE | CONTEXT_MEMBER,
{{"value", kStringArrayType, /* required= */ true}}},
- {AidlAnnotation::Type::ENFORCE,
+ {AidlAnnotation::Type::PERMISSION_ENFORCE,
"Enforce",
CONTEXT_TYPE_INTERFACE | CONTEXT_METHOD,
{{"condition", kStringType, /* required= */ true}}},
+ {AidlAnnotation::Type::PERMISSION_MANUAL,
+ "PermissionManuallyEnforced",
+ CONTEXT_TYPE_INTERFACE | CONTEXT_METHOD,
+ {}},
+ {AidlAnnotation::Type::PERMISSION_NONE,
+ "NoPermissionRequired",
+ CONTEXT_TYPE_INTERFACE | CONTEXT_METHOD,
+ {}},
};
return kSchemas;
}
@@ -293,7 +301,7 @@
return false;
}
// For @Enforce annotations, validates the expression.
- if (schema_.type == AidlAnnotation::Type::ENFORCE) {
+ if (schema_.type == AidlAnnotation::Type::PERMISSION_ENFORCE) {
auto expr = EnforceExpression();
if (!expr.ok()) {
AIDL_ERROR(this) << "Unable to parse @Enforce annotation: " << expr.error();
@@ -443,7 +451,7 @@
// Parses the @Enforce annotation expression.
std::unique_ptr<perm::Expression> AidlAnnotatable::EnforceExpression() const {
- auto annot = GetAnnotation(annotations_, AidlAnnotation::Type::ENFORCE);
+ auto annot = GetAnnotation(annotations_, AidlAnnotation::Type::PERMISSION_ENFORCE);
if (annot) {
auto perm_expr = annot->EnforceExpression();
if (!perm_expr.ok()) {
@@ -455,6 +463,14 @@
return {};
}
+bool AidlAnnotatable::IsPermissionManual() const {
+ return GetAnnotation(annotations_, AidlAnnotation::Type::PERMISSION_MANUAL);
+}
+
+bool AidlAnnotatable::IsPermissionNone() const {
+ return GetAnnotation(annotations_, AidlAnnotation::Type::PERMISSION_NONE);
+}
+
bool AidlAnnotatable::IsStableApiParcelable(Options::Language lang) const {
return lang == Options::Language::JAVA &&
GetAnnotation(annotations_, AidlAnnotation::Type::JAVA_STABLE_PARCELABLE);
@@ -1564,6 +1580,10 @@
AIDL_ERROR(m) << " method " << m->Signature() << " is reserved for internal use.";
return false;
}
+
+ if (!CheckValidPermissionAnnotations(*m.get())) {
+ return false;
+ }
}
bool success = true;
@@ -1579,6 +1599,31 @@
return success;
}
+bool AidlInterface::CheckValidPermissionAnnotations(const AidlMethod& m) const {
+ if (IsPermissionNone() || IsPermissionManual()) {
+ if (m.GetType().IsPermissionNone() || m.GetType().IsPermissionManual() ||
+ m.GetType().EnforceExpression()) {
+ std::string interface_annotation = IsPermissionNone()
+ ? "requiring no permission"
+ : "manually implementing permission checks";
+ AIDL_ERROR(m) << "The interface " << GetName() << " is annotated as " << interface_annotation
+ << " but the method " << m.GetName() << " is also annotated.\n"
+ << "Consider distributing the annotation to each method.";
+ return false;
+ }
+ } else if (EnforceExpression()) {
+ if (m.GetType().IsPermissionNone() || m.GetType().IsPermissionManual()) {
+ AIDL_ERROR(m) << "The interface " << GetName()
+ << " enforces permissions using annotations"
+ " but the method "
+ << m.GetName() << " is also annotated.\n"
+ << "Consider distributing the annotation to each method.";
+ return false;
+ }
+ }
+ return true;
+}
+
std::string AidlInterface::GetDescriptor() const {
std::string annotatedDescriptor = AidlAnnotatable::GetDescriptor();
if (annotatedDescriptor != "") {
diff --git a/aidl_language.h b/aidl_language.h
index 7e20e03..44e6f51 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -236,7 +236,9 @@
DESCRIPTOR,
RUST_DERIVE,
SUPPRESS_WARNINGS,
- ENFORCE,
+ PERMISSION_ENFORCE,
+ PERMISSION_NONE,
+ PERMISSION_MANUAL
};
using TargetContext = uint16_t;
@@ -355,6 +357,8 @@
const AidlAnnotation* BackingType() const;
std::vector<std::string> SuppressWarnings() const;
std::unique_ptr<perm::Expression> EnforceExpression() const;
+ bool IsPermissionManual() const;
+ bool IsPermissionNone() const;
// ToString is for dumping AIDL.
// Returns string representation of annotations.
@@ -1157,6 +1161,7 @@
std::string GetPreprocessDeclarationName() const override { return "interface"; }
bool CheckValid(const AidlTypenames& typenames) const override;
+ bool CheckValidPermissionAnnotations(const AidlMethod& m) const;
std::string GetDescriptor() const;
void DispatchVisit(AidlVisitor& v) const override { v.Visit(*this); }
};
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 4e87ecb..0fda47c 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -3845,6 +3845,67 @@
EXPECT_TRUE(compile_aidl(options, io_delegate_));
}
+TEST_F(AidlTest, NoPermissionInterfaceEnforceMethod) {
+ io_delegate_.SetFileContents("a/IFoo.aidl", R"(package a;
+ @NoPermissionRequired
+ interface IFoo {
+ @Enforce(condition="permission = INTERNET")
+ void Protected();
+ })");
+
+ Options options = Options::From("aidl --lang=java -o out a/IFoo.aidl");
+ CaptureStderr();
+ EXPECT_FALSE(compile_aidl(options, io_delegate_));
+ EXPECT_THAT(GetCapturedStderr(),
+ HasSubstr("The interface IFoo is annotated as requiring no permission"));
+}
+
+TEST_F(AidlTest, ManualPermissionInterfaceEnforceMethod) {
+ io_delegate_.SetFileContents("a/IFoo.aidl", R"(package a;
+ @PermissionManuallyEnforced
+ interface IFoo {
+ @Enforce(condition="permission = INTERNET")
+ void Protected();
+ })");
+
+ Options options = Options::From("aidl --lang=java -o out a/IFoo.aidl");
+ CaptureStderr();
+ EXPECT_FALSE(compile_aidl(options, io_delegate_));
+ EXPECT_THAT(
+ GetCapturedStderr(),
+ HasSubstr("The interface IFoo is annotated as manually implementing permission checks"));
+}
+
+TEST_F(AidlTest, EnforceInterfaceNoPermissionsMethod) {
+ io_delegate_.SetFileContents("a/IFoo.aidl", R"(package a;
+ @Enforce(condition="permission = INTERNET")
+ interface IFoo {
+ @NoPermissionRequired
+ void Protected();
+ })");
+
+ Options options = Options::From("aidl --lang=java -o out a/IFoo.aidl");
+ CaptureStderr();
+ EXPECT_FALSE(compile_aidl(options, io_delegate_));
+ EXPECT_THAT(GetCapturedStderr(),
+ HasSubstr("The interface IFoo enforces permissions using annotations"));
+}
+
+TEST_F(AidlTest, EnforceInterfaceManualPermissionMethod) {
+ io_delegate_.SetFileContents("a/IFoo.aidl", R"(package a;
+ @Enforce(condition="permission = INTERNET")
+ interface IFoo {
+ @PermissionManuallyEnforced
+ void Protected();
+ })");
+
+ Options options = Options::From("aidl --lang=java -o out a/IFoo.aidl");
+ CaptureStderr();
+ EXPECT_FALSE(compile_aidl(options, io_delegate_));
+ EXPECT_THAT(GetCapturedStderr(),
+ HasSubstr("The interface IFoo enforces permissions using annotations"));
+}
+
class AidlOutputPathTest : public AidlTest {
protected:
void SetUp() override {