diff options
12 files changed, 716 insertions, 23 deletions
diff --git a/apishim/30/com/android/networkstack/apishim/api30/CaptivePortalDataShimImpl.java b/apishim/30/com/android/networkstack/apishim/api30/CaptivePortalDataShimImpl.java index 0c75f27..5639386 100644 --- a/apishim/30/com/android/networkstack/apishim/api30/CaptivePortalDataShimImpl.java +++ b/apishim/30/com/android/networkstack/apishim/api30/CaptivePortalDataShimImpl.java @@ -93,6 +93,16 @@ public class CaptivePortalDataShimImpl } @Override + public long getByteLimit() { + return mData.getByteLimit(); + } + + @Override + public long getExpiryTimeMillis() { + return mData.getExpiryTimeMillis(); + } + + @Override public Uri getUserPortalUrl() { return mData.getUserPortalUrl(); } diff --git a/apishim/common/com/android/networkstack/apishim/common/CaptivePortalDataShim.java b/apishim/common/com/android/networkstack/apishim/common/CaptivePortalDataShim.java index fe99c13..a18ba49 100644 --- a/apishim/common/com/android/networkstack/apishim/common/CaptivePortalDataShim.java +++ b/apishim/common/com/android/networkstack/apishim/common/CaptivePortalDataShim.java @@ -30,6 +30,16 @@ public interface CaptivePortalDataShim { boolean isCaptive(); /** + * @see android.net.CaptivePortalData#getByteLimit() + */ + long getByteLimit(); + + /** + * @see android.net.CaptivePortalData#getExpiryTimeMillis() + */ + long getExpiryTimeMillis(); + + /** * @see android.net.CaptivePortalData#getUserPortalUrl() */ Uri getUserPortalUrl(); diff --git a/jarjar-rules-shared.txt b/jarjar-rules-shared.txt index 438fc61..048c976 100644 --- a/jarjar-rules-shared.txt +++ b/jarjar-rules-shared.txt @@ -11,4 +11,4 @@ rule com.android.net.module.util.** com.android.networkstack.util.@1 # TODO: move DhcpResults into services.net and delete from here rule android.net.DhcpResultsParcelable* @0 rule android.net.DhcpResults* android.net.networkstack.DhcpResults@1 -rule android.net.LocalLog* android.net.networkstack.LocalLog@1 +rule android.util.LocalLog* android.net.networkstack.util.LocalLog@1 diff --git a/src/android/net/captiveportal/CapportApiProbeResult.java b/src/android/net/captiveportal/CapportApiProbeResult.java index f693bed..e35b791 100644 --- a/src/android/net/captiveportal/CapportApiProbeResult.java +++ b/src/android/net/captiveportal/CapportApiProbeResult.java @@ -25,11 +25,12 @@ import com.android.networkstack.apishim.common.CaptivePortalDataShim; * @hide */ public class CapportApiProbeResult extends CaptivePortalProbeResult { - @NonNull + // CaptivePortalData may be null if the capport API does not send any valid reply. + @Nullable private final CaptivePortalDataShim mCapportData; public CapportApiProbeResult(@NonNull CaptivePortalProbeResult result, - @NonNull CaptivePortalDataShim capportData) { + @Nullable CaptivePortalDataShim capportData) { this(result.mHttpResponseCode, result.redirectUrl, result.detectUrl, capportData, result.probeType); } diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java index 19ca4b5..94de7c3 100755 --- a/src/android/net/util/NetworkStackUtils.java +++ b/src/android/net/util/NetworkStackUtils.java @@ -226,6 +226,15 @@ public class NetworkStackUtils { public static final String DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION = "dns_probe_private_ip_no_internet"; + /** + * Experiment flag to enable validation metrics sent by NetworkMonitor. + * + * Metrics are sent by default. They can be disabled by setting the flag to a number greater + * than the APK version (for example 999999999). + * @see #isFeatureEnabled(Context, String, String, boolean) + */ + public static final String VALIDATION_METRICS_VERSION = "validation_metrics_version"; + static { System.loadLibrary("networkstackutilsjni"); } @@ -348,6 +357,9 @@ public class NetworkStackUtils { * {@link DeviceConfig} is enabled by comparing NetworkStack module version {@link NetworkStack} * with current version of property. If this property version is valid, the corresponding * experimental feature would be enabled, otherwise disabled. + * + * This is useful to ensure that if a module install is rolled back, flags are not left fully + * rolled out on a version where they have not been well tested. * @param context The global context information about an app environment. * @param namespace The namespace containing the property to look up. * @param name The name of the property to look up. @@ -363,6 +375,9 @@ public class NetworkStackUtils { * {@link DeviceConfig} is enabled by comparing NetworkStack module version {@link NetworkStack} * with current version of property. If this property version is valid, the corresponding * experimental feature would be enabled, otherwise disabled. + * + * This is useful to ensure that if a module install is rolled back, flags are not left fully + * rolled out on a version where they have not been well tested. * @param context The global context information about an app environment. * @param namespace The namespace containing the property to look up. * @param name The name of the property to look up. diff --git a/src/com/android/networkstack/NetworkStackNotifier.java b/src/com/android/networkstack/NetworkStackNotifier.java index 872834a..dbb62b1 100644 --- a/src/com/android/networkstack/NetworkStackNotifier.java +++ b/src/com/android/networkstack/NetworkStackNotifier.java @@ -239,7 +239,7 @@ public class NetworkStackNotifier { .setContentText(res.getString(R.string.tap_for_info)) .setContentIntent(mDependencies.getActivityPendingIntent( getContextAsUser(mContext, UserHandle.CURRENT), - infoIntent, PendingIntent.FLAG_UPDATE_CURRENT)); + infoIntent, PendingIntent.FLAG_IMMUTABLE)); networkStatus.mShownNotification = NOTE_VENUE_INFO; } else if (showValidated) { @@ -252,7 +252,7 @@ public class NetworkStackNotifier { .setContentIntent(mDependencies.getActivityPendingIntent( getContextAsUser(mContext, UserHandle.CURRENT), new Intent(Settings.ACTION_WIFI_SETTINGS), - PendingIntent.FLAG_UPDATE_CURRENT)); + PendingIntent.FLAG_IMMUTABLE)); networkStatus.mShownNotification = NOTE_CONNECTED; } else { diff --git a/src/com/android/networkstack/metrics/NetworkValidationMetrics.java b/src/com/android/networkstack/metrics/NetworkValidationMetrics.java new file mode 100644 index 0000000..f27a939 --- /dev/null +++ b/src/com/android/networkstack/metrics/NetworkValidationMetrics.java @@ -0,0 +1,254 @@ +/* + * Copyright (C) 2020 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.android.networkstack.metrics; + +import static android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; +import static android.net.NetworkCapabilities.TRANSPORT_LOWPAN; +import static android.net.NetworkCapabilities.TRANSPORT_VPN; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE; + +import static java.lang.System.currentTimeMillis; + +import android.net.INetworkMonitor; +import android.net.NetworkCapabilities; +import android.net.captiveportal.CaptivePortalProbeResult; +import android.net.metrics.ValidationProbeEvent; +import android.net.util.NetworkStackUtils; +import android.net.util.Stopwatch; +import android.stats.connectivity.ProbeResult; +import android.stats.connectivity.ProbeType; +import android.stats.connectivity.TransportType; +import android.stats.connectivity.ValidationResult; + +import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; + +import com.android.networkstack.apishim.common.CaptivePortalDataShim; + +/** + * Class to record the network validation into statsd. + * 1. Fill in NetworkValidationReported proto. + * 2. Write the NetworkValidationReported proto into statsd. + * @hide + */ + +public class NetworkValidationMetrics { + private final NetworkValidationReported.Builder mStatsBuilder = + NetworkValidationReported.newBuilder(); + private final ProbeEvents.Builder mProbeEventsBuilder = ProbeEvents.newBuilder(); + private final CapportApiData.Builder mCapportApiDataBuilder = CapportApiData.newBuilder(); + private final Stopwatch mWatch = new Stopwatch(); + private int mValidationIndex = 0; + // Define a maximum size that can store events. + public static final int MAX_PROBE_EVENTS_COUNT = 20; + + /** + * Reset this NetworkValidationMetrics and start collecting timing and metrics. + * + * <p>This must be called when validation starts. + */ + public void startCollection(@Nullable NetworkCapabilities nc) { + mStatsBuilder.clear(); + mProbeEventsBuilder.clear(); + mCapportApiDataBuilder.clear(); + mWatch.restart(); + mStatsBuilder.setTransportType(getTransportTypeFromNC(nc)); + mValidationIndex++; + } + + /** + * Returns the enum TransportType. + * + * <p>This method only supports a limited set of common transport type combinations that can be + * measured through metrics, and will return {@link TransportType#TT_UNKNOWN} for others. This + * ensures that, for example, metrics for a TRANSPORT_NEW_UNKNOWN | TRANSPORT_ETHERNET network + * cannot get aggregated with / compared with a "normal" TRANSPORT_ETHERNET network without + * noticing. + * + * @param nc Capabilities to extract transport type from. + * @return the TransportType which is defined in + * core/proto/android/stats/connectivity/network_stack.proto + */ + @VisibleForTesting + public static TransportType getTransportTypeFromNC(@Nullable NetworkCapabilities nc) { + if (nc == null) return TransportType.TT_UNKNOWN; + + final int trCount = nc.getTransportTypes().length; + boolean hasCellular = nc.hasTransport(TRANSPORT_CELLULAR); + boolean hasWifi = nc.hasTransport(TRANSPORT_WIFI); + boolean hasBT = nc.hasTransport(TRANSPORT_BLUETOOTH); + boolean hasEthernet = nc.hasTransport(TRANSPORT_ETHERNET); + boolean hasVpn = nc.hasTransport(TRANSPORT_VPN); + boolean hasWifiAware = nc.hasTransport(TRANSPORT_WIFI_AWARE); + boolean hasLopan = nc.hasTransport(TRANSPORT_LOWPAN); + + // VPN networks are not subject to validation and should not see validation stats, but + // metrics could be added to measure private DNS probes only. + if (trCount == 3 && hasCellular && hasWifi && hasVpn) { + return TransportType.TT_WIFI_CELLULAR_VPN; + } + + if (trCount == 2 && hasVpn) { + if (hasWifi) return TransportType.TT_WIFI_VPN; + if (hasCellular) return TransportType.TT_CELLULAR_VPN; + if (hasBT) return TransportType.TT_BLUETOOTH_VPN; + if (hasEthernet) return TransportType.TT_ETHERNET_VPN; + } + + if (trCount == 1) { + if (hasWifi) return TransportType.TT_WIFI; + if (hasCellular) return TransportType.TT_CELLULAR; + if (hasBT) return TransportType.TT_BLUETOOTH; + if (hasEthernet) return TransportType.TT_ETHERNET; + if (hasWifiAware) return TransportType.TT_WIFI_AWARE; + if (hasLopan) return TransportType.TT_LOWPAN; + // TODO: consider having a TT_VPN for VPN-only transport + } + + return TransportType.TT_UNKNOWN; + } + + /** + * Map {@link ValidationProbeEvent} to {@link ProbeType}. + */ + public static ProbeType probeTypeToEnum(final int probeType) { + switch(probeType) { + case ValidationProbeEvent.PROBE_DNS: + return ProbeType.PT_DNS; + case ValidationProbeEvent.PROBE_HTTP: + return ProbeType.PT_HTTP; + case ValidationProbeEvent.PROBE_HTTPS: + return ProbeType.PT_HTTPS; + case ValidationProbeEvent.PROBE_PAC: + return ProbeType.PT_PAC; + case ValidationProbeEvent.PROBE_FALLBACK: + return ProbeType.PT_FALLBACK; + case ValidationProbeEvent.PROBE_PRIVDNS: + return ProbeType.PT_PRIVDNS; + default: + return ProbeType.PT_UNKNOWN; + } + } + + /** + * Map {@link CaptivePortalProbeResult} to {@link ProbeResult}. + */ + public static ProbeResult httpProbeResultToEnum(final CaptivePortalProbeResult result) { + if (result == null) return ProbeResult.PR_UNKNOWN; + + if (result.isSuccessful()) { + return ProbeResult.PR_SUCCESS; + } else if (result.isDnsPrivateIpResponse()) { + return ProbeResult.PR_PRIVATE_IP_DNS; + } else if (result.isFailed()) { + return ProbeResult.PR_FAILURE; + } else if (result.isPortal()) { + return ProbeResult.PR_PORTAL; + } else { + return ProbeResult.PR_UNKNOWN; + } + } + + /** + * Map validation result (as per INetworkMonitor) to {@link ValidationResult}. + */ + @VisibleForTesting + public static ValidationResult validationResultToEnum(int result, String redirectUrl) { + // TODO: consider adding a VR_PARTIAL_SUCCESS field to track cases where users accepted + // partial connectivity + if ((result & INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID) != 0) { + return ValidationResult.VR_SUCCESS; + } else if (redirectUrl != null) { + return ValidationResult.VR_PORTAL; + } else if ((result & INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL) != 0) { + return ValidationResult.VR_PARTIAL; + } else { + return ValidationResult.VR_FAILURE; + } + } + + /** + * Add a network probe event to the metrics builder. + */ + public void addProbeEvent(final ProbeType type, final long durationUs, final ProbeResult result, + @Nullable final CaptivePortalDataShim capportData) { + // When the number of ProbeEvents of mProbeEventsBuilder exceeds + // MAX_PROBE_EVENTS_COUNT, stop adding ProbeEvent. + // TODO: consider recording the total number of probes in a separate field to know how + // many probes are skipped. + if (mProbeEventsBuilder.getProbeEventCount() >= MAX_PROBE_EVENTS_COUNT) return; + + int latencyUs = NetworkStackUtils.saturatedCast(durationUs); + + final ProbeEvent.Builder probeEventBuilder = ProbeEvent.newBuilder() + .setLatencyMicros(latencyUs) + .setProbeType(type) + .setProbeResult(result); + + if (capportData != null) { + final long secondsRemaining = + (capportData.getExpiryTimeMillis() - currentTimeMillis()) / 1000; + mCapportApiDataBuilder + .setRemainingTtlSecs(NetworkStackUtils.saturatedCast(secondsRemaining)) + // TODO: rename this field to setRemainingKBytes, or use a long + .setRemainingBytes( + NetworkStackUtils.saturatedCast(capportData.getByteLimit() / 1000)) + .setHasPortalUrl((capportData.getUserPortalUrl() != null)) + .setHasVenueInfo((capportData.getVenueInfoUrl() != null)); + probeEventBuilder.setCapportApiData(mCapportApiDataBuilder); + } + + mProbeEventsBuilder.addProbeEvent(probeEventBuilder); + } + + /** + * Write the network validation info to mStatsBuilder. + */ + public void setValidationResult(int result, String redirectUrl) { + mStatsBuilder.setValidationResult(validationResultToEnum(result, redirectUrl)); + } + + /** + * Write the NetworkValidationReported proto to statsd. + * + * <p>This is a no-op if {@link #startCollection(NetworkCapabilities)} was not called since the + * last call to this method. + */ + public NetworkValidationReported maybeStopCollectionAndSend() { + if (!mWatch.isStarted()) return null; + mStatsBuilder.setProbeEvents(mProbeEventsBuilder); + mStatsBuilder.setLatencyMicros(NetworkStackUtils.saturatedCast(mWatch.stop())); + mStatsBuilder.setValidationIndex(mValidationIndex); + // write a random value(0 ~ 999) for sampling. + mStatsBuilder.setRandomNumber((int) (Math.random() * 1000)); + final NetworkValidationReported stats = mStatsBuilder.build(); + final byte[] probeEvents = stats.getProbeEvents().toByteArray(); + + NetworkStackStatsLog.write(NetworkStackStatsLog.NETWORK_VALIDATION_REPORTED, + stats.getTransportType().getNumber(), + probeEvents, + stats.getValidationResult().getNumber(), + stats.getLatencyMicros(), + stats.getValidationIndex(), + stats.getRandomNumber()); + mWatch.reset(); + return stats; + } +} diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 8493816..40de26e 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java @@ -129,6 +129,8 @@ import android.os.SystemClock; import android.os.UserHandle; import android.provider.DeviceConfig; import android.provider.Settings; +import android.stats.connectivity.ProbeResult; +import android.stats.connectivity.ProbeType; import android.telephony.AccessNetworkConstants; import android.telephony.CellIdentityNr; import android.telephony.CellInfo; @@ -155,6 +157,7 @@ import androidx.annotation.Nullable; import androidx.annotation.StringRes; import androidx.annotation.VisibleForTesting; +import com.android.internal.annotations.GuardedBy; import com.android.internal.util.RingBufferIndices; import com.android.internal.util.State; import com.android.internal.util.StateMachine; @@ -168,6 +171,7 @@ import com.android.networkstack.apishim.common.ShimUtils; import com.android.networkstack.apishim.common.UnsupportedApiLevelException; import com.android.networkstack.metrics.DataStallDetectionStats; import com.android.networkstack.metrics.DataStallStatsUtils; +import com.android.networkstack.metrics.NetworkValidationMetrics; import com.android.networkstack.netlink.TcpSocketTracker; import com.android.networkstack.util.DnsUtils; import com.android.server.NetworkStackService.NetworkStackServiceManager; @@ -446,7 +450,14 @@ public class NetworkMonitor extends StateMachine { protected boolean mIsCaptivePortalCheckEnabled; private boolean mUseHttps; - // The total number of captive portal detection attempts for this NetworkMonitor instance. + /** + * The total number of completed validation attempts (network validated or a captive portal was + * detected) for this NetworkMonitor instance. + * This does not include attempts that were interrupted, retried or finished with a result that + * is not success or portal. See {@code mValidationIndex} in {@link NetworkValidationMetrics} + * for a count of all attempts. + * TODO: remove when removing legacy metrics. + */ private int mValidations = 0; // Set if the user explicitly selected "Do not use this network" in captive portal sign-in app. @@ -500,6 +511,18 @@ public class NetworkMonitor extends StateMachine { private final boolean mPrivateIpNoInternetEnabled; + private final boolean mMetricsEnabled; + + // The validation metrics are accessed by individual probe threads, and by the StateMachine + // thread. All accesses must be synchronized to make sure the StateMachine thread can see + // reports from all probes. + // TODO: as that most usage is in the StateMachine thread and probes only add their probe + // events, consider having probes return their stats to the StateMachine, and only access this + // member on the StateMachine thread without synchronization. + @GuardedBy("mNetworkValidationMetrics") + private final NetworkValidationMetrics mNetworkValidationMetrics = + new NetworkValidationMetrics(); + private int getCallbackVersion(INetworkMonitorCallbacks cb) { int version; try { @@ -563,6 +586,8 @@ public class NetworkMonitor extends StateMachine { mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled(); mPrivateIpNoInternetEnabled = getIsPrivateIpNoInternetEnabled(); + mMetricsEnabled = deps.isFeatureEnabled(context, NAMESPACE_CONNECTIVITY, + NetworkStackUtils.VALIDATION_METRICS_VERSION, true /* defaultEnabled */); mUseHttps = getUseHttpsValidation(); mCaptivePortalUserAgent = getCaptivePortalUserAgent(); mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls(); @@ -775,6 +800,51 @@ public class NetworkMonitor extends StateMachine { } } + private void startMetricsCollection() { + if (!mMetricsEnabled) return; + try { + synchronized (mNetworkValidationMetrics) { + mNetworkValidationMetrics.startCollection(mNetworkCapabilities); + } + } catch (Exception e) { + Log.wtf(TAG, "Error resetting validation metrics", e); + } + } + + private void recordProbeEventMetrics(ProbeType type, long latencyMicros, ProbeResult result, + CaptivePortalDataShim capportData) { + if (!mMetricsEnabled) return; + try { + synchronized (mNetworkValidationMetrics) { + mNetworkValidationMetrics.addProbeEvent(type, latencyMicros, result, capportData); + } + } catch (Exception e) { + Log.wtf(TAG, "Error recording probe event", e); + } + } + + private void recordValidationResult(int result, String redirectUrl) { + if (!mMetricsEnabled) return; + try { + synchronized (mNetworkValidationMetrics) { + mNetworkValidationMetrics.setValidationResult(result, redirectUrl); + } + } catch (Exception e) { + Log.wtf(TAG, "Error recording validation result", e); + } + } + + private void maybeStopCollectionAndSendMetrics() { + if (!mMetricsEnabled) return; + try { + synchronized (mNetworkValidationMetrics) { + mNetworkValidationMetrics.maybeStopCollectionAndSend(); + } + } catch (Exception e) { + Log.wtf(TAG, "Error sending validation stats", e); + } + } + // DefaultState is the parent of all States. It exists only to handle CMD_* messages but // does not entail any real state (hence no enter() or exit() routines). private class DefaultState extends State { @@ -787,6 +857,7 @@ public class NetworkMonitor extends StateMachine { transitionTo(mEvaluatingState); return HANDLED; case CMD_NETWORK_DISCONNECTED: + maybeStopCollectionAndSendMetrics(); logNetworkEvent(NetworkEvent.NETWORK_DISCONNECTED); quit(); return HANDLED; @@ -938,6 +1009,7 @@ public class NetworkMonitor extends StateMachine { initSocketTrackingIfRequired(); // start periodical polling. sendTcpPollingEvent(); + maybeStopCollectionAndSendMetrics(); } private void initSocketTrackingIfRequired() { @@ -957,6 +1029,9 @@ public class NetworkMonitor extends StateMachine { transitionTo(mValidatedState); break; case CMD_EVALUATE_PRIVATE_DNS: + // TODO: this causes reevaluation of a single probe that is not counted in + // metrics. Add support for such reevaluation probes in metrics, and log them + // separately. transitionTo(mEvaluatingPrivateDnsState); break; case EVENT_DNS_NOTIFICATION: @@ -1285,6 +1360,7 @@ public class NetworkMonitor extends StateMachine { sendMessageDelayed(CMD_CAPTIVE_PORTAL_RECHECK, 0 /* no UID */, CAPTIVE_PORTAL_REEVALUATE_DELAY_MS); mValidations++; + maybeStopCollectionAndSendMetrics(); } @Override @@ -1316,6 +1392,12 @@ public class NetworkMonitor extends StateMachine { notifyPrivateDnsConfigResolved(); } else { handlePrivateDnsEvaluationFailure(); + // The private DNS probe fails-fast if the server hostname cannot + // be resolved. Record it as a failure with zero latency. + // TODO: refactor this together with the probe recorded in + // sendPrivateDnsProbe, so logging is symmetric / easier to follow. + recordProbeEventMetrics(ProbeType.PT_PRIVDNS, 0 /* latency */, + ProbeResult.PR_FAILURE, null /* capportData */); break; } } @@ -1425,6 +1507,8 @@ public class NetworkMonitor extends StateMachine { validationLog(PROBE_PRIVDNS, host, String.format("%dus - Error: %s", time, uhe.getMessage())); } + recordProbeEventMetrics(ProbeType.PT_PRIVDNS, time, success ? ProbeResult.PR_SUCCESS : + ProbeResult.PR_FAILURE, null /* capportData */); logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); return success; } @@ -1435,6 +1519,14 @@ public class NetworkMonitor extends StateMachine { @Override public void enter() { + // When starting a full probe cycle here, record any pending stats (for example if + // CMD_FORCE_REEVALUATE was called before evaluation finished, as can happen in + // EvaluatingPrivateDnsState). + maybeStopCollectionAndSendMetrics(); + // Restart the metrics collection timers. Metrics will be stopped and sent when the + // validation attempt finishes (as success, failure or portal), or if it is interrupted + // (by being restarted or if NetworkMonitor stops). + startMetricsCollection(); if (mEvaluateAttempts >= BLAME_FOR_EVALUATION_ATTEMPTS) { //Don't continue to blame UID forever. TrafficStats.clearThreadStatsUid(); @@ -1516,6 +1608,9 @@ public class NetworkMonitor extends StateMachine { private class WaitingForNextProbeState extends State { @Override public void enter() { + // Send metrics for this evaluation attempt. Metrics collection (and its timers) will be + // restarted when the next probe starts. + maybeStopCollectionAndSendMetrics(); scheduleNextProbe(); } @@ -2265,6 +2360,8 @@ public class NetworkMonitor extends StateMachine { // network validation (the HTTPS probe, which would likely fail anyway) or the PAC probe. if (mPrivateIpNoInternetEnabled && probeType == ValidationProbeEvent.PROBE_HTTP && (proxy == null) && hasPrivateIpAddress(resolvedAddr)) { + recordProbeEventMetrics(NetworkValidationMetrics.probeTypeToEnum(probeType), + 0 /* latency */, ProbeResult.PR_PRIVATE_IP_DNS, null /* capportData */); return CaptivePortalProbeResult.PRIVATE_IP; } return sendHttpProbe(url, probeType, null); @@ -2296,6 +2393,9 @@ public class NetworkMonitor extends StateMachine { result = ValidationProbeEvent.DNS_FAILURE; } final long latency = watch.stop(); + recordProbeEventMetrics(ProbeType.PT_DNS, latency, + (result == ValidationProbeEvent.DNS_SUCCESS) ? ProbeResult.PR_SUCCESS : + ProbeResult.PR_FAILURE, null /* capportData */); logValidationProbe(latency, ValidationProbeEvent.PROBE_DNS, result); return addresses; } @@ -2413,12 +2513,17 @@ public class NetworkMonitor extends StateMachine { } logValidationProbe(probeTimer.stop(), probeType, httpResponseCode); + final CaptivePortalProbeResult probeResult; if (probeSpec == null) { - return new CaptivePortalProbeResult(httpResponseCode, redirectUrl, url.toString(), - 1 << probeType); + probeResult = new CaptivePortalProbeResult(httpResponseCode, redirectUrl, + url.toString(), 1 << probeType); } else { - return probeSpec.getResult(httpResponseCode, redirectUrl); + probeResult = probeSpec.getResult(httpResponseCode, redirectUrl); } + recordProbeEventMetrics(NetworkValidationMetrics.probeTypeToEnum(probeType), + probeTimer.stop(), NetworkValidationMetrics.httpProbeResultToEnum(probeResult), + null /* capportData */); + return probeResult; } @VisibleForTesting @@ -2570,8 +2675,9 @@ public class NetworkMonitor extends StateMachine { super(deps, proxy, url, captivePortalApiUrl); } - private CaptivePortalDataShim tryCapportApiProbe() { - if (mCaptivePortalApiUrl == null) return null; + private CaptivePortalDataShim sendCapportApiProbe() { + // TODO: consider adding metrics counters for each case returning null in this method + // (cases where the API is not implemented properly). validationLog("Fetching captive portal data from " + mCaptivePortalApiUrl); final String apiContent; @@ -2610,26 +2716,38 @@ public class NetworkMonitor extends StateMachine { try { final JSONObject info = new JSONObject(apiContent); - return CaptivePortalDataShimImpl.fromJson(info); + final CaptivePortalDataShim capportData = CaptivePortalDataShimImpl.fromJson(info); + if (capportData != null && capportData.isCaptive() + && capportData.getUserPortalUrl() == null) { + validationLog("Missing user-portal-url from capport response"); + return null; + } + return capportData; } catch (JSONException e) { validationLog("Could not parse capport API JSON: " + e.getMessage()); return null; } catch (UnsupportedApiLevelException e) { + // This should never happen because LinkProperties would not have a capport URL + // before R. validationLog("Platform API too low to support capport API"); return null; } } + private CaptivePortalDataShim tryCapportApiProbe() { + if (mCaptivePortalApiUrl == null) return null; + final Stopwatch capportApiWatch = new Stopwatch().start(); + final CaptivePortalDataShim capportData = sendCapportApiProbe(); + recordProbeEventMetrics(ProbeType.PT_CAPPORT_API, capportApiWatch.stop(), + capportData == null ? ProbeResult.PR_FAILURE : ProbeResult.PR_SUCCESS, + capportData); + return capportData; + } + @Override protected CaptivePortalProbeResult sendProbe() { final CaptivePortalDataShim capportData = tryCapportApiProbe(); if (capportData != null && capportData.isCaptive()) { - if (capportData.getUserPortalUrl() == null) { - validationLog("Missing user-portal-url from capport response"); - return new CapportApiProbeResult( - sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTP), - capportData); - } final String loginUrlString = capportData.getUserPortalUrl().toString(); // Starting from R (where CaptivePortalData was introduced), the captive portal app // delegates to NetworkMonitor for verifying when the network validates instead of @@ -3343,6 +3461,7 @@ public class NetworkMonitor extends StateMachine { p.redirectUrl = redirectUrl; p.timestampMillis = SystemClock.elapsedRealtime(); notifyNetworkTested(p); + recordValidationResult(result, redirectUrl); } @VisibleForTesting diff --git a/tests/integration/Android.bp b/tests/integration/Android.bp index 1a985a9..af63f0e 100644 --- a/tests/integration/Android.bp +++ b/tests/integration/Android.bp @@ -82,6 +82,18 @@ android_test { test_suites: ["device-tests"], } +// The static lib needs to be jarjared by each module so they do not conflict with each other +// (e.g. wifi, system server, network stack need to use different package names when including it). +// Apply NetworkStack jarjar rules to the tests as well so classes in NetworkStaticLibTests have the +// same package names as in module code. +android_library { + name: "NetworkStackStaticLibTestsLib", + platform_apis: true, + min_sdk_version: "29", + jarjar_rules: ":NetworkStackJarJarRules", + static_libs: ["NetworkStaticLibTestsLib"], +} + // Special version of the network stack tests that includes all tests necessary for code coverage // purposes. This is currently the union of NetworkStackTests and NetworkStackIntegrationTests. android_test { @@ -95,7 +107,7 @@ android_test { static_libs: [ "NetworkStackTestsLib", "NetworkStackIntegrationTestsLib", - "NetworkStaticLibTestsLib", + "NetworkStackStaticLibTestsLib", ], compile_multilib: "both", manifest: "AndroidManifest_coverage.xml", diff --git a/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt b/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt index b2607eb..348392d 100644 --- a/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt +++ b/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt @@ -22,7 +22,7 @@ import android.app.NotificationManager import android.app.NotificationManager.IMPORTANCE_DEFAULT import android.app.NotificationManager.IMPORTANCE_NONE import android.app.PendingIntent -import android.app.PendingIntent.FLAG_UPDATE_CURRENT +import android.app.PendingIntent.FLAG_IMMUTABLE import android.content.Context import android.content.Intent import android.content.res.Resources @@ -57,6 +57,7 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq +import org.mockito.ArgumentMatchers.intThat import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito.any @@ -188,7 +189,8 @@ class NetworkStackNotifierTest { assertEquals(CHANNEL_CONNECTED, note.channelId) assertEquals(timeout, note.timeoutAfter) verify(mDependencies).getActivityPendingIntent( - eq(mCurrentUserContext), mIntentCaptor.capture(), eq(FLAG_UPDATE_CURRENT)) + eq(mCurrentUserContext), mIntentCaptor.capture(), + intThat { it or FLAG_IMMUTABLE != 0 }) } private fun verifyCanceledNotificationAfterNetworkLost() { @@ -279,7 +281,8 @@ class NetworkStackNotifierTest { verify(mNm).notify(eq(TEST_NETWORK_TAG), mNoteIdCaptor.capture(), mNoteCaptor.capture()) verify(mDependencies).getActivityPendingIntent( - eq(mCurrentUserContext), mIntentCaptor.capture(), eq(FLAG_UPDATE_CURRENT)) + eq(mCurrentUserContext), mIntentCaptor.capture(), + intThat { it or FLAG_IMMUTABLE != 0 }) verifyVenueInfoIntent(mIntentCaptor.value) verifyCanceledNotificationAfterDefaultNetworkLost() } diff --git a/tests/unit/src/com/android/networkstack/metrics/NetworkValidationMetricsTest.java b/tests/unit/src/com/android/networkstack/metrics/NetworkValidationMetricsTest.java new file mode 100644 index 0000000..98e7b63 --- /dev/null +++ b/tests/unit/src/com/android/networkstack/metrics/NetworkValidationMetricsTest.java @@ -0,0 +1,247 @@ +/* + * Copyright (C) 2020 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.android.networkstack.metrics; + +import static android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; +import static android.net.NetworkCapabilities.TRANSPORT_VPN; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; + +import static org.junit.Assert.assertTrue; + +import android.net.INetworkMonitor; +import android.net.NetworkCapabilities; +import android.net.captiveportal.CaptivePortalProbeResult; +import android.net.metrics.ValidationProbeEvent; +import android.stats.connectivity.ProbeResult; +import android.stats.connectivity.ProbeType; +import android.stats.connectivity.TransportType; +import android.stats.connectivity.ValidationResult; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import com.android.networkstack.apishim.CaptivePortalDataShimImpl; +import com.android.networkstack.apishim.common.CaptivePortalDataShim; + +import org.json.JSONObject; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NetworkValidationMetricsTest { + private static final String TEST_LOGIN_URL = "https://testportal.example.com/login"; + private static final String TEST_VENUE_INFO_URL = "https://venue.example.com/info"; + private static final int TTL_TOLERANCE_SECS = 10; + + private static final NetworkCapabilities WIFI_CAPABILITIES = + new NetworkCapabilities() + .addTransportType(NetworkCapabilities.TRANSPORT_WIFI); + + @Test + public void testNetworkValidationMetrics_VerifyProbeTypeToEnum() throws Exception { + verifyProbeType(ValidationProbeEvent.PROBE_DNS, ProbeType.PT_DNS); + verifyProbeType(ValidationProbeEvent.PROBE_HTTP, ProbeType.PT_HTTP); + verifyProbeType(ValidationProbeEvent.PROBE_HTTPS, ProbeType.PT_HTTPS); + verifyProbeType(ValidationProbeEvent.PROBE_PAC, ProbeType.PT_PAC); + verifyProbeType(ValidationProbeEvent.PROBE_FALLBACK, ProbeType.PT_FALLBACK); + verifyProbeType(ValidationProbeEvent.PROBE_PRIVDNS, ProbeType.PT_PRIVDNS); + } + + private void verifyProbeType(int inputProbeType, ProbeType expectedEnumType) { + assertEquals(expectedEnumType, NetworkValidationMetrics.probeTypeToEnum(inputProbeType)); + } + + @Test + public void testNetworkValidationMetrics_VerifyHttpProbeResultToEnum() throws Exception { + verifyProbeType(new CaptivePortalProbeResult(CaptivePortalProbeResult.SUCCESS_CODE, + ValidationProbeEvent.PROBE_HTTP), ProbeResult.PR_SUCCESS); + verifyProbeType(new CaptivePortalProbeResult(CaptivePortalProbeResult.FAILED_CODE, + ValidationProbeEvent.PROBE_HTTP), ProbeResult.PR_FAILURE); + verifyProbeType(new CaptivePortalProbeResult(CaptivePortalProbeResult.PORTAL_CODE, + ValidationProbeEvent.PROBE_HTTP), ProbeResult.PR_PORTAL); + verifyProbeType(CaptivePortalProbeResult.PRIVATE_IP, ProbeResult.PR_PRIVATE_IP_DNS); + } + + private void verifyProbeType(CaptivePortalProbeResult inputResult, + ProbeResult expectedResult) { + assertEquals(expectedResult, NetworkValidationMetrics.httpProbeResultToEnum(inputResult)); + } + + @Test + public void testNetworkValidationMetrics_VerifyValidationResultToEnum() throws Exception { + verifyProbeType(INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID, null, + ValidationResult.VR_SUCCESS); + verifyProbeType(0, TEST_LOGIN_URL, ValidationResult.VR_PORTAL); + verifyProbeType(INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL, null, + ValidationResult.VR_PARTIAL); + verifyProbeType(0, null, ValidationResult.VR_FAILURE); + } + + private void verifyProbeType(int inputResult, String inputRedirectUrl, + ValidationResult expectedResult) { + assertEquals(expectedResult, NetworkValidationMetrics.validationResultToEnum(inputResult, + inputRedirectUrl)); + } + + @Test + public void testNetworkValidationMetrics_VerifyTransportTypeToEnum() throws Exception { + final NetworkValidationMetrics metrics = new NetworkValidationMetrics(); + NetworkCapabilities nc = new NetworkCapabilities(); + nc.addTransportType(TRANSPORT_WIFI); + assertEquals(TransportType.TT_WIFI, metrics.getTransportTypeFromNC(nc)); + nc.addTransportType(TRANSPORT_VPN); + assertEquals(TransportType.TT_WIFI_VPN, metrics.getTransportTypeFromNC(nc)); + nc.addTransportType(TRANSPORT_CELLULAR); + assertEquals(TransportType.TT_WIFI_CELLULAR_VPN, metrics.getTransportTypeFromNC(nc)); + + nc = new NetworkCapabilities(); + nc.addTransportType(TRANSPORT_CELLULAR); + assertEquals(TransportType.TT_CELLULAR, metrics.getTransportTypeFromNC(nc)); + nc.addTransportType(TRANSPORT_VPN); + assertEquals(TransportType.TT_CELLULAR_VPN, metrics.getTransportTypeFromNC(nc)); + + nc = new NetworkCapabilities(); + nc.addTransportType(TRANSPORT_BLUETOOTH); + assertEquals(TransportType.TT_BLUETOOTH, metrics.getTransportTypeFromNC(nc)); + nc.addTransportType(TRANSPORT_VPN); + assertEquals(TransportType.TT_BLUETOOTH_VPN, metrics.getTransportTypeFromNC(nc)); + + nc = new NetworkCapabilities(); + nc.addTransportType(TRANSPORT_ETHERNET); + assertEquals(TransportType.TT_ETHERNET, metrics.getTransportTypeFromNC(nc)); + nc.addTransportType(TRANSPORT_VPN); + assertEquals(TransportType.TT_ETHERNET_VPN, metrics.getTransportTypeFromNC(nc)); + } + + @Test + public void testNetworkValidationMetrics_VerifyConsecutiveProbeFailure() throws Exception { + final NetworkValidationMetrics metrics = new NetworkValidationMetrics(); + metrics.startCollection(WIFI_CAPABILITIES); + // 1. PT_DNS probe + metrics.addProbeEvent(ProbeType.PT_DNS, 1234, ProbeResult.PR_SUCCESS, null); + // 2. Consecutive PT_HTTP probe failure + for (int i = 0; i < 30; i++) { + metrics.addProbeEvent(ProbeType.PT_HTTP, 1234, ProbeResult.PR_FAILURE, null); + } + + // Write metric into statsd + final NetworkValidationReported stats = metrics.maybeStopCollectionAndSend(); + + // The maximum number of probe records should be the same as MAX_PROBE_EVENTS_COUNT + final ProbeEvents probeEvents = stats.getProbeEvents(); + assertEquals(NetworkValidationMetrics.MAX_PROBE_EVENTS_COUNT, + probeEvents.getProbeEventCount()); + } + + @Test + public void testNetworkValidationMetrics_VerifyCollectMetrics() throws Exception { + final long bytesRemaining = 12_345L; + final long secondsRemaining = 3000L; + String apiContent = "{'captive': true," + + "'user-portal-url': '" + TEST_LOGIN_URL + "'," + + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'," + + "'bytes-remaining': " + bytesRemaining + "," + + "'seconds-remaining': " + secondsRemaining + "}"; + final NetworkValidationMetrics metrics = new NetworkValidationMetrics(); + final int validationIndex = 1; + final long longlatency = Integer.MAX_VALUE + 12344567L; + metrics.startCollection(WIFI_CAPABILITIES); + + final JSONObject info = new JSONObject(apiContent); + final CaptivePortalDataShim captivePortalData = CaptivePortalDataShimImpl.isSupported() + ? CaptivePortalDataShimImpl.fromJson(info) : null; + + // 1. PT_CAPPORT_API probe w CapportApiData info + metrics.addProbeEvent(ProbeType.PT_CAPPORT_API, 1234, ProbeResult.PR_SUCCESS, + captivePortalData); + // 2. PT_CAPPORT_API probe w/o CapportApiData info + metrics.addProbeEvent(ProbeType.PT_CAPPORT_API, 1234, ProbeResult.PR_FAILURE, null); + + // 3. PT_DNS probe + metrics.addProbeEvent(ProbeType.PT_DNS, 5678, ProbeResult.PR_FAILURE, null); + + // 4. PT_HTTP probe + metrics.addProbeEvent(ProbeType.PT_HTTP, longlatency, ProbeResult.PR_PORTAL, null); + + // add Validation result + metrics.setValidationResult(INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL, null); + + // Write metric into statsd + final NetworkValidationReported stats = metrics.maybeStopCollectionAndSend(); + + // Verify: TransportType: WIFI + assertEquals(TransportType.TT_WIFI, stats.getTransportType()); + + // Verify: validationIndex + assertEquals(validationIndex, stats.getValidationIndex()); + + // probe Count: 4 (PT_CAPPORT_API, PT_DNS, PT_HTTP, PT_CAPPORT_API) + final ProbeEvents probeEvents = stats.getProbeEvents(); + assertEquals(4, probeEvents.getProbeEventCount()); + + // Verify the 1st probe: ProbeType = PT_CAPPORT_API, Latency_us = 1234, + // ProbeResult = PR_SUCCESS, CapportApiData = capportData + ProbeEvent probeEvent = probeEvents.getProbeEvent(0); + assertEquals(ProbeType.PT_CAPPORT_API, probeEvent.getProbeType()); + assertEquals(1234, probeEvent.getLatencyMicros()); + assertEquals(ProbeResult.PR_SUCCESS, probeEvent.getProbeResult()); + if (CaptivePortalDataShimImpl.isSupported()) { + assertTrue(probeEvent.hasCapportApiData()); + // Set secondsRemaining to 3000 and check that getRemainingTtlSecs is within 10 seconds + final CapportApiData capportData = probeEvent.getCapportApiData(); + assertTrue(capportData.getRemainingTtlSecs() <= secondsRemaining); + assertTrue(capportData.getRemainingTtlSecs() + TTL_TOLERANCE_SECS > secondsRemaining); + assertEquals(captivePortalData.getByteLimit() / 1000, capportData.getRemainingBytes()); + } else { + assertFalse(probeEvent.hasCapportApiData()); + } + + // Verify the 2nd probe: ProbeType = PT_CAPPORT_API, Latency_us = 1234, + // ProbeResult = PR_SUCCESS, CapportApiData = null + probeEvent = probeEvents.getProbeEvent(1); + assertEquals(ProbeType.PT_CAPPORT_API, probeEvent.getProbeType()); + assertEquals(1234, probeEvent.getLatencyMicros()); + assertEquals(ProbeResult.PR_FAILURE, probeEvent.getProbeResult()); + assertEquals(false, probeEvent.hasCapportApiData()); + + // Verify the 3rd probe: ProbeType = PT_DNS, Latency_us = 5678, + // ProbeResult = PR_FAILURE, CapportApiData = null + probeEvent = probeEvents.getProbeEvent(2); + assertEquals(ProbeType.PT_DNS, probeEvent.getProbeType()); + assertEquals(5678, probeEvent.getLatencyMicros()); + assertEquals(ProbeResult.PR_FAILURE, probeEvent.getProbeResult()); + assertEquals(false, probeEvent.hasCapportApiData()); + + // Verify the 4th probe: ProbeType = PT_HTTP, Latency_us = longlatency, + // ProbeResult = PR_PORTAL, CapportApiData = null + probeEvent = probeEvents.getProbeEvent(3); + assertEquals(ProbeType.PT_HTTP, probeEvent.getProbeType()); + // The latency exceeds Integer.MAX_VALUE(2147483647), it is limited to Integer.MAX_VALUE + assertEquals(Integer.MAX_VALUE, probeEvent.getLatencyMicros()); + assertEquals(ProbeResult.PR_PORTAL, probeEvent.getProbeResult()); + assertEquals(false, probeEvent.hasCapportApiData()); + + // Verify the ValidationResult + assertEquals(ValidationResult.VR_PARTIAL, stats.getValidationResult()); + } +} diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 00fe17c..a3ef532 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java @@ -1152,7 +1152,29 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_CapportApiIsPortal() throws Exception { + public void testIsCaptivePortal_CapportApiIsPortalWithNullPortalUrl() throws Exception { + assumeTrue(CaptivePortalDataShimImpl.isSupported()); + setSslException(mHttpsConnection); + final long bytesRemaining = 10_000L; + final long secondsRemaining = 500L; + // Set content without partal url. + setApiContent(mCapportApiConnection, "{'captive': true," + + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'," + + "'bytes-remaining': " + bytesRemaining + "," + + "'seconds-remaining': " + secondsRemaining + "}"); + setPortal302(mHttpConnection); + + runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL, + 0 /* probesSucceeded*/, TEST_LOGIN_URL); + + verify(mCapportApiConnection).getResponseCode(); + + verify(mHttpConnection, times(1)).getResponseCode(); + verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any()); + } + + @Test + public void testIsCaptivePortal_CapportApiIsPortalWithValidPortalUrl() throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); setSslException(mHttpsConnection); final long bytesRemaining = 10_000L; |