diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index 246e5bcce2419b2a469f80176554ba264ab37dc0..2a25a86f34c49f2fd1b2c53107204b541607c749 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -827,7 +827,38 @@ public class IpServer extends StateMachine { } } catch (ServiceSpecificException | RemoteException e) { mLog.e("Failed to add " + mIfaceName + " to local table: ", e); - return; + } + } + + private void addInterfaceForward(@NonNull final String fromIface, + @NonNull final String toIface) throws ServiceSpecificException, RemoteException { + if (null != mRoutingCoordinator.value) { + mRoutingCoordinator.value.addInterfaceForward(fromIface, toIface); + } else { + mNetd.tetherAddForward(fromIface, toIface); + mNetd.ipfwdAddInterfaceForward(fromIface, toIface); + } + } + + private void removeInterfaceForward(@NonNull final String fromIface, + @NonNull final String toIface) { + if (null != mRoutingCoordinator.value) { + try { + mRoutingCoordinator.value.removeInterfaceForward(fromIface, toIface); + } catch (ServiceSpecificException e) { + mLog.e("Exception in removeInterfaceForward", e); + } + } else { + try { + mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception in ipfwdRemoveInterfaceForward", e); + } + try { + mNetd.tetherRemoveForward(fromIface, toIface); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception in disableNat", e); + } } } @@ -1337,16 +1368,7 @@ public class IpServer extends StateMachine { // to remove their rules, which generates errors. // Just do the best we can. mBpfCoordinator.maybeDetachProgram(mIfaceName, upstreamIface); - try { - mNetd.ipfwdRemoveInterfaceForward(mIfaceName, upstreamIface); - } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception in ipfwdRemoveInterfaceForward: " + e.toString()); - } - try { - mNetd.tetherRemoveForward(mIfaceName, upstreamIface); - } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception in disableNat: " + e.toString()); - } + removeInterfaceForward(mIfaceName, upstreamIface); } @Override @@ -1402,10 +1424,9 @@ public class IpServer extends StateMachine { mBpfCoordinator.maybeAttachProgram(mIfaceName, ifname); try { - mNetd.tetherAddForward(mIfaceName, ifname); - mNetd.ipfwdAddInterfaceForward(mIfaceName, ifname); + addInterfaceForward(mIfaceName, ifname); } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception enabling NAT: " + e.toString()); + mLog.e("Exception enabling iface forward", e); cleanupUpstream(); mLastError = TETHER_ERROR_ENABLE_FORWARDING_ERROR; transitionTo(mInitialState); diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index fc9928d6a7a29f610dcfc72fbd218ad412006069..15df3babcd59a8f5ac7e26d19297e4b36c7ad386 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -321,6 +321,23 @@ public class IpServerTest { when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(DEFAULT_USING_BPF_OFFLOAD); when(mTetherConfig.useLegacyDhcpServer()).thenReturn(false /* default value */); + // Simulate the behavior of RoutingCoordinator + if (null != mRoutingCoordinatorManager.value) { + doAnswer(it -> { + final String fromIface = (String) it.getArguments()[0]; + final String toIface = (String) it.getArguments()[1]; + mNetd.tetherAddForward(fromIface, toIface); + mNetd.ipfwdAddInterfaceForward(fromIface, toIface); + return null; + }).when(mRoutingCoordinatorManager.value).addInterfaceForward(any(), any()); + doAnswer(it -> { + final String fromIface = (String) it.getArguments()[0]; + final String toIface = (String) it.getArguments()[1]; + mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); + mNetd.tetherRemoveForward(fromIface, toIface); + return null; + }).when(mRoutingCoordinatorManager.value).removeInterfaceForward(any(), any()); + } mBpfDeps = new BpfCoordinator.Dependencies() { @NonNull public Handler getHandler() { diff --git a/framework/src/android/net/IRoutingCoordinator.aidl b/framework/src/android/net/IRoutingCoordinator.aidl index a5cda9874e1606337e02e693bcd8c3efa5ca3540..cf02ec47aa42de9e8ed52ff7d94455fa8a333841 100644 --- a/framework/src/android/net/IRoutingCoordinator.aidl +++ b/framework/src/android/net/IRoutingCoordinator.aidl @@ -72,4 +72,24 @@ interface IRoutingCoordinator { * unix errno. */ void removeInterfaceFromNetwork(int netId, in String iface); + + /** + * Add forwarding ip rule + * + * @param fromIface interface name to add forwarding ip rule + * @param toIface interface name to add forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + void addInterfaceForward(in String fromIface, in String toIface); + + /** + * Remove forwarding ip rule + * + * @param fromIface interface name to remove forwarding ip rule + * @param toIface interface name to remove forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + void removeInterfaceForward(in String fromIface, in String toIface); } diff --git a/framework/src/android/net/RoutingCoordinatorManager.java b/framework/src/android/net/RoutingCoordinatorManager.java index 5576cb0f23316b3bef880d338cf5a3111c122e55..a9e7eef51e2c5381dea9cedb4eeffdc79fb4e633 100644 --- a/framework/src/android/net/RoutingCoordinatorManager.java +++ b/framework/src/android/net/RoutingCoordinatorManager.java @@ -123,4 +123,36 @@ public class RoutingCoordinatorManager { throw e.rethrowFromSystemServer(); } } + + /** + * Add forwarding ip rule + * + * @param fromIface interface name to add forwarding ip rule + * @param toIface interface name to add forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + public void addInterfaceForward(final String fromIface, final String toIface) { + try { + mService.addInterfaceForward(fromIface, toIface); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Remove forwarding ip rule + * + * @param fromIface interface name to remove forwarding ip rule + * @param toIface interface name to remove forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + public void removeInterfaceForward(final String fromIface, final String toIface) { + try { + mService.removeInterfaceForward(fromIface, toIface); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } } diff --git a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java index 50e84d4d220209efc19ba63066afe6c6f2bb3b4d..92ea610235f52e977a86f20704328c0e4f5d4827 100644 --- a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java +++ b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java @@ -19,12 +19,16 @@ package com.android.server.connectivity; import static com.android.net.module.util.NetdUtils.toRouteInfoParcel; import android.annotation.NonNull; -import android.content.Context; import android.net.INetd; import android.net.IRoutingCoordinator; import android.net.RouteInfo; import android.os.RemoteException; import android.os.ServiceSpecificException; +import android.util.ArraySet; + +import com.android.internal.annotations.GuardedBy; + +import java.util.Objects; /** * Class to coordinate routing across multiple clients. @@ -115,4 +119,89 @@ public class RoutingCoordinatorService extends IRoutingCoordinator.Stub { throws ServiceSpecificException, RemoteException { mNetd.networkRemoveInterface(netId, iface); } + + private final Object mIfacesLock = new Object(); + private static final class ForwardingPair { + @NonNull public final String fromIface; + @NonNull public final String toIface; + ForwardingPair(@NonNull final String fromIface, @NonNull final String toIface) { + this.fromIface = fromIface; + this.toIface = toIface; + } + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (!(o instanceof ForwardingPair)) return false; + + final ForwardingPair that = (ForwardingPair) o; + + return fromIface.equals(that.fromIface) && toIface.equals(that.toIface); + } + + @Override + public int hashCode() { + int result = fromIface.hashCode(); + result = 2 * result + toIface.hashCode(); + return result; + } + } + + @GuardedBy("mIfacesLock") + private final ArraySet<ForwardingPair> mForwardedInterfaces = new ArraySet<>(); + + /** + * Add forwarding ip rule + * + * @param fromIface interface name to add forwarding ip rule + * @param toIface interface name to add forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + public void addInterfaceForward(final String fromIface, final String toIface) + throws ServiceSpecificException, RemoteException { + Objects.requireNonNull(fromIface); + Objects.requireNonNull(toIface); + synchronized (mIfacesLock) { + if (mForwardedInterfaces.size() == 0) { + mNetd.ipfwdEnableForwarding("RoutingCoordinator"); + } + final ForwardingPair fwp = new ForwardingPair(fromIface, toIface); + if (mForwardedInterfaces.contains(fwp)) { + throw new IllegalStateException("Forward already exists between ifaces " + + fromIface + " → " + toIface); + } + mForwardedInterfaces.add(fwp); + // Enables NAT for v4 and filters packets from unknown interfaces + mNetd.tetherAddForward(fromIface, toIface); + mNetd.ipfwdAddInterfaceForward(fromIface, toIface); + } + } + + /** + * Remove forwarding ip rule + * + * @param fromIface interface name to remove forwarding ip rule + * @param toIface interface name to remove forwarding ip rule + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + public void removeInterfaceForward(final String fromIface, final String toIface) + throws ServiceSpecificException, RemoteException { + Objects.requireNonNull(fromIface); + Objects.requireNonNull(toIface); + synchronized (mIfacesLock) { + final ForwardingPair fwp = new ForwardingPair(fromIface, toIface); + if (!mForwardedInterfaces.contains(fwp)) { + throw new IllegalStateException("No forward set up between interfaces " + + fromIface + " → " + toIface); + } + mForwardedInterfaces.remove(fwp); + mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); + mNetd.tetherRemoveForward(fromIface, toIface); + if (mForwardedInterfaces.size() == 0) { + mNetd.ipfwdDisableForwarding("RoutingCoordinator"); + } + } + } } diff --git a/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt b/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..8adf309aadab26416df148f79a4914dbf680f959 --- /dev/null +++ b/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2023 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.server.connectivity + +import android.net.INetd +import android.os.Build +import androidx.test.platform.app.InstrumentationRegistry +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import com.android.testutils.DevSdkIgnoreRunner +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.inOrder +import org.mockito.Mockito.mock +import kotlin.test.assertFailsWith + +@RunWith(DevSdkIgnoreRunner::class) +@IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) +class RoutingCoordinatorServiceTest { + val mNetd = mock(INetd::class.java) + val mService = RoutingCoordinatorService(mNetd) + + @Test + fun testInterfaceForward() { + val inOrder = inOrder(mNetd) + + mService.addInterfaceForward("from1", "to1") + inOrder.verify(mNetd).ipfwdEnableForwarding(any()) + inOrder.verify(mNetd).tetherAddForward("from1", "to1") + inOrder.verify(mNetd).ipfwdAddInterfaceForward("from1", "to1") + + mService.addInterfaceForward("from2", "to1") + inOrder.verify(mNetd).tetherAddForward("from2", "to1") + inOrder.verify(mNetd).ipfwdAddInterfaceForward("from2", "to1") + + assertFailsWith<IllegalStateException> { + // Can't add the same pair again + mService.addInterfaceForward("from2", "to1") + } + + mService.removeInterfaceForward("from1", "to1") + inOrder.verify(mNetd).ipfwdRemoveInterfaceForward("from1", "to1") + inOrder.verify(mNetd).tetherRemoveForward("from1", "to1") + + mService.removeInterfaceForward("from2", "to1") + inOrder.verify(mNetd).ipfwdRemoveInterfaceForward("from2", "to1") + inOrder.verify(mNetd).tetherRemoveForward("from2", "to1") + + inOrder.verify(mNetd).ipfwdDisableForwarding(any()) + } +}