Remove requireNonNull check from matchesWifiNetworkKey

matchesWifiNetworkKey expects a non-null value of the
wifiNetworkKey, however, it might be null in practice
and the null wifiNetworkKey will be stored into disk.
And then when the matchesWifiNetworkKey is called, the
requireNonNull will crash the system. Thus, remove the
requireNonNull from matchesWifiNetworkKey to avoid
system crash and handle if the wifiNetworkKey is null
then it should not match a template with non-empty
mMatchWifiNetworkKeys.

Check if WifiInfo contains a null network key then skip it
to prevent adding the identity to the network identity set.
Also, add a Log.wtf when setWifiNetworkKey(info.getNetworkKey()),
this might be useful to catch why the wifiNetworkKey is
null.

Bug: 267815242
Bug: 266598304
Test: FrmeworksNetTests
Change-Id: I9c21f7e3dca9482133c7e331741cf808105414e9
This commit is contained in:
Aaron Huang 2023-02-04 16:07:05 +08:00 • committed by Junyu Lai
commit 8ac1143594
4 changed files with 52 additions and 3 deletions

View file

@ -32,6 +32,7 @@ import android.content.Context;
import android.net.wifi.WifiInfo;
import android.service.NetworkIdentityProto;
import android.telephony.TelephonyManager;
import android.util.Log;
import android.util.proto.ProtoOutputStream;
import com.android.net.module.util.BitUtils;
@ -406,10 +407,18 @@ public class NetworkIdentity {
setOemManaged(getOemBitfield(snapshot.getNetworkCapabilities()));
if (mType == TYPE_WIFI) {
final TransportInfo transportInfo = snapshot.getNetworkCapabilities()
.getTransportInfo();
final NetworkCapabilities nc = snapshot.getNetworkCapabilities();
final TransportInfo transportInfo = nc.getTransportInfo();
if (transportInfo instanceof WifiInfo) {
final WifiInfo info = (WifiInfo) transportInfo;
// Log.wtf to catch trying to set a null wifiNetworkKey into NetworkIdentity.
// See b/266598304. The problematic data that has null wifi network key is
// thrown out when storing data, which is handled by the service.
if (info.getNetworkKey() == null) {
Log.wtf(TAG, "WifiInfo contains a null wifiNetworkKey and it will"
+ " be set into NetworkIdentity, netId=" + snapshot.getNetwork()
+ "NetworkCapabilities=" + nc);
}
setWifiNetworkKey(info.getNetworkKey());
}
} else if (mType == TYPE_TEST) {

View file

@ -700,7 +700,15 @@ public final class NetworkTemplate implements Parcelable {
* to know details about the key.
*/
private boolean matchesWifiNetworkKey(@NonNull String wifiNetworkKey) {
Objects.requireNonNull(wifiNetworkKey);
// Note that this code accepts null wifi network keys because of a past bug where wifi
// code was sending a null network key for some connected networks, which isn't expected
// and ended up stored in the data on many devices.
// A null network key in the data matches a wildcard template (one where
// {@code mMatchWifiNetworkKeys} is empty), but not one where {@code MatchWifiNetworkKeys}
// contains null. See b/266598304.
if (wifiNetworkKey == null) {
return CollectionUtils.isEmpty(mMatchWifiNetworkKeys);
}
return CollectionUtils.isEmpty(mMatchWifiNetworkKeys)
|| CollectionUtils.contains(mMatchWifiNetworkKeys, wifiNetworkKey);
}

View file

@ -106,6 +106,7 @@ import android.net.TelephonyNetworkSpecifier;
import android.net.TetherStatsParcel;
import android.net.TetheringManager;
import android.net.TrafficStats;
import android.net.TransportInfo;
import android.net.UnderlyingNetworkInfo;
import android.net.Uri;
import android.net.netstats.IUsageCallback;
@ -113,6 +114,7 @@ import android.net.netstats.NetworkStatsDataMigrationUtils;
import android.net.netstats.provider.INetworkStatsProvider;
import android.net.netstats.provider.INetworkStatsProviderCallback;
import android.net.netstats.provider.NetworkStatsProvider;
import android.net.wifi.WifiInfo;
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
@ -2125,6 +2127,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
final NetworkIdentity ident = NetworkIdentity.buildNetworkIdentity(mContext, snapshot,
isDefault, ratType);
// If WifiInfo contains a null network key then this identity should not be added into
// the network identity set. See b/266598304.
final TransportInfo transportInfo = snapshot.getNetworkCapabilities()
.getTransportInfo();
if (transportInfo instanceof WifiInfo) {
final WifiInfo info = (WifiInfo) transportInfo;
if (info.getNetworkKey() == null) continue;
}
// Traffic occurring on the base interface is always counted for
// both total usage and UID details.
final String baseIface = snapshot.getLinkProperties().getInterfaceName();

View file

@ -130,10 +130,17 @@ class NetworkTemplateTest {
mockContext, buildWifiNetworkState(null, TEST_WIFI_KEY1), true, 0)
val identWifiImsi1Key1 = buildNetworkIdentity(
mockContext, buildWifiNetworkState(TEST_IMSI1, TEST_WIFI_KEY1), true, 0)
// This identity with a null wifiNetworkKey is to test matchesWifiNetworkKey won't crash
// the system when a null wifiNetworkKey is provided, which happens because of a bug in wifi
// and it should still match the wifi wildcard template. See b/266598304.
val identWifiNullKey = buildNetworkIdentity(
mockContext, buildWifiNetworkState(null /* subscriberId */,
null /* wifiNetworkKey */), true, 0)
templateWifiWildcard.assertDoesNotMatch(identMobileImsi1)
templateWifiWildcard.assertMatches(identWifiImsiNullKey1)
templateWifiWildcard.assertMatches(identWifiImsi1Key1)
templateWifiWildcard.assertMatches(identWifiNullKey)
}
@Test
@ -148,6 +155,9 @@ class NetworkTemplateTest {
.setWifiNetworkKeys(setOf(TEST_WIFI_KEY1)).build()
val templateWifiKeyAllImsi1 = NetworkTemplate.Builder(MATCH_WIFI)
.setSubscriberIds(setOf(TEST_IMSI1)).build()
val templateNullWifiKey = NetworkTemplate(MATCH_WIFI,
emptyArray<String>() /* subscriberIds */, arrayOf(null) /* wifiNetworkKeys */,
METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL)
val identMobile1 = buildNetworkIdentity(mockContext, buildMobileNetworkState(TEST_IMSI1),
false, TelephonyManager.NETWORK_TYPE_UMTS)
@ -159,6 +169,12 @@ class NetworkTemplateTest {
mockContext, buildWifiNetworkState(TEST_IMSI2, TEST_WIFI_KEY1), true, 0)
val identWifiImsi1Key2 = buildNetworkIdentity(
mockContext, buildWifiNetworkState(TEST_IMSI1, TEST_WIFI_KEY2), true, 0)
// This identity with a null wifiNetworkKey is to test the matchesWifiNetworkKey won't crash
// the system when a null wifiNetworkKey is provided, which would happen in some unknown
// cases, see b/266598304.
val identWifiNullKey = buildNetworkIdentity(
mockContext, buildWifiNetworkState(null /* subscriberId */,
null /* wifiNetworkKey */), true, 0)
// Verify that template with WiFi Network Key only matches any subscriberId and
// specific WiFi Network Key.
@ -191,6 +207,12 @@ class NetworkTemplateTest {
templateWifiKeyAllImsi1.assertMatches(identWifiImsi1Key1)
templateWifiKeyAllImsi1.assertDoesNotMatch(identWifiImsi2Key1)
templateWifiKeyAllImsi1.assertMatches(identWifiImsi1Key2)
// Test a network identity with null wifiNetworkKey won't crash.
// It should not match a template with wifiNetworkKeys is non-null.
// Also, it should not match a template with wifiNetworkKeys that contains null.
templateWifiKey1.assertDoesNotMatch(identWifiNullKey)
templateNullWifiKey.assertDoesNotMatch(identWifiNullKey)
}
@Test