More Bluetooth API annotation updates.

This change adds a "BluetoothPermissionChecker" that ensures that
all Bluetooth permission annotations are consistent.  In addition, it
verifies that all Bluetooth public APIs have been audited to be
permission protected where relevant.

We've currently standardized on saying that APIs that return device
or Bluetooth state information (without sharing details about any
particular remote Bluetooth device) do not need to be permission
protected.

This change is only annotations and has no behavior changes.

Bug: 183626724
Test: ./build/soong/soong_ui.bash --make-mode Bluetooth RUN_ERROR_PRONE=true
Change-Id: Ie80b15b058359bf1e9a6ee881b89cb3e5b584ca1

4 files changed, 395 insertions, 5 deletions

diff --git a/errorprone/java/android/annotation/RequiresNoPermission.java b/errorprone/java/android/annotation/RequiresNoPermission.java
new file mode 100644
index 000000000000..6ff4d6e34957
--- /dev/null
+++ b/errorprone/java/android/annotation/RequiresNoPermission.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2021 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 android.annotation;
+
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
+import static java.lang.annotation.ElementType.CONSTRUCTOR;
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.CLASS;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Denotes that the annotated element requires no permissions.
+ *
+ * @hide
+ */
+@Retention(CLASS)
+@Target({ANNOTATION_TYPE,METHOD,CONSTRUCTOR,FIELD,PARAMETER})
+public @interface RequiresNoPermission {
+}
diff --git a/errorprone/java/android/annotation/RequiresPermission.java b/errorprone/java/android/annotation/RequiresPermission.java
new file mode 100644
index 000000000000..303ab41bec8e
--- /dev/null
+++ b/errorprone/java/android/annotation/RequiresPermission.java
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2015 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 android.annotation;
+
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
+import static java.lang.annotation.ElementType.CONSTRUCTOR;
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.CLASS;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Denotes that the annotated element requires (or may require) one or more permissions.
+ * <p/>
+ * Example of requiring a single permission:
+ * <pre>{@code
+ *   {@literal @}RequiresPermission(Manifest.permission.SET_WALLPAPER)
+ *   public abstract void setWallpaper(Bitmap bitmap) throws IOException;
+ *
+ *   {@literal @}RequiresPermission(ACCESS_COARSE_LOCATION)
+ *   public abstract Location getLastKnownLocation(String provider);
+ * }</pre>
+ * Example of requiring at least one permission from a set:
+ * <pre>{@code
+ *   {@literal @}RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
+ *   public abstract Location getLastKnownLocation(String provider);
+ * }</pre>
+ * Example of requiring multiple permissions:
+ * <pre>{@code
+ *   {@literal @}RequiresPermission(allOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
+ *   public abstract Location getLastKnownLocation(String provider);
+ * }</pre>
+ * Example of requiring separate read and write permissions for a content provider:
+ * <pre>{@code
+ *   {@literal @}RequiresPermission.Read(@RequiresPermission(READ_HISTORY_BOOKMARKS))
+ *   {@literal @}RequiresPermission.Write(@RequiresPermission(WRITE_HISTORY_BOOKMARKS))
+ *   public static final Uri BOOKMARKS_URI = Uri.parse("content://browser/bookmarks");
+ * }</pre>
+ * <p>
+ * When specified on a parameter, the annotation indicates that the method requires
+ * a permission which depends on the value of the parameter. For example, consider
+ * {@link android.app.Activity#startActivity(android.content.Intent)
+ * Activity#startActivity(Intent)}:
+ * <pre>{@code
+ *   public void startActivity(@RequiresPermission Intent intent) { ... }
+ * }</pre>
+ * Notice how there are no actual permission names listed in the annotation. The actual
+ * permissions required will depend on the particular intent passed in. For example,
+ * the code may look like this:
+ * <pre>{@code
+ *   Intent intent = new Intent(Intent.ACTION_CALL);
+ *   startActivity(intent);
+ * }</pre>
+ * and the actual permission requirement for this particular intent is described on
+ * the Intent name itself:
+ * <pre>{@code
+ *   {@literal @}RequiresPermission(Manifest.permission.CALL_PHONE)
+ *   public static final String ACTION_CALL = "android.intent.action.CALL";
+ * }</pre>
+ *
+ * @hide
+ */
+@Retention(CLASS)
+@Target({ANNOTATION_TYPE,METHOD,CONSTRUCTOR,FIELD,PARAMETER})
+public @interface RequiresPermission {
+    /**
+     * The name of the permission that is required, if precisely one permission
+     * is required. If more than one permission is required, specify either
+     * {@link #allOf()} or {@link #anyOf()} instead.
+     * <p>
+     * If specified, {@link #anyOf()} and {@link #allOf()} must both be null.
+     */
+    String value() default "";
+
+    /**
+     * Specifies a list of permission names that are all required.
+     * <p>
+     * If specified, {@link #anyOf()} and {@link #value()} must both be null.
+     */
+    String[] allOf() default {};
+
+    /**
+     * Specifies a list of permission names where at least one is required
+     * <p>
+     * If specified, {@link #allOf()} and {@link #value()} must both be null.
+     */
+    String[] anyOf() default {};
+
+    /**
+     * If true, the permission may not be required in all cases (e.g. it may only be
+     * enforced on certain platforms, or for certain call parameters, etc.
+     */
+    boolean conditional() default false;
+
+    /**
+     * Specifies that the given permission is required for read operations.
+     * <p>
+     * When specified on a parameter, the annotation indicates that the method requires
+     * a permission which depends on the value of the parameter (and typically
+     * the corresponding field passed in will be one of a set of constants which have
+     * been annotated with a <code>@RequiresPermission</code> annotation.)
+     */
+    @Target({FIELD, METHOD, PARAMETER})
+    @interface Read {
+        RequiresPermission value() default @RequiresPermission;
+    }
+
+    /**
+     * Specifies that the given permission is required for write operations.
+     * <p>
+     * When specified on a parameter, the annotation indicates that the method requires
+     * a permission which depends on the value of the parameter (and typically
+     * the corresponding field passed in will be one of a set of constants which have
+     * been annotated with a <code>@RequiresPermission</code> annotation.)
+     */
+    @Target({FIELD, METHOD, PARAMETER})
+    @interface Write {
+        RequiresPermission value() default @RequiresPermission;
+    }
+}
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/BluetoothPermissionChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/BluetoothPermissionChecker.java
new file mode 100644
index 000000000000..9d1cf87a5f9d
--- /dev/null
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/BluetoothPermissionChecker.java
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) 2021 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.errorprone.bugpatterns.android;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.bugpatterns.android.RequiresPermissionChecker.simpleNameMatches;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.enclosingClass;
+import static com.google.errorprone.matchers.Matchers.isStatic;
+import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
+import static com.google.errorprone.matchers.Matchers.methodHasVisibility;
+import static com.google.errorprone.matchers.Matchers.methodIsConstructor;
+import static com.google.errorprone.matchers.Matchers.methodIsNamed;
+import static com.google.errorprone.matchers.Matchers.not;
+import static com.google.errorprone.matchers.Matchers.packageStartsWith;
+
+import android.annotation.RequiresNoPermission;
+import android.annotation.RequiresPermission;
+import android.annotation.SuppressLint;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.MethodVisibility.Visibility;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Pattern;
+
+/**
+ * Verifies that all Bluetooth APIs have consistent permissions.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "AndroidFrameworkBluetoothPermission",
+    summary = "Verifies that all Bluetooth APIs have consistent permissions",
+    severity = WARNING)
+public final class BluetoothPermissionChecker extends BugChecker implements MethodTreeMatcher {
+    private static final Matcher<MethodTree> BLUETOOTH_API = allOf(
+            packageStartsWith("android.bluetooth"),
+            methodHasVisibility(Visibility.PUBLIC),
+            not(isStatic()),
+            not(methodIsConstructor()),
+            not(enclosingClass(isInsideParcelable())),
+            not(enclosingClass(simpleNameMatches(Pattern.compile(".+Callback$")))),
+            not(enclosingClass(isSubtypeOf("android.bluetooth.BluetoothProfileConnector"))),
+            not(enclosingClass(isSubtypeOf("android.app.PropertyInvalidatedCache"))));
+
+    private static final Matcher<ClassTree> PARCELABLE_CLASS =
+            isSubtypeOf("android.os.Parcelable");
+    private static final Matcher<MethodTree> BINDER_METHOD = enclosingClass(
+            isSubtypeOf("android.os.IInterface"));
+
+    private static final Matcher<MethodTree> BINDER_INTERNALS = allOf(
+            enclosingClass(isSubtypeOf("android.os.IInterface")),
+            anyOf(
+                    methodIsNamed("onTransact"),
+                    methodIsNamed("dump"),
+                    enclosingClass(simpleNameMatches(Pattern.compile("^(Stub|Default|Proxy)$")))));
+
+    private static final Matcher<MethodTree> GENERIC_INTERNALS = anyOf(
+            methodIsNamed("close"),
+            methodIsNamed("finalize"),
+            methodIsNamed("equals"),
+            methodIsNamed("hashCode"),
+            methodIsNamed("toString"));
+
+    private static final String PERMISSION_ADVERTISE = "android.permission.BLUETOOTH_ADVERTISE";
+    private static final String PERMISSION_CONNECT = "android.permission.BLUETOOTH_CONNECT";
+    private static final String PERMISSION_SCAN = "android.permission.BLUETOOTH_SCAN";
+
+    private static final String ANNOTATION_ADVERTISE =
+            "android.bluetooth.annotations.RequiresBluetoothAdvertisePermission";
+    private static final String ANNOTATION_CONNECT =
+            "android.bluetooth.annotations.RequiresBluetoothConnectPermission";
+    private static final String ANNOTATION_SCAN =
+            "android.bluetooth.annotations.RequiresBluetoothScanPermission";
+
+    @Override
+    public Description matchMethod(MethodTree tree, VisitorState state) {
+        // Ignore methods outside Bluetooth area
+        if (!BLUETOOTH_API.matches(tree, state)) return Description.NO_MATCH;
+
+        // Ignore certain types of generated or internal code
+        if (BINDER_INTERNALS.matches(tree, state)) return Description.NO_MATCH;
+        if (GENERIC_INTERNALS.matches(tree, state)) return Description.NO_MATCH;
+
+        // Skip abstract methods, except for binder interfaces
+        if (tree.getBody() == null && !BINDER_METHOD.matches(tree, state)) {
+            return Description.NO_MATCH;
+        }
+
+        // Ignore callbacks which don't need permission enforcement
+        final MethodSymbol symbol = ASTHelpers.getSymbol(tree);
+        if (isCallbackOrWrapper(symbol)) return Description.NO_MATCH;
+
+        // Ignore when suppressed
+        if (isSuppressed(symbol)) return Description.NO_MATCH;
+
+        final RequiresPermission requiresPerm = ASTHelpers.getAnnotation(tree,
+                RequiresPermission.class);
+        final RequiresNoPermission requiresNoPerm = ASTHelpers.getAnnotation(tree,
+                RequiresNoPermission.class);
+
+        final boolean requiresValid = requiresPerm != null
+                && (requiresPerm.value() != null || requiresPerm.allOf() != null);
+        final boolean requiresNoValid = requiresNoPerm != null;
+        if (!requiresValid && !requiresNoValid) {
+            return buildDescription(tree)
+                    .setMessage("Method " + symbol.name.toString()
+                            + "() must be protected by at least one permission")
+                    .build();
+        }
+
+        // No additional checks needed for Binder generated code
+        if (BINDER_METHOD.matches(tree, state)) return Description.NO_MATCH;
+
+        if (ASTHelpers.hasAnnotation(tree, ANNOTATION_ADVERTISE,
+                state) != isPermissionReferenced(requiresPerm, PERMISSION_ADVERTISE)) {
+            return buildDescription(tree)
+                    .setMessage("Method " + symbol.name.toString()
+                            + "() has inconsistent annotations for " + PERMISSION_ADVERTISE)
+                    .build();
+        }
+        if (ASTHelpers.hasAnnotation(tree, ANNOTATION_CONNECT,
+                state) != isPermissionReferenced(requiresPerm, PERMISSION_CONNECT)) {
+            return buildDescription(tree)
+                    .setMessage("Method " + symbol.name.toString()
+                            + "() has inconsistent annotations for " + PERMISSION_CONNECT)
+                    .build();
+        }
+        if (ASTHelpers.hasAnnotation(tree, ANNOTATION_SCAN,
+                state) != isPermissionReferenced(requiresPerm, PERMISSION_SCAN)) {
+            return buildDescription(tree)
+                    .setMessage("Method " + symbol.name.toString()
+                            + "() has inconsistent annotations for " + PERMISSION_SCAN)
+                    .build();
+        }
+
+        return Description.NO_MATCH;
+    }
+
+    private static boolean isPermissionReferenced(RequiresPermission anno, String perm) {
+        if (anno == null) return false;
+        if (perm.equals(anno.value())) return true;
+        return anno.allOf() != null && Arrays.asList(anno.allOf()).contains(perm);
+    }
+
+    private static boolean isCallbackOrWrapper(Symbol symbol) {
+        if (symbol == null) return false;
+        final String name = symbol.name.toString();
+        return isCallbackOrWrapper(ASTHelpers.enclosingClass(symbol))
+                || name.endsWith("Callback")
+                || name.endsWith("Wrapper");
+    }
+
+    public boolean isSuppressed(Symbol symbol) {
+        if (symbol == null) return false;
+        return isSuppressed(ASTHelpers.enclosingClass(symbol))
+                || isSuppressed(ASTHelpers.getAnnotation(symbol, SuppressWarnings.class))
+                || isSuppressed(ASTHelpers.getAnnotation(symbol, SuppressLint.class));
+    }
+
+    private boolean isSuppressed(SuppressWarnings anno) {
+        return (anno != null) && !Collections.disjoint(Arrays.asList(anno.value()), allNames());
+    }
+
+    private boolean isSuppressed(SuppressLint anno) {
+        return (anno != null) && !Collections.disjoint(Arrays.asList(anno.value()), allNames());
+    }
+
+    private static Matcher<ClassTree> isInsideParcelable() {
+        return new Matcher<ClassTree>() {
+            @Override
+            public boolean matches(ClassTree tree, VisitorState state) {
+                final TreePath path = state.getPath();
+                for (Tree node : path) {
+                    if (node instanceof ClassTree
+                            && PARCELABLE_CLASS.matches((ClassTree) node, state)) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        };
+    }
+}
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
index 3b5a58c46f71..f54782d4eb77 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
@@ -143,8 +143,11 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho
                     final ParsedRequiresPermission nodePerm = parseRequiresPermissionRecursively(
                             node, state);
                     if (!expectedPerm.containsAll(nodePerm)) {
-                        return buildDescription(node).setMessage("Annotated " + expectedPerm
-                                + " but too narrow; invokes method requiring " + nodePerm).build();
+                        return buildDescription(node)
+                                .setMessage("Method " + method.name.toString() + "() annotated "
+                                        + expectedPerm
+                                        + " but too narrow; invokes method requiring " + nodePerm)
+                                .build();
                     } else {
                         actualPerm.addAll(nodePerm);
                     }
@@ -162,8 +165,10 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho
         // Second, determine if we actually used all permissions that we claim
         // to require; yell if we're too broad
         if (!actualPerm.containsAll(expectedPerm)) {
-            return buildDescription(tree).setMessage("Annotated " + expectedPerm
-                    + " but too wide; only invokes methods requiring " + actualPerm).build();
+            return buildDescription(tree)
+                    .setMessage("Method  " + method.name.toString() + "() annotated " + expectedPerm
+                            + " but too wide; only invokes methods requiring " + actualPerm)
+                    .build();
         }
 
         return Description.NO_MATCH;
@@ -316,7 +321,7 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho
         return (anno != null) && !Collections.disjoint(Arrays.asList(anno.value()), allNames());
     }
 
-    private static Matcher<ClassTree> simpleNameMatches(Pattern pattern) {
+    static Matcher<ClassTree> simpleNameMatches(Pattern pattern) {
         return new Matcher<ClassTree>() {
             @Override
             public boolean matches(ClassTree tree, VisitorState state) {