From 912e80d3450943ac2bbca03f33c31c042799a0a1 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 24 Feb 2017 11:00:55 -0700 Subject: [PATCH] Require valid authorities for Uri notifications. Content changed notifications are really only valid for content:// Uris, which are really only valid when we have a valid ContentProvider backing them. This has been implicit for a long time, but we actually need to start enforcing it based on target API. We also now tell developers about why their notification requests are being denied, instead of silently logging. Test: builds, boots, common operations work Bug: 34049049 Change-Id: Ie8ab8d8674cff13e3e9269ffddc4ad998cb848c4 --- .../java/android/content/ContentResolver.java | 99 ++++++++++++------- .../java/android/content/IContentService.aidl | 4 +- .../server/am/ActivityManagerService.java | 5 +- .../server/content/ContentService.java | 37 +++++-- 4 files changed, 95 insertions(+), 50 deletions(-) diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index 4480b41ed73a2..687d590cc8209 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -500,6 +500,7 @@ public abstract class ContentResolver { public ContentResolver(Context context) { mContext = context != null ? context : ActivityThread.currentApplication(); mPackageName = mContext.getOpPackageName(); + mTargetSdkVersion = mContext.getApplicationInfo().targetSdkVersion; } /** @hide */ @@ -1868,13 +1869,18 @@ public abstract class ContentResolver { /** * Register an observer class that gets callbacks when data identified by a * given content URI changes. + *

+ * Starting in {@link android.os.Build.VERSION_CODES#O}, all content + * notifications must be backed by a valid {@link ContentProvider}. * - * @param uri The URI to watch for changes. This can be a specific row URI, or a base URI - * for a whole class of content. - * @param notifyForDescendants When false, the observer will be notified whenever a - * change occurs to the exact URI specified by uri or to one of the - * URI's ancestors in the path hierarchy. When true, the observer will also be notified - * whenever a change occurs to the URI's descendants in the path hierarchy. + * @param uri The URI to watch for changes. This can be a specific row URI, + * or a base URI for a whole class of content. + * @param notifyForDescendants When false, the observer will be notified + * whenever a change occurs to the exact URI specified by + * uri or to one of the URI's ancestors in the path + * hierarchy. When true, the observer will also be notified + * whenever a change occurs to the URI's descendants in the path + * hierarchy. * @param observer The object that receives callbacks when changes occur. * @see #unregisterContentObserver */ @@ -1894,7 +1900,7 @@ public abstract class ContentResolver { ContentObserver observer, @UserIdInt int userHandle) { try { getContentService().registerContentObserver(uri, notifyForDescendents, - observer.getContentObserver(), userHandle); + observer.getContentObserver(), userHandle, mTargetSdkVersion); } catch (RemoteException e) { } } @@ -1918,16 +1924,22 @@ public abstract class ContentResolver { } /** - * Notify registered observers that a row was updated and attempt to sync changes - * to the network. - * To register, call {@link #registerContentObserver(android.net.Uri , boolean, android.database.ContentObserver) registerContentObserver()}. - * By default, CursorAdapter objects will get this notification. + * Notify registered observers that a row was updated and attempt to sync + * changes to the network. + *

+ * To observe events sent through this call, use + * {@link #registerContentObserver(Uri, boolean, ContentObserver)}. + *

+ * Starting in {@link android.os.Build.VERSION_CODES#O}, all content + * notifications must be backed by a valid {@link ContentProvider}. * * @param uri The uri of the content that was changed. - * @param observer The observer that originated the change, may be null. - * The observer that originated the change will only receive the notification if it - * has requested to receive self-change notifications by implementing - * {@link ContentObserver#deliverSelfNotifications()} to return true. + * @param observer The observer that originated the change, may be + * null. The observer that originated the change + * will only receive the notification if it has requested to + * receive self-change notifications by implementing + * {@link ContentObserver#deliverSelfNotifications()} to return + * true. */ public void notifyChange(@NonNull Uri uri, @Nullable ContentObserver observer) { notifyChange(uri, observer, true /* sync to network */); @@ -1935,17 +1947,25 @@ public abstract class ContentResolver { /** * Notify registered observers that a row was updated. - * To register, call {@link #registerContentObserver(android.net.Uri , boolean, android.database.ContentObserver) registerContentObserver()}. - * By default, CursorAdapter objects will get this notification. - * If syncToNetwork is true, this will attempt to schedule a local sync using the sync - * adapter that's registered for the authority of the provided uri. No account will be - * passed to the sync adapter, so all matching accounts will be synchronized. + *

+ * To observe events sent through this call, use + * {@link #registerContentObserver(Uri, boolean, ContentObserver)}. + *

+ * If syncToNetwork is true, this will attempt to schedule a local sync + * using the sync adapter that's registered for the authority of the + * provided uri. No account will be passed to the sync adapter, so all + * matching accounts will be synchronized. + *

+ * Starting in {@link android.os.Build.VERSION_CODES#O}, all content + * notifications must be backed by a valid {@link ContentProvider}. * * @param uri The uri of the content that was changed. - * @param observer The observer that originated the change, may be null. - * The observer that originated the change will only receive the notification if it - * has requested to receive self-change notifications by implementing - * {@link ContentObserver#deliverSelfNotifications()} to return true. + * @param observer The observer that originated the change, may be + * null. The observer that originated the change + * will only receive the notification if it has requested to + * receive self-change notifications by implementing + * {@link ContentObserver#deliverSelfNotifications()} to return + * true. * @param syncToNetwork If true, same as {@link #NOTIFY_SYNC_TO_NETWORK}. * @see #requestSync(android.accounts.Account, String, android.os.Bundle) */ @@ -1961,17 +1981,25 @@ public abstract class ContentResolver { /** * Notify registered observers that a row was updated. - * To register, call {@link #registerContentObserver(android.net.Uri, boolean, android.database.ContentObserver) registerContentObserver()}. - * By default, CursorAdapter objects will get this notification. - * If syncToNetwork is true, this will attempt to schedule a local sync using the sync - * adapter that's registered for the authority of the provided uri. No account will be - * passed to the sync adapter, so all matching accounts will be synchronized. + *

+ * To observe events sent through this call, use + * {@link #registerContentObserver(Uri, boolean, ContentObserver)}. + *

+ * If syncToNetwork is true, this will attempt to schedule a local sync + * using the sync adapter that's registered for the authority of the + * provided uri. No account will be passed to the sync adapter, so all + * matching accounts will be synchronized. + *

+ * Starting in {@link android.os.Build.VERSION_CODES#O}, all content + * notifications must be backed by a valid {@link ContentProvider}. * * @param uri The uri of the content that was changed. - * @param observer The observer that originated the change, may be null. - * The observer that originated the change will only receive the notification if it - * has requested to receive self-change notifications by implementing - * {@link ContentObserver#deliverSelfNotifications()} to return true. + * @param observer The observer that originated the change, may be + * null. The observer that originated the change + * will only receive the notification if it has requested to + * receive self-change notifications by implementing + * {@link ContentObserver#deliverSelfNotifications()} to return + * true. * @param flags Additional flags: {@link #NOTIFY_SYNC_TO_NETWORK}. * @see #requestSync(android.accounts.Account, String, android.os.Bundle) */ @@ -1997,7 +2025,7 @@ public abstract class ContentResolver { uri, observer == null ? null : observer.getContentObserver(), observer != null && observer.deliverSelfNotifications(), syncToNetwork ? NOTIFY_SYNC_TO_NETWORK : 0, - userHandle); + userHandle, mTargetSdkVersion); } catch (RemoteException e) { } } @@ -2013,7 +2041,7 @@ public abstract class ContentResolver { getContentService().notifyChange( uri, observer == null ? null : observer.getContentObserver(), observer != null && observer.deliverSelfNotifications(), flags, - userHandle); + userHandle, mTargetSdkVersion); } catch (RemoteException e) { } } @@ -2932,6 +2960,7 @@ public abstract class ContentResolver { private final Context mContext; final String mPackageName; + final int mTargetSdkVersion; private static final String TAG = "ContentResolver"; diff --git a/core/java/android/content/IContentService.aidl b/core/java/android/content/IContentService.aidl index 3446e0315a1d0..c5001166d77a7 100644 --- a/core/java/android/content/IContentService.aidl +++ b/core/java/android/content/IContentService.aidl @@ -42,7 +42,7 @@ interface IContentService { * USER_CURRENT are properly handled. */ void registerContentObserver(in Uri uri, boolean notifyForDescendants, - IContentObserver observer, int userHandle); + IContentObserver observer, int userHandle, int targetSdkVersion); /** * Notify observers of a particular user's view of the provider. @@ -53,7 +53,7 @@ interface IContentService { */ void notifyChange(in Uri uri, IContentObserver observer, boolean observerWantsSelfNotifications, int flags, - int userHandle); + int userHandle, int targetSdkVersion); void requestSync(in Account account, String authority, in Bundle extras); /** diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index aaed0e9c99beb..ec9b5fa96d44d 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -10736,10 +10736,7 @@ public class ActivityManagerService extends IActivityManager.Stub } catch (RemoteException ignored) { } if (cpi == null) { - // TODO: make this an outright failure in a future platform release; - // until then anonymous content notifications are unprotected - //return "Failed to find provider " + authority + " for user " + userId; - return null; + return "Failed to find provider " + authority + " for user " + userId; } ProcessRecord r = null; diff --git a/services/core/java/com/android/server/content/ContentService.java b/services/core/java/com/android/server/content/ContentService.java index 886c97f4ce179..4b34eba56e9b6 100644 --- a/services/core/java/com/android/server/content/ContentService.java +++ b/services/core/java/com/android/server/content/ContentService.java @@ -43,6 +43,7 @@ import android.database.IContentObserver; import android.database.sqlite.SQLiteException; import android.net.Uri; import android.os.Binder; +import android.os.Build; import android.os.Bundle; import android.os.FactoryTest; import android.os.IBinder; @@ -287,7 +288,7 @@ public final class ContentService extends IContentService.Stub { */ @Override public void registerContentObserver(Uri uri, boolean notifyForDescendants, - IContentObserver observer, int userHandle) { + IContentObserver observer, int userHandle, int targetSdkVersion) { if (observer == null || uri == null) { throw new IllegalArgumentException("You must pass a valid uri and observer"); } @@ -301,8 +302,17 @@ public final class ContentService extends IContentService.Stub { final String msg = LocalServices.getService(ActivityManagerInternal.class) .checkContentProviderAccess(uri.getAuthority(), userHandle); if (msg != null) { - Log.w(TAG, "Ignoring content changes for " + uri + " from " + uid + ": " + msg); - return; + if (targetSdkVersion >= Build.VERSION_CODES.O) { + throw new SecurityException(msg); + } else { + if (msg.startsWith("Failed to find provider")) { + // Sigh, we need to quietly let apps targeting older API + // levels notify on non-existent providers. + } else { + Log.w(TAG, "Ignoring content changes for " + uri + " from " + uid + ": " + msg); + return; + } + } } synchronized (mRootNode) { @@ -316,7 +326,7 @@ public final class ContentService extends IContentService.Stub { public void registerContentObserver(Uri uri, boolean notifyForDescendants, IContentObserver observer) { registerContentObserver(uri, notifyForDescendants, observer, - UserHandle.getCallingUserId()); + UserHandle.getCallingUserId(), Build.VERSION_CODES.CUR_DEVELOPMENT); } @Override @@ -340,8 +350,8 @@ public final class ContentService extends IContentService.Stub { */ @Override public void notifyChange(Uri uri, IContentObserver observer, - boolean observerWantsSelfNotifications, int flags, - int userHandle) { + boolean observerWantsSelfNotifications, int flags, int userHandle, + int targetSdkVersion) { if (DEBUG) Slog.d(TAG, "Notifying update of " + uri + " for user " + userHandle + " from observer " + observer + ", flags " + Integer.toHexString(flags)); @@ -359,8 +369,17 @@ public final class ContentService extends IContentService.Stub { final String msg = LocalServices.getService(ActivityManagerInternal.class) .checkContentProviderAccess(uri.getAuthority(), userHandle); if (msg != null) { - Log.w(TAG, "Ignoring notify for " + uri + " from " + uid + ": " + msg); - return; + if (targetSdkVersion >= Build.VERSION_CODES.O) { + throw new SecurityException(msg); + } else { + if (msg.startsWith("Failed to find provider")) { + // Sigh, we need to quietly let apps targeting older API + // levels notify on non-existent providers. + } else { + Log.w(TAG, "Ignoring notify for " + uri + " from " + uid + ": " + msg); + return; + } + } } // This makes it so that future permission checks will be in the context of this @@ -427,7 +446,7 @@ public final class ContentService extends IContentService.Stub { boolean observerWantsSelfNotifications, boolean syncToNetwork) { notifyChange(uri, observer, observerWantsSelfNotifications, syncToNetwork ? ContentResolver.NOTIFY_SYNC_TO_NETWORK : 0, - UserHandle.getCallingUserId()); + UserHandle.getCallingUserId(), Build.VERSION_CODES.CUR_DEVELOPMENT); } /**