Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 627faea6 authored by Thiébaud Weksteen's avatar Thiébaud Weksteen
Browse files

Add PermissionAnnotationDetector

This detector ensures that all AIDL-generated interfaces are annotated
with one permission annotation (@EnforcePermission,
@NoPermissionRequired, or @PermissionManuallyEnforced). It is part of
the framework linters and disabled by default.

It will be enabled explicitly in a subsequent change (for
services/Android.bp). The baseline of the services will also be updated
to capture the current status of non-annotated interfaces.

Bug: 220214993
Test: atest --host AndroidFrameworkLintCheckerTest
Change-Id: I0fbd41b63465f119d90505153fa3a98c0fcdc798
parent eb133bf3
Loading
Loading
Loading
Loading
+2 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import com.android.tools.lint.client.api.IssueRegistry
import com.android.tools.lint.client.api.Vendor
import com.android.tools.lint.detector.api.CURRENT_API
import com.google.android.lint.parcel.SaferParcelChecker
import com.google.android.lint.aidl.PermissionAnnotationDetector
import com.google.auto.service.AutoService

@AutoService(IssueRegistry::class)
@@ -37,6 +38,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
        SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
        // TODO: Currently crashes due to OOM issue
        // PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
        PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
        PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
        PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
    )
+87 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2023 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.aidl

import com.android.tools.lint.detector.api.Category
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 org.jetbrains.uast.UBlockExpression
import org.jetbrains.uast.UMethod

/**
 * Ensures all AIDL-generated methods are annotated.
 *
 * This detector is run on system_server to validate that any method that may
 * be exposed via an AIDL interface is permission-annotated. That is, it must
 * have one of the following annotation:
 *   - @EnforcePermission
 *   - @RequiresNoPermission
 *   - @PermissionManuallyEnforced
 */
class PermissionAnnotationDetector : AidlImplementationDetector() {

    override fun visitAidlMethod(
      context: JavaContext,
      node: UMethod,
      interfaceName: String,
      body: UBlockExpression
    ) {
        if (context.evaluator.isAbstract(node)) return

        if (AIDL_PERMISSION_ANNOTATIONS.any { node.hasAnnotation(it) }) return

        context.report(
            ISSUE_MISSING_PERMISSION_ANNOTATION,
            node,
            context.getLocation(node),
            "The method ${node.name} is not permission-annotated."
        )
    }

    companion object {

        private val EXPLANATION_MISSING_PERMISSION_ANNOTATION = """
          Interfaces that are exposed by system_server are required to have an annotation which
          denotes the type of permission enforced. There are 3 possible options:
            - @EnforcePermission
            - @RequiresNoPermission
            - @PermissionManuallyEnforced
          See the documentation of each annotation for further details.

          The annotation on the Java implementation must be the same that the AIDL interface
          definition. This is verified by a lint in the build system.
          """.trimIndent()

        @JvmField
        val ISSUE_MISSING_PERMISSION_ANNOTATION = Issue.create(
            id = "MissingPermissionAnnotation",
            briefDescription = "No permission annotation on exposed AIDL interface.",
            explanation = EXPLANATION_MISSING_PERMISSION_ANNOTATION,
            category = Category.CORRECTNESS,
            priority = 5,
            severity = Severity.ERROR,
            implementation = Implementation(
                PermissionAnnotationDetector::class.java,
                Scope.JAVA_FILE_SCOPE
            ),
            enabledByDefault = false
        )
    }
}
+134 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2023 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.aidl

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 PermissionAnnotationDetectorTest : LintDetectorTest() {
    override fun getDetector(): Detector = PermissionAnnotationDetector()

    override fun getIssues(): List<Issue> = listOf(
        PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
    )

    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

    /** No issue scenario */

    fun testDoesNotDetectIssuesInCorrectScenario() {
        lint().files(
            java(
            """
            public class Foo extends IFoo.Stub {
                @Override
                @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
                public void testMethod() { }
            }
            """
            ).indented(),
            *stubs
        )
            .run()
            .expectClean()
    }

    fun testMissingAnnotation() {
        lint().files(
            java(
            """
            public class Bar extends IBar.Stub {
                public void testMethod() { }
            }
            """
            ).indented(),
            *stubs
        )
            .run()
            .expect(
                """
                src/Bar.java:2: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
                    public void testMethod() { }
                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                1 errors, 0 warnings
                """
            )
    }

    fun testNoIssueWhenExtendingWithAnotherSubclass() {
        lint().files(
            java(
            """
            public class Foo extends IFoo.Stub {
                @Override
                @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
                public void testMethod() { }
                // not an AIDL method, just another method
                public void someRandomMethod() { }
            }
            """).indented(),
            java(
            """
            public class Baz extends Bar {
              @Override
              public void someRandomMethod() { }
            }
            """).indented(),
            *stubs
        )
            .run()
            .expectClean()
    }

    /* Stubs */

    // A service with permission annotation on the method.
    private val interfaceIFoo: TestFile = java(
        """
        public interface IFoo extends android.os.IInterface {
         public static abstract class Stub extends android.os.Binder implements IFoo {
          }
          @Override
          @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
          public void testMethod();
          @Override
          @android.annotation.RequiresNoPermission
          public void testMethodNoPermission();
          @Override
          @android.annotation.PermissionManuallyEnforced
          public void testMethodManual();
        }
        """
    ).indented()

    // A service with no permission annotation.
    private val interfaceIBar: TestFile = java(
        """
        public interface IBar extends android.os.IInterface {
         public static abstract class Stub extends android.os.Binder implements IBar {
          }
          public void testMethod();
        }
        """
    ).indented()

    private val stubs = arrayOf(interfaceIFoo, interfaceIBar)
}