Merge "Add @EnforcePermission linter"

3 files changed, 374 insertions, 1 deletions

diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
index 900c2145975a..a6fd9bba6192 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
@@ -31,7 +31,9 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
             CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
             CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
             CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
-            CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED
+            CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
+            EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
+            EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
     )
 
     override val api: Int
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt
new file mode 100644
index 000000000000..8011b36c9a8f
--- /dev/null
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt
@@ -0,0 +1,169 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * 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 com.google.android.lint
+
+import com.android.tools.lint.detector.api.AnnotationInfo
+import com.android.tools.lint.detector.api.AnnotationOrigin
+import com.android.tools.lint.detector.api.AnnotationUsageInfo
+import com.android.tools.lint.detector.api.AnnotationUsageType
+import com.android.tools.lint.detector.api.ConstantEvaluator
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Implementation
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UElement
+
+/**
+ * Lint Detector that ensures that any method overriding a method annotated
+ * with @EnforcePermission is also annotated with the exact same annotation.
+ * The intent is to surface the effective permission checks to the service
+ * implementations.
+ */
+class EnforcePermissionDetector : Detector(), SourceCodeScanner {
+
+    val ENFORCE_PERMISSION = "android.annotation.EnforcePermission"
+
+    override fun applicableAnnotations(): List<String> {
+        return listOf(ENFORCE_PERMISSION)
+    }
+
+    private fun areAnnotationsEquivalent(
+        context: JavaContext,
+        anno1: PsiAnnotation,
+        anno2: PsiAnnotation
+    ): Boolean {
+        if (anno1.qualifiedName != anno2.qualifiedName) {
+            return false
+        }
+        val attr1 = anno1.parameterList.attributes
+        val attr2 = anno2.parameterList.attributes
+        if (attr1.size != attr2.size) {
+            return false
+        }
+        for (i in attr1.indices) {
+            if (attr1[i].name != attr2[i].name) {
+                return false
+            }
+            val v1 = ConstantEvaluator.evaluate(context, attr1[i].value)
+            val v2 = ConstantEvaluator.evaluate(context, attr2[i].value)
+            if (v1 != v2) {
+                return false
+            }
+        }
+        return true
+    }
+
+    override fun visitAnnotationUsage(
+        context: JavaContext,
+        element: UElement,
+        annotationInfo: AnnotationInfo,
+        usageInfo: AnnotationUsageInfo
+    ) {
+        if (usageInfo.type == AnnotationUsageType.EXTENDS) {
+            val newClass = element.sourcePsi?.parent?.parent as PsiClass
+            val extendedClass: PsiClass = usageInfo.referenced as PsiClass
+            val newAnnotation = newClass.getAnnotation(ENFORCE_PERMISSION)
+            val extendedAnnotation = extendedClass.getAnnotation(ENFORCE_PERMISSION)!!
+
+            val location = context.getLocation(element)
+            val newClassName = newClass.qualifiedName
+            val extendedClassName = extendedClass.qualifiedName
+            if (newAnnotation == null) {
+                val msg = "The class $newClassName extends the class $extendedClassName which " +
+                    "is annotated with @EnforcePermission. The same annotation must be used " +
+                    "on $newClassName."
+                context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
+            } else if (!areAnnotationsEquivalent(context, newAnnotation, extendedAnnotation)) {
+                val msg = "The class $newClassName is annotated with ${newAnnotation.text} " +
+                    "which differs from the parent class $extendedClassName: " +
+                    "${extendedAnnotation.text}. The same annotation must be used for " +
+                    "both classes."
+                context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
+            }
+        } else if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
+            annotationInfo.origin == AnnotationOrigin.METHOD) {
+            val overridingMethod = element.sourcePsi as PsiMethod
+            val overriddenMethod = usageInfo.referenced as PsiMethod
+            val overridingAnnotation = overridingMethod.getAnnotation(ENFORCE_PERMISSION)
+            val overriddenAnnotation = overriddenMethod.getAnnotation(ENFORCE_PERMISSION)!!
+
+            val location = context.getLocation(element)
+            val overridingClass = overridingMethod.parent as PsiClass
+            val overriddenClass = overriddenMethod.parent as PsiClass
+            val overridingName = "${overridingClass.name}.${overridingMethod.name}"
+            val overriddenName = "${overriddenClass.name}.${overriddenMethod.name}"
+            if (overridingAnnotation == null) {
+                val msg = "The method $overridingName overrides the method $overriddenName which " +
+                    "is annotated with @EnforcePermission. The same annotation must be used " +
+                    "on $overridingName"
+                context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
+            } else if (!areAnnotationsEquivalent(
+                        context, overridingAnnotation, overriddenAnnotation)) {
+                val msg = "The method $overridingName is annotated with " +
+                    "${overridingAnnotation.text} which differs from the overridden " +
+                    "method $overriddenName: ${overriddenAnnotation.text}. The same " +
+                    "annotation must be used for both methods."
+                context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
+            }
+        }
+    }
+
+    companion object {
+        val EXPLANATION = """
+            The @EnforcePermission annotation is used to indicate that the underlying binder code
+            has already verified the caller's permissions before calling the appropriate method. The
+            verification code is usually generated by the AIDL compiler, which also takes care of
+            annotating the generated Java code.
+
+            In order to surface that information to platform developers, the same annotation must be
+            used on the implementation class or methods.
+            """
+
+        val ISSUE_MISSING_ENFORCE_PERMISSION: Issue = Issue.create(
+            id = "MissingEnforcePermissionAnnotation",
+            briefDescription = "Missing @EnforcePermission annotation on Binder method",
+            explanation = EXPLANATION,
+            category = Category.SECURITY,
+            priority = 6,
+            severity = Severity.ERROR,
+            implementation = Implementation(
+                    EnforcePermissionDetector::class.java,
+                    Scope.JAVA_FILE_SCOPE
+            )
+        )
+
+        val ISSUE_MISMATCHING_ENFORCE_PERMISSION: Issue = Issue.create(
+            id = "MismatchingEnforcePermissionAnnotation",
+            briefDescription = "Incorrect @EnforcePermission annotation on Binder method",
+            explanation = EXPLANATION,
+            category = Category.SECURITY,
+            priority = 6,
+            severity = Severity.ERROR,
+            implementation = Implementation(
+                    EnforcePermissionDetector::class.java,
+                    Scope.JAVA_FILE_SCOPE
+            )
+        )
+    }
+}
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt
new file mode 100644
index 000000000000..f5f4ebee24e0
--- /dev/null
+++ b/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * 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 com.google.android.lint
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class EnforcePermissionDetectorTest : LintDetectorTest() {
+    override fun getDetector(): Detector = EnforcePermissionDetector()
+
+    override fun getIssues(): List<Issue> = listOf(
+            EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
+            EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
+    )
+
+    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+    fun testDoesNotDetectIssuesCorrectAnnotationOnClass() {
+        lint().files(java(
+            """
+            package test.pkg;
+            @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+            public class TestClass1 extends IFoo.Stub {
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expectClean()
+    }
+
+    fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
+        lint().files(java(
+            """
+            package test.pkg;
+            import android.annotation.EnforcePermission;
+            public class TestClass2 extends IFooMethod.Stub {
+                @Override
+                @EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expectClean()
+    }
+
+    fun testDetectIssuesMismatchingAnnotationOnClass() {
+        lint().files(java(
+            """
+            package test.pkg;
+            @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
+            public class TestClass3 extends IFoo.Stub {
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expect("""src/test/pkg/TestClass3.java:3: Error: The class test.pkg.TestClass3 is \
+annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
+which differs from the parent class IFoo.Stub: \
+@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The \
+same annotation must be used for both classes. [MismatchingEnforcePermissionAnnotation]
+public class TestClass3 extends IFoo.Stub {
+                                ~~~~~~~~~
+1 errors, 0 warnings""".addLineContinuation())
+    }
+
+    fun testDetectIssuesMismatchingAnnotationOnMethod() {
+        lint().files(java(
+            """
+            package test.pkg;
+            public class TestClass4 extends IFooMethod.Stub {
+                @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expect("""src/test/pkg/TestClass4.java:4: Error: The method TestClass4.testMethod is \
+annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
+which differs from the overridden method Stub.testMethod: \
+@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The same \
+annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+    public void testMethod() {}
+                ~~~~~~~~~~
+1 errors, 0 warnings""".addLineContinuation())
+    }
+
+    fun testDetectIssuesMissingAnnotationOnClass() {
+        lint().files(java(
+            """
+            package test.pkg;
+            public class TestClass5 extends IFoo.Stub {
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expect("""src/test/pkg/TestClass5.java:2: Error: The class test.pkg.TestClass5 extends \
+the class IFoo.Stub which is annotated with @EnforcePermission. The same annotation must be \
+used on test.pkg.TestClass5. [MissingEnforcePermissionAnnotation]
+public class TestClass5 extends IFoo.Stub {
+                                ~~~~~~~~~
+1 errors, 0 warnings""".addLineContinuation())
+    }
+
+    fun testDetectIssuesMissingAnnotationOnMethod() {
+        lint().files(java(
+            """
+            package test.pkg;
+            public class TestClass6 extends IFooMethod.Stub {
+                public void testMethod() {}
+            }
+            """).indented(),
+                *stubs
+        )
+        .run()
+        .expect("""src/test/pkg/TestClass6.java:3: Error: The method TestClass6.testMethod \
+overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same \
+annotation must be used on TestClass6.testMethod [MissingEnforcePermissionAnnotation]
+    public void testMethod() {}
+                ~~~~~~~~~~
+1 errors, 0 warnings""".addLineContinuation())
+    }
+
+    /* Stubs */
+
+    private val interfaceIFooStub: TestFile = java(
+        """
+        @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+        public interface IFoo {
+         @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+         public static abstract class Stub implements IFoo {
+           @Override
+           public void testMethod() {}
+         }
+         public void testMethod();
+        }
+        """
+    ).indented()
+
+    private val interfaceIFooMethodStub: TestFile = java(
+        """
+        public interface IFooMethod {
+         public static abstract class Stub implements IFooMethod {
+            @Override
+            @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+            public void testMethod() {}
+          }
+          @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+          public void testMethod();
+        }
+        """
+    ).indented()
+
+    private val manifestPermissionStub: TestFile = java(
+        """
+        package android.Manifest;
+        class permission {
+          public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
+          public static final String INTERNET = "android.permission.INTERNET";
+        }
+        """
+    ).indented()
+
+    private val enforcePermissionAnnotationStub: TestFile = java(
+        """
+        package android.annotation;
+        public @interface EnforcePermission {}
+        """
+    ).indented()
+
+    private val stubs = arrayOf(interfaceIFooStub, interfaceIFooMethodStub,
+            manifestPermissionStub, enforcePermissionAnnotationStub)
+
+    // Substitutes "backslash + new line" with an empty string to imitate line continuation
+    private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
+}