From bbf352a2c136bfeb1fa7ad751df75d3069e9fdb0 Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Thu, 3 Nov 2016 15:46:17 +0000 Subject: [PATCH] DO NOT MERGE [DPM] DO uses batch token to retrieve network logs, and can retrieve the same batch many times This allows DO to: a) know that some logs were dropped (by trying with token and not getting anything) b) know how many logs were there in each batch (useful especially for the dropped ones) c) retry batch retrieval if it failed Test: will be CTS tested once APIs unhidden Bug: 29748723 (cherry picked from commit a9ff206af26871695bfce54969428b8ad03e31e6) Change-Id: Iac10e61cdf3b100719a9c029ff897bd5ef5c8e2f --- .../app/admin/DeviceAdminReceiver.java | 34 ++++++++++++++++--- .../app/admin/DevicePolicyManager.java | 17 ++++++---- .../app/admin/IDevicePolicyManager.aidl | 2 +- .../DevicePolicyManagerService.java | 22 +++++++----- .../server/devicepolicy/NetworkLogger.java | 4 +-- .../devicepolicy/NetworkLoggingHandler.java | 20 +++++++---- 6 files changed, 72 insertions(+), 27 deletions(-) diff --git a/core/java/android/app/admin/DeviceAdminReceiver.java b/core/java/android/app/admin/DeviceAdminReceiver.java index 360087c4dfe1d..cbd5a6d548328 100644 --- a/core/java/android/app/admin/DeviceAdminReceiver.java +++ b/core/java/android/app/admin/DeviceAdminReceiver.java @@ -284,6 +284,27 @@ public class DeviceAdminReceiver extends BroadcastReceiver { public static final String ACTION_NETWORK_LOGS_AVAILABLE = "android.app.action.NETWORK_LOGS_AVAILABLE"; + /** + * A {@code long} containing a token of the current batch of network logs, that has to be used + * to retrieve the batch of logs by the device owner. + * + * @see #ACTION_NETWORK_LOGS_AVAILABLE + * @see DevicePolicyManager#retrieveNetworkLogs + * @hide + */ + public static final String EXTRA_NETWORK_LOGS_TOKEN = + "android.app.extra.EXTRA_NETWORK_LOGS_TOKEN"; + + /** + * An {@code int} count representing a total count of network logs inside the current batch of + * network logs. + * + * @see #ACTION_NETWORK_LOGS_AVAILABLE + * @hide + */ + public static final String EXTRA_NETWORK_LOGS_COUNT = + "android.app.extra.EXTRA_NETWORK_LOGS_COUNT"; + /** * A string containing the SHA-256 hash of the bugreport file. * @@ -644,19 +665,22 @@ public class DeviceAdminReceiver extends BroadcastReceiver { } /** - * Called when a new batch of network logs can be retrieved. This callback method will only ever - * be called when network logging is enabled. The logs can only be retrieved while network + * Called each time a new batch of network logs can be retrieved. This callback method will only + * ever be called when network logging is enabled. The logs can only be retrieved while network * logging is enabled. * *

This callback is only applicable to device owners. * * @param context The running context as per {@link #onReceive}. * @param intent The received intent as per {@link #onReceive}. + * @param batchToken The token representing the current batch of network logs. + * @param networkLogsCount The total count of events in the current batch of network logs. * @see DevicePolicyManager#retrieveNetworkLogs(ComponentName) * * @hide */ - public void onNetworkLogsAvailable(Context context, Intent intent) { + public void onNetworkLogsAvailable(Context context, Intent intent, long batchToken, + int networkLogsCount) { } /** @@ -714,7 +738,9 @@ public class DeviceAdminReceiver extends BroadcastReceiver { } else if (ACTION_SECURITY_LOGS_AVAILABLE.equals(action)) { onSecurityLogsAvailable(context, intent); } else if (ACTION_NETWORK_LOGS_AVAILABLE.equals(action)) { - onNetworkLogsAvailable(context, intent); + long batchToken = intent.getLongExtra(EXTRA_NETWORK_LOGS_TOKEN, -1); + int networkLogsCount = intent.getIntExtra(EXTRA_NETWORK_LOGS_COUNT, 0); + onNetworkLogsAvailable(context, intent, batchToken, networkLogsCount); } } } diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 0f72ae8134211..c2197b55ef974 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -6621,8 +6621,6 @@ public class DevicePolicyManager { * @param admin Which {@link DeviceAdminReceiver} this request is associated with. * @param enabled whether network logging should be enabled or not. * @throws {@link SecurityException} if {@code admin} is not a device owner. - * @throws {@link RemoteException} if network logging could not be enabled or disabled due to - * the logging service not being available * @see #retrieveNetworkLogs * * @hide @@ -6655,7 +6653,10 @@ public class DevicePolicyManager { } /** - * Called by device owner to retrieve a new batch of network logging events. + * Called by device owner to retrieve the most recent batch of network logging events. + * A device owner has to provide a batchToken provided as part of + * {@link DeviceAdminReceiver#onNetworkLogsAvailable} callback. If the token doesn't match the + * token of the most recent available batch of logs, {@code null} will be returned. * *

{@link NetworkEvent} can be one of {@link DnsEvent} or {@link ConnectEvent}. * @@ -6666,16 +6667,20 @@ public class DevicePolicyManager { * {@link DeviceAdminReceiver#onNetworkLogsAvailable}. * * @param admin Which {@link DeviceAdminReceiver} this request is associated with. + * @param batchToken A token of the batch to retrieve * @return A new batch of network logs which is a list of {@link NetworkEvent}. Returns - * {@code null} if there's no batch currently awaiting for retrieval or if logging is disabled. + * {@code null} if the batch represented by batchToken is no longer available or if + * logging is disabled. * @throws {@link SecurityException} if {@code admin} is not a device owner. + * @see DeviceAdminReceiver#onNetworkLogsAvailable * * @hide */ - public List retrieveNetworkLogs(@NonNull ComponentName admin) { + public @Nullable List retrieveNetworkLogs(@NonNull ComponentName admin, + long batchToken) { throwIfParentInstance("retrieveNetworkLogs"); try { - return mService.retrieveNetworkLogs(admin); + return mService.retrieveNetworkLogs(admin, batchToken); } catch (RemoteException re) { throw re.rethrowFromSystemServer(); } diff --git a/core/java/android/app/admin/IDevicePolicyManager.aidl b/core/java/android/app/admin/IDevicePolicyManager.aidl index 3779bf398e0f2..f0710ec269eba 100644 --- a/core/java/android/app/admin/IDevicePolicyManager.aidl +++ b/core/java/android/app/admin/IDevicePolicyManager.aidl @@ -315,5 +315,5 @@ interface IDevicePolicyManager { void setNetworkLoggingEnabled(in ComponentName admin, boolean enabled); boolean isNetworkLoggingEnabled(in ComponentName admin); - List retrieveNetworkLogs(in ComponentName admin); + List retrieveNetworkLogs(in ComponentName admin, long batchToken); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index ad020b9bbde9f..86b697fd7fcec 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -488,9 +488,12 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { * to listen for events. */ if (Intent.ACTION_USER_STARTED.equals(action) - && userHandle == mOwners.getDeviceOwnerUserId() - && isNetworkLoggingEnabledInternal()) { - setNetworkLoggingActiveInternal(true); + && userHandle == mOwners.getDeviceOwnerUserId()) { + synchronized (DevicePolicyManagerService.this) { + if (isNetworkLoggingEnabledInternalLocked()) { + setNetworkLoggingActiveInternal(true); + } + } } if (Intent.ACTION_BOOT_COMPLETED.equals(action) && userHandle == mOwners.getDeviceOwnerUserId() @@ -9433,7 +9436,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { Preconditions.checkNotNull(admin); ensureDeviceOwnerManagingSingleUser(admin); - if (enabled == isNetworkLoggingEnabledInternal()) { + if (enabled == isNetworkLoggingEnabledInternalLocked()) { // already in the requested state return; } @@ -9474,11 +9477,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { Preconditions.checkNotNull(admin); synchronized (this) { getActiveAdminForCallerLocked(admin, DeviceAdminInfo.USES_POLICY_DEVICE_OWNER); - return isNetworkLoggingEnabledInternal(); + return isNetworkLoggingEnabledInternalLocked(); } } - private synchronized boolean isNetworkLoggingEnabledInternal() { + private boolean isNetworkLoggingEnabledInternalLocked() { ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); return (deviceOwner != null) && deviceOwner.isNetworkLoggingEnabled; } @@ -9489,7 +9492,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { * Ideally this would be done with ParceledList, however it only supports homogeneous types. */ @Override - public synchronized List retrieveNetworkLogs(ComponentName admin) { + public synchronized List retrieveNetworkLogs(ComponentName admin, + long batchToken) { if (!mHasFeature) { return null; } @@ -9499,6 +9503,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { if (mNetworkLogger == null) { return null; } - return isNetworkLoggingEnabledInternal() ? mNetworkLogger.retrieveLogs() : null; + return isNetworkLoggingEnabledInternalLocked() + ? mNetworkLogger.retrieveLogs(batchToken) + : null; } } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java index 185ccc232aa9c..8cb13da07c16f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java @@ -147,7 +147,7 @@ final class NetworkLogger { } } - List retrieveLogs() { - return mNetworkLoggingHandler.retrieveFullLogBatch(); + List retrieveLogs(long batchToken) { + return mNetworkLoggingHandler.retrieveFullLogBatch(batchToken); } } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java index 96884e6db7119..98e5570a2f6b5 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java @@ -55,11 +55,15 @@ final class NetworkLoggingHandler extends Handler { private final DevicePolicyManagerService mDpm; // threadsafe as it's Handler's thread confined + @GuardedBy("this") private ArrayList mNetworkEvents = new ArrayList(); @GuardedBy("this") private ArrayList mFullBatch; + @GuardedBy("this") + private long currentFullBatchToken; + NetworkLoggingHandler(Looper looper, DevicePolicyManagerService dpm) { super(looper); mDpm = dpm; @@ -97,17 +101,21 @@ final class NetworkLoggingHandler extends Handler { scheduleBatchFinalization(BATCH_FINALIZATION_TIMEOUT_MS); // notify DO that there's a new non-empty batch waiting if (mFullBatch.size() > 0) { - mDpm.sendDeviceOwnerCommand(DeviceAdminReceiver.ACTION_NETWORK_LOGS_AVAILABLE, - /* extras */ null); + currentFullBatchToken++; + Bundle extras = new Bundle(); + extras.putLong(DeviceAdminReceiver.EXTRA_NETWORK_LOGS_TOKEN, currentFullBatchToken); + extras.putInt(DeviceAdminReceiver.EXTRA_NETWORK_LOGS_COUNT, mFullBatch.size()); + mDpm.sendDeviceOwnerCommand(DeviceAdminReceiver.ACTION_NETWORK_LOGS_AVAILABLE, extras); } else { mFullBatch = null; } } - synchronized List retrieveFullLogBatch() { - List ret = mFullBatch; - mFullBatch = null; - return ret; + synchronized List retrieveFullLogBatch(long batchToken) { + if (batchToken != currentFullBatchToken) { + return null; + } + return mFullBatch; } }