Merge "Set EthernetNetworkSpecifier on ethernet networks"
diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
index cc8103b..f266386 100644
--- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -20,6 +20,7 @@
import android.annotation.Nullable;
import android.content.Context;
import android.net.ConnectivityManager;
+import android.net.EthernetManager;
import android.net.EthernetNetworkSpecifier;
import android.net.IEthernetNetworkManagementListener;
import android.net.EthernetNetworkManagementException;
@@ -190,6 +191,18 @@
updateCapabilityFilter();
}
+ @VisibleForTesting
+ protected int getInterfaceState(@NonNull String iface) {
+ final NetworkInterfaceState interfaceState = mTrackingInterfaces.get(iface);
+ if (interfaceState == null) {
+ return EthernetManager.STATE_ABSENT;
+ } else if (!interfaceState.mLinkUp) {
+ return EthernetManager.STATE_LINK_DOWN;
+ } else {
+ return EthernetManager.STATE_LINK_UP;
+ }
+ }
+
/**
* Update a network's configuration and restart it if necessary.
*
@@ -274,7 +287,8 @@
return iface.updateLinkState(up, listener);
}
- boolean hasInterface(String ifaceName) {
+ @VisibleForTesting
+ protected boolean hasInterface(String ifaceName) {
return mTrackingInterfaces.containsKey(ifaceName);
}
diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
index dffac37..c1c6d3a 100644
--- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
@@ -143,10 +143,8 @@
* Adds a listener.
* @param listener A {@link IEthernetServiceListener} to add.
*/
- public void addListener(IEthernetServiceListener listener) {
- if (listener == null) {
- throw new IllegalArgumentException("listener must not be null");
- }
+ public void addListener(IEthernetServiceListener listener) throws RemoteException {
+ Objects.requireNonNull(listener, "listener must not be null");
PermissionUtils.enforceAccessNetworkStatePermission(mContext, TAG);
mTracker.addListener(listener, checkUseRestrictedNetworksPermission());
}
@@ -208,6 +206,12 @@
pw.decreaseIndent();
}
+ private void enforceNetworkManagementPermission() {
+ mContext.enforceCallingOrSelfPermission(
+ android.Manifest.permission.MANAGE_ETHERNET_NETWORKS,
+ "EthernetServiceImpl");
+ }
+
/**
* Validate the state of ethernet for APIs tied to network management.
*
@@ -216,12 +220,12 @@
*/
private void validateNetworkManagementState(@NonNull final String iface,
final @NonNull String methodName) {
+ enforceAutomotiveDevice(methodName);
+ enforceNetworkManagementPermission();
logIfEthernetNotStarted();
- // TODO: add permission check here for MANAGE_INTERNAL_NETWORKS when it's available.
Objects.requireNonNull(iface, "Pass a non-null iface.");
Objects.requireNonNull(methodName, "Pass a non-null methodName.");
- enforceAutomotiveDevice(methodName);
enforceInterfaceIsTracked(iface);
}
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 6c8e6d3..2571fe6 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -23,6 +23,7 @@
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.Context;
+import android.net.EthernetManager;
import android.net.IEthernetServiceListener;
import android.net.IEthernetNetworkManagementListener;
import android.net.INetd;
@@ -165,7 +166,10 @@
Log.i(TAG, "updateIpConfiguration, iface: " + iface + ", cfg: " + ipConfiguration);
}
writeIpConfiguration(iface, ipConfiguration);
- mHandler.post(() -> mFactory.updateInterface(iface, ipConfiguration, null, null));
+ mHandler.post(() -> {
+ mFactory.updateInterface(iface, ipConfiguration, null, null);
+ broadcastInterfaceStateChange(iface);
+ });
}
private void writeIpConfiguration(@NonNull final String iface,
@@ -174,6 +178,55 @@
mIpConfigurations.put(iface, ipConfig);
}
+ private IpConfiguration getIpConfigurationForCallback(String iface, int state) {
+ return (state == EthernetManager.STATE_ABSENT) ? null : getOrCreateIpConfiguration(iface);
+ }
+
+ private void ensureRunningOnEthernetServiceThread() {
+ if (mHandler.getLooper().getThread() != Thread.currentThread()) {
+ throw new IllegalStateException(
+ "Not running on EthernetService thread: "
+ + Thread.currentThread().getName());
+ }
+ }
+
+ /**
+ * Broadcast the link state or IpConfiguration change of existing Ethernet interfaces to all
+ * listeners.
+ */
+ protected void broadcastInterfaceStateChange(@NonNull String iface) {
+ ensureRunningOnEthernetServiceThread();
+ final int state = mFactory.getInterfaceState(iface);
+ final int role = getInterfaceRole(iface);
+ final IpConfiguration config = getIpConfigurationForCallback(iface, state);
+ final int n = mListeners.beginBroadcast();
+ for (int i = 0; i < n; i++) {
+ try {
+ mListeners.getBroadcastItem(i).onInterfaceStateChanged(iface, state, role, config);
+ } catch (RemoteException e) {
+ // Do nothing here.
+ }
+ }
+ mListeners.finishBroadcast();
+ }
+
+ /**
+ * Unicast the interface state or IpConfiguration change of existing Ethernet interfaces to a
+ * specific listener.
+ */
+ protected void unicastInterfaceStateChange(@NonNull IEthernetServiceListener listener,
+ @NonNull String iface) {
+ ensureRunningOnEthernetServiceThread();
+ final int state = mFactory.getInterfaceState(iface);
+ final int role = getInterfaceRole(iface);
+ final IpConfiguration config = getIpConfigurationForCallback(iface, state);
+ try {
+ listener.onInterfaceStateChanged(iface, state, role, config);
+ } catch (RemoteException e) {
+ // Do nothing here.
+ }
+ }
+
@VisibleForTesting(visibility = PACKAGE)
protected void updateConfiguration(@NonNull final String iface,
@NonNull final StaticIpConfiguration staticIpConfig,
@@ -186,8 +239,10 @@
final IpConfiguration ipConfig = createIpConfiguration(staticIpConfig);
writeIpConfiguration(iface, ipConfig);
mNetworkCapabilities.put(iface, capabilities);
- mHandler.post(() ->
- mFactory.updateInterface(iface, ipConfig, capabilities, listener));
+ mHandler.post(() -> {
+ mFactory.updateInterface(iface, ipConfig, capabilities, listener);
+ broadcastInterfaceStateChange(iface);
+ });
}
@VisibleForTesting(visibility = PACKAGE)
@@ -225,11 +280,19 @@
}
void addListener(IEthernetServiceListener listener, boolean canUseRestrictedNetworks) {
- mListeners.register(listener, new ListenerInfo(canUseRestrictedNetworks));
+ mHandler.post(() -> {
+ if (!mListeners.register(listener, new ListenerInfo(canUseRestrictedNetworks))) {
+ // Remote process has already died
+ return;
+ }
+ for (String iface : getInterfaces(canUseRestrictedNetworks)) {
+ unicastInterfaceStateChange(listener, iface);
+ }
+ });
}
void removeListener(IEthernetServiceListener listener) {
- mListeners.unregister(listener);
+ mHandler.post(() -> mListeners.unregister(listener));
}
public void setIncludeTestInterfaces(boolean include) {
@@ -295,6 +358,14 @@
}
}
+ private int getInterfaceRole(final String iface) {
+ if (!mFactory.hasInterface(iface)) return EthernetManager.ROLE_NONE;
+ final int mode = getInterfaceMode(iface);
+ return (mode == INTERFACE_MODE_CLIENT)
+ ? EthernetManager.ROLE_CLIENT
+ : EthernetManager.ROLE_SERVER;
+ }
+
private int getInterfaceMode(final String iface) {
if (iface.equals(mDefaultInterface)) {
return mDefaultInterfaceMode;
@@ -312,6 +383,7 @@
if (iface.equals(mDefaultInterface)) {
mDefaultInterface = null;
}
+ broadcastInterfaceStateChange(iface);
}
private void addInterface(String iface) {
@@ -346,11 +418,7 @@
final int mode = getInterfaceMode(iface);
if (mode == INTERFACE_MODE_CLIENT) {
- IpConfiguration ipConfiguration = mIpConfigurations.get(iface);
- if (ipConfiguration == null) {
- ipConfiguration = createDefaultIpConfiguration();
- }
-
+ IpConfiguration ipConfiguration = getOrCreateIpConfiguration(iface);
Log.d(TAG, "Tracking interface in client mode: " + iface);
mFactory.addInterface(iface, hwAddress, ipConfiguration, nc);
} else {
@@ -376,22 +444,7 @@
&& mFactory.updateInterfaceLinkState(iface, up, listener);
if (factoryLinkStateUpdated) {
- boolean restricted = isRestrictedInterface(iface);
- int n = mListeners.beginBroadcast();
- for (int i = 0; i < n; i++) {
- try {
- if (restricted) {
- ListenerInfo listenerInfo = (ListenerInfo) mListeners.getBroadcastCookie(i);
- if (!listenerInfo.canUseRestrictedNetworks) {
- continue;
- }
- }
- mListeners.getBroadcastItem(i).onAvailabilityChanged(iface, up);
- } catch (RemoteException e) {
- // Do nothing here.
- }
- }
- mListeners.finishBroadcast();
+ broadcastInterfaceStateChange(iface);
}
}
@@ -438,6 +491,8 @@
}
addInterface(iface);
+
+ broadcastInterfaceStateChange(iface);
}
private void trackAvailableInterfaces() {
@@ -463,11 +518,17 @@
@Override
public void onInterfaceAdded(String iface) {
+ if (DBG) {
+ Log.i(TAG, "onInterfaceAdded, iface: " + iface);
+ }
mHandler.post(() -> maybeTrackInterface(iface));
}
@Override
public void onInterfaceRemoved(String iface) {
+ if (DBG) {
+ Log.i(TAG, "onInterfaceRemoved, iface: " + iface);
+ }
mHandler.post(() -> stopTrackingInterface(iface));
}
}
@@ -663,8 +724,10 @@
return ret;
}
- private static IpConfiguration createDefaultIpConfiguration() {
- final IpConfiguration ret = new IpConfiguration();
+ private IpConfiguration getOrCreateIpConfiguration(String iface) {
+ IpConfiguration ret = mIpConfigurations.get(iface);
+ if (ret != null) return ret;
+ ret = new IpConfiguration();
ret.setIpAssignment(IpAssignment.DHCP);
ret.setProxySettings(ProxySettings.NONE);
return ret;
diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
index ae52a75..6cb13ef 100644
--- a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
+++ b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
@@ -367,7 +367,7 @@
assertFalse(ret);
verifyNoStopOrStart();
- assertFailedListener(listener, "can't be updated as it is not configured");
+ assertFailedListener(listener, "can't be updated as it is not available");
}
@Test
@@ -461,9 +461,10 @@
.build();
mNetFactory.needNetworkFor(specificNetRequest);
- // TODO(b/155707957): BUG: IPClient should not be started when the interface link state
- // is down.
- verify(mDeps).makeIpClient(any(), any(), any());
+ verify(mDeps, never()).makeIpClient(any(), any(), any());
+
+ mNetFactory.updateInterfaceLinkState(TEST_IFACE, true, NULL_LISTENER);
+ verify(mDeps).makeIpClient(any(), eq(TEST_IFACE), any());
}
@Test
diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
index 18d6f3b..0ac28c4 100644
--- a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
@@ -18,10 +18,13 @@
import static org.junit.Assert.assertThrows;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
+import android.Manifest;
import android.annotation.NonNull;
import android.content.Context;
import android.content.pm.PackageManager;
@@ -176,6 +179,36 @@
});
}
+ private void denyManageEthPermission() {
+ doThrow(new SecurityException("")).when(mContext)
+ .enforceCallingOrSelfPermission(
+ eq(Manifest.permission.MANAGE_ETHERNET_NETWORKS), anyString());
+ }
+
+ @Test
+ public void testUpdateConfigurationRejectsWithoutManageEthPermission() {
+ denyManageEthPermission();
+ assertThrows(SecurityException.class, () -> {
+ mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER);
+ });
+ }
+
+ @Test
+ public void testConnectNetworkRejectsWithoutManageEthPermission() {
+ denyManageEthPermission();
+ assertThrows(SecurityException.class, () -> {
+ mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER);
+ });
+ }
+
+ @Test
+ public void testDisconnectNetworkRejectsWithoutManageEthPermission() {
+ denyManageEthPermission();
+ assertThrows(SecurityException.class, () -> {
+ mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER);
+ });
+ }
+
@Test
public void testUpdateConfiguration() {
mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER);