From 59d98c07a74fbd7fc1706c1c3d83bb7c47e9d053 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Thu, 16 Jun 2022 14:21:25 -0700 Subject: [PATCH] Validate or set the eth specifier on update Iface When a calling the updateInterface API, if no eth specifier was set it would be removed not allowing follow-up network requests to request by ethernet specifier. This CL makes sure that the eth specifier is always set as expected when updateInterface is called by either validating the input or setting the specifier if the passed one is null. Bug: 236294399 Bug: 229207021 Test: atest FrameworksNetTests :com.android.server.ethernet.EthernetNetworkFactoryTest Change-Id: Ic05ca7fe4adaa94f79e59758569f7595ab4c4f54 --- .../server/ethernet/EthernetServiceImpl.java | 46 ++++++++++--- .../ethernet/EthernetServiceImplTest.java | 64 ++++++++++++++++++- 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index 71d3e4f43e..f058f94da5 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -16,12 +16,14 @@ package com.android.server.ethernet; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; import android.content.pm.PackageManager; +import android.net.EthernetNetworkSpecifier; import android.net.EthernetNetworkUpdateRequest; import android.net.IEthernetManager; import android.net.IEthernetServiceListener; @@ -29,6 +31,7 @@ import android.net.INetworkInterfaceOutcomeReceiver; import android.net.ITetheredInterfaceCallback; import android.net.IpConfiguration; import android.net.NetworkCapabilities; +import android.net.NetworkSpecifier; import android.os.Binder; import android.os.Handler; import android.os.RemoteException; @@ -216,19 +219,39 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { "EthernetServiceImpl"); } - private void maybeValidateTestCapabilities(final String iface, - @Nullable final NetworkCapabilities nc) { + private void validateOrSetNetworkSpecifier(String iface, NetworkCapabilities nc) { + final NetworkSpecifier spec = nc.getNetworkSpecifier(); + if (spec == null) { + nc.setNetworkSpecifier(new EthernetNetworkSpecifier(iface)); + return; + } + if (!(spec instanceof EthernetNetworkSpecifier)) { + throw new IllegalArgumentException("Invalid specifier type for request."); + } + if (!((EthernetNetworkSpecifier) spec).getInterfaceName().matches(iface)) { + throw new IllegalArgumentException("Invalid interface name set on specifier."); + } + } + + private void maybeValidateTestCapabilities(String iface, NetworkCapabilities nc) { if (!mTracker.isValidTestInterface(iface)) { return; } - // For test interfaces, only null or capabilities that include TRANSPORT_TEST are - // allowed. - if (nc != null && !nc.hasTransport(TRANSPORT_TEST)) { + if (!nc.hasTransport(TRANSPORT_TEST)) { throw new IllegalArgumentException( "Updates to test interfaces must have NetworkCapabilities.TRANSPORT_TEST."); } } + private void maybeValidateEthernetTransport(String iface, NetworkCapabilities nc) { + if (mTracker.isValidTestInterface(iface)) { + return; + } + if (!nc.hasSingleTransport(TRANSPORT_ETHERNET)) { + throw new IllegalArgumentException("Invalid transport type for request."); + } + } + private void enforceAdminPermission(final String iface, boolean enforceAutomotive, final String logMessage) { if (mTracker.isValidTestInterface(iface)) { @@ -251,12 +274,17 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { // TODO: validate that iface is listed in overlay config_ethernet_interfaces // only automotive devices are allowed to set the NetworkCapabilities using this API - enforceAdminPermission(iface, request.getNetworkCapabilities() != null, - "updateConfiguration() with non-null capabilities"); - maybeValidateTestCapabilities(iface, request.getNetworkCapabilities()); + final NetworkCapabilities nc = request.getNetworkCapabilities(); + enforceAdminPermission( + iface, nc != null, "updateConfiguration() with non-null capabilities"); + if (nc != null) { + validateOrSetNetworkSpecifier(iface, nc); + maybeValidateTestCapabilities(iface, nc); + maybeValidateEthernetTransport(iface, nc); + } mTracker.updateConfiguration( - iface, request.getIpConfiguration(), request.getNetworkCapabilities(), listener); + iface, request.getIpConfiguration(), nc, listener); } @Override diff --git a/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java index b2b9f2c3d4..a1d93a025e 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -16,6 +16,7 @@ package com.android.server.ethernet; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static org.junit.Assert.assertThrows; @@ -35,10 +36,12 @@ import android.Manifest; import android.annotation.NonNull; import android.content.Context; import android.content.pm.PackageManager; +import android.net.EthernetNetworkSpecifier; import android.net.EthernetNetworkUpdateRequest; import android.net.INetworkInterfaceOutcomeReceiver; import android.net.IpConfiguration; import android.net.NetworkCapabilities; +import android.net.StringNetworkSpecifier; import android.os.Build; import android.os.Handler; @@ -56,10 +59,14 @@ import org.junit.runner.RunWith; @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) public class EthernetServiceImplTest { private static final String TEST_IFACE = "test123"; + private static final NetworkCapabilities DEFAULT_CAPS = new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_ETHERNET) + .setNetworkSpecifier(new EthernetNetworkSpecifier(TEST_IFACE)) + .build(); private static final EthernetNetworkUpdateRequest UPDATE_REQUEST = new EthernetNetworkUpdateRequest.Builder() .setIpConfiguration(new IpConfiguration()) - .setNetworkCapabilities(new NetworkCapabilities.Builder().build()) + .setNetworkCapabilities(DEFAULT_CAPS) .build(); private static final EthernetNetworkUpdateRequest UPDATE_REQUEST_WITHOUT_CAPABILITIES = new EthernetNetworkUpdateRequest.Builder() @@ -67,7 +74,7 @@ public class EthernetServiceImplTest { .build(); private static final EthernetNetworkUpdateRequest UPDATE_REQUEST_WITHOUT_IP_CONFIG = new EthernetNetworkUpdateRequest.Builder() - .setNetworkCapabilities(new NetworkCapabilities.Builder().build()) + .setNetworkCapabilities(DEFAULT_CAPS) .build(); private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null; private EthernetServiceImpl mEthernetServiceImpl; @@ -160,6 +167,41 @@ public class EthernetServiceImplTest { }); } + @Test + public void testUpdateConfigurationRejectsWithInvalidSpecifierType() { + final StringNetworkSpecifier invalidSpecifierType = new StringNetworkSpecifier("123"); + final EthernetNetworkUpdateRequest request = + new EthernetNetworkUpdateRequest.Builder() + .setNetworkCapabilities( + new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_ETHERNET) + .setNetworkSpecifier(invalidSpecifierType) + .build() + ).build(); + assertThrows(IllegalArgumentException.class, () -> { + mEthernetServiceImpl.updateConfiguration( + "" /* iface */, request, null /* listener */); + }); + } + + @Test + public void testUpdateConfigurationRejectsWithInvalidSpecifierName() { + final String ifaceToUpdate = "eth0"; + final String ifaceOnSpecifier = "wlan0"; + EthernetNetworkUpdateRequest request = + new EthernetNetworkUpdateRequest.Builder() + .setNetworkCapabilities( + new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_ETHERNET) + .setNetworkSpecifier( + new EthernetNetworkSpecifier(ifaceOnSpecifier)) + .build() + ).build(); + assertThrows(IllegalArgumentException.class, () -> { + mEthernetServiceImpl.updateConfiguration(ifaceToUpdate, request, null /* listener */); + }); + } + @Test public void testUpdateConfigurationWithCapabilitiesWithAutomotiveFeature() { toggleAutomotiveFeature(false); @@ -246,6 +288,24 @@ public class EthernetServiceImplTest { eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER)); } + @Test + public void testUpdateConfigurationAddsSpecifierWhenNotSet() { + final NetworkCapabilities nc = new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_ETHERNET).build(); + final EthernetNetworkUpdateRequest requestSansSpecifier = + new EthernetNetworkUpdateRequest.Builder() + .setNetworkCapabilities(nc) + .build(); + final NetworkCapabilities ncWithSpecifier = new NetworkCapabilities(nc) + .setNetworkSpecifier(new EthernetNetworkSpecifier(TEST_IFACE)); + + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, requestSansSpecifier, NULL_LISTENER); + verify(mEthernetTracker).updateConfiguration( + eq(TEST_IFACE), + isNull(), + eq(ncWithSpecifier), eq(NULL_LISTENER)); + } + @Test public void testEnableInterface() { mEthernetServiceImpl.enableInterface(TEST_IFACE, NULL_LISTENER);