Add a PermissionMethodDetector lint to enforce proper usage of the

annotation

See: go/enforcepermission-migration-design
Bug: 247537842
Test: Tested manually, ultimately will be tested in presubmit (atest
AndroidFrameworkLintCheckerTest, see TODO b/240445172)

Change-Id: I9d2c0125a3d1c22288e191b891245ca216b49112

4 files changed, 209 insertions, 0 deletions

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e7e1be2e709b..a91b02b78fb9 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -6023,6 +6023,7 @@ public class ActivityManagerService extends IActivityManager.Stub
      * This can be called with or without the global lock held.
      */
     @Override
+    @PackageManager.PermissionResult
     @PermissionMethod
     public int checkPermission(@PermissionName String permission, int pid, int uid) {
         if (permission == null) {
@@ -6035,6 +6036,7 @@ public class ActivityManagerService extends IActivityManager.Stub
      * Binder IPC calls go through the public entry point.
      * This can be called with or without the global lock held.
      */
+    @PackageManager.PermissionResult
     @PermissionMethod
     int checkCallingPermission(@PermissionName String permission) {
         return checkPermission(permission,
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 8aa3e2583071..4d69d26e46db 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
@@ -42,6 +42,8 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
         SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
         PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
         RegisterReceiverFlagDetector.ISSUE_RECEIVER_EXPORTED_FLAG,
+        PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
+        PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
     )
 
     override val api: Int
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/Constants.kt b/tools/lint/checks/src/main/java/com/google/android/lint/Constants.kt
index 82eb8ed8f621..3d5d01c9b7a0 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/Constants.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/Constants.kt
@@ -34,3 +34,7 @@ val ENFORCE_PERMISSION_METHODS = listOf(
         Method(CLASS_ACTIVITY_MANAGER_SERVICE, "checkPermission"),
         Method(CLASS_ACTIVITY_MANAGER_INTERNAL, "enforceCallingPermission")
 )
+
+const val ANNOTATION_PERMISSION_METHOD = "android.content.pm.PermissionMethod"
+const val ANNOTATION_PERMISSION_NAME = "android.content.pm.PermissionName"
+const val ANNOTATION_PERMISSION_RESULT = "android.content.pm.PackageManager.PermissionResult"
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
new file mode 100644
index 000000000000..68a450d956a8
--- /dev/null
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
@@ -0,0 +1,201 @@
+/*
+ * 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.client.api.UElementHandler
+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.android.tools.lint.detector.api.getUMethod
+import com.intellij.psi.PsiType
+import org.jetbrains.uast.UAnnotation
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UIfExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UQualifiedReferenceExpression
+import org.jetbrains.uast.UReturnExpression
+import org.jetbrains.uast.getContainingUMethod
+
+/**
+ * Stops incorrect usage of {@link PermissionMethod}
+ * TODO: add tests once re-enabled (b/240445172, b/247542171)
+ */
+class PermissionMethodDetector : Detector(), SourceCodeScanner {
+
+    override fun getApplicableUastTypes(): List<Class<out UElement>> =
+        listOf(UAnnotation::class.java, UMethod::class.java)
+
+    override fun createUastHandler(context: JavaContext): UElementHandler =
+        PermissionMethodHandler(context)
+
+    private inner class PermissionMethodHandler(val context: JavaContext) : UElementHandler() {
+        override fun visitMethod(node: UMethod) {
+            if (hasPermissionMethodAnnotation(node)) return
+            if (onlyCallsPermissionMethod(node)) {
+                val location = context.getLocation(node.javaPsi.modifierList)
+                val fix = fix()
+                    .annotate(ANNOTATION_PERMISSION_METHOD)
+                    .range(location)
+                    .autoFix()
+                    .build()
+
+                context.report(
+                    ISSUE_CAN_BE_PERMISSION_METHOD,
+                    location,
+                    "Annotate method with @PermissionMethod",
+                    fix
+                )
+            }
+        }
+
+        override fun visitAnnotation(node: UAnnotation) {
+            if (node.qualifiedName != ANNOTATION_PERMISSION_METHOD) return
+            val method = node.getContainingUMethod() ?: return
+
+            if (!isPermissionMethodReturnType(method)) {
+                context.report(
+                    ISSUE_PERMISSION_METHOD_USAGE,
+                    context.getLocation(node),
+                    """
+                            Methods annotated with `@PermissionMethod` should return `void`, \
+                            `boolean`, or `@PackageManager.PermissionResult int`."
+                    """.trimIndent()
+                )
+            }
+
+            if (method.returnType == PsiType.INT &&
+                method.annotations.none { it.hasQualifiedName(ANNOTATION_PERMISSION_RESULT) }
+            ) {
+                context.report(
+                    ISSUE_PERMISSION_METHOD_USAGE,
+                    context.getLocation(node),
+                    """
+                            Methods annotated with `@PermissionMethod` that return `int` should \
+                            also be annotated with `@PackageManager.PermissionResult.`"
+                    """.trimIndent()
+                )
+            }
+        }
+    }
+
+    companion object {
+
+        private val EXPLANATION_PERMISSION_METHOD_USAGE = """
+            `@PermissionMethod` should annotate methods that ONLY perform permission lookups. \
+            Said methods should return `boolean`, `@PackageManager.PermissionResult int`, or return \
+            `void` and potentially throw `SecurityException`.
+        """.trimIndent()
+
+        @JvmField
+        val ISSUE_PERMISSION_METHOD_USAGE = Issue.create(
+            id = "PermissionMethodUsage",
+            briefDescription = "@PermissionMethod used incorrectly",
+            explanation = EXPLANATION_PERMISSION_METHOD_USAGE,
+            category = Category.CORRECTNESS,
+            priority = 5,
+            severity = Severity.ERROR,
+            implementation = Implementation(
+                PermissionMethodDetector::class.java,
+                Scope.JAVA_FILE_SCOPE
+            ),
+            enabledByDefault = true
+        )
+
+        private val EXPLANATION_CAN_BE_PERMISSION_METHOD = """
+            Methods that only call other methods annotated with @PermissionMethod (and do NOTHING else) can themselves \
+            be annotated with @PermissionMethod.  For example:
+            ```
+            void wrapperHelper() {
+              // Context.enforceCallingPermission is annotated with @PermissionMethod
+              context.enforceCallingPermission(SOME_PERMISSION)
+            }
+            ```
+        """.trimIndent()
+
+        @JvmField
+        val ISSUE_CAN_BE_PERMISSION_METHOD = Issue.create(
+            id = "CanBePermissionMethod",
+            briefDescription = "Method can be annotated with @PermissionMethod",
+            explanation = EXPLANATION_CAN_BE_PERMISSION_METHOD,
+            category = Category.SECURITY,
+            priority = 5,
+            severity = Severity.WARNING,
+            implementation = Implementation(
+                PermissionMethodDetector::class.java,
+                Scope.JAVA_FILE_SCOPE
+            ),
+            enabledByDefault = false
+        )
+
+        private fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations
+            .any {
+                it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD)
+            }
+
+        private fun isPermissionMethodReturnType(method: UMethod): Boolean =
+            listOf(PsiType.VOID, PsiType.INT, PsiType.BOOLEAN).contains(method.returnType)
+
+        /**
+         * Identifies methods that...
+         * DO call other methods annotated with @PermissionMethod
+         * DO NOT do anything else
+         */
+        private fun onlyCallsPermissionMethod(method: UMethod): Boolean {
+            val body = method.uastBody as? UBlockExpression ?: return false
+            if (body.expressions.isEmpty()) return false
+            for (expression in body.expressions) {
+                when (expression) {
+                    is UQualifiedReferenceExpression -> {
+                        if (!isPermissionMethodCall(expression.selector)) return false
+                    }
+                    is UReturnExpression -> {
+                        if (!isPermissionMethodCall(expression.returnExpression)) return false
+                    }
+                    is UCallExpression -> {
+                        if (!isPermissionMethodCall(expression)) return false
+                    }
+                    is UIfExpression -> {
+                        if (expression.thenExpression !is UReturnExpression) return false
+                        if (!isPermissionMethodCall(expression.condition)) return false
+                    }
+                    else -> return false
+                }
+            }
+            return true
+        }
+
+        private fun isPermissionMethodCall(expression: UExpression?): Boolean {
+            return when (expression) {
+                is UQualifiedReferenceExpression ->
+                    return isPermissionMethodCall(expression.selector)
+                is UCallExpression -> {
+                    val calledMethod = expression.resolve()?.getUMethod() ?: return false
+                    return hasPermissionMethodAnnotation(calledMethod)
+                }
+                else -> false
+            }
+        }
+    }
+}