Fix service resolve on tethering downstreams
Tethering downstreams do not have NetworkAgents, and although they have
a netid of 99, Networks with netId 99 are not usable by apps for most
connectivity APIs.
Recent refactoring in NsdService adds the Network of a found service
into its NsdServiceInfo, and uses that network to resolve the service.
In that case the Network has netId 99 and resolving the service fails.
Avoid that problem by:
- Keeping the Network field null when a service is found on a tethering
downstream; this avoids giving apps a confusing and unusable Network
with netId 99
- Using the interface index found during discovery to resolve the
service, if the app uses the NsdServiceInfo that was obtained from
discovery to resolve. If not, all interfaces will be used to resolve,
as per legacy APIs.
Bug: 233979892
Test: atest NsdServiceTest
Also manual test with 2 devices connected via hotspot
Change-Id: Idd176153b67ccbd1d4f1b1fd66dafaa2f3a9e27a
(cherry picked from commit 1a8ee102d3cc9829b669b09685b17ea92815b671)
Merged-In: Idd176153b67ccbd1d4f1b1fd66dafaa2f3a9e27a
diff --git a/framework-t/src/android/net/nsd/NsdServiceInfo.java b/framework-t/src/android/net/nsd/NsdServiceInfo.java
index 2621594..200c808 100644
--- a/framework-t/src/android/net/nsd/NsdServiceInfo.java
+++ b/framework-t/src/android/net/nsd/NsdServiceInfo.java
@@ -53,6 +53,8 @@
@Nullable
private Network mNetwork;
+ private int mInterfaceIndex;
+
public NsdServiceInfo() {
}
@@ -312,8 +314,11 @@
/**
* Get the network where the service can be found.
*
- * This is never null if this {@link NsdServiceInfo} was obtained from
- * {@link NsdManager#discoverServices} or {@link NsdManager#resolveService}.
+ * This is set if this {@link NsdServiceInfo} was obtained from
+ * {@link NsdManager#discoverServices} or {@link NsdManager#resolveService}, unless the service
+ * was found on a network interface that does not have a {@link Network} (such as a tethering
+ * downstream, where services are advertised from devices connected to this device via
+ * tethering).
*/
@Nullable
public Network getNetwork() {
@@ -329,6 +334,26 @@
mNetwork = network;
}
+ /**
+ * Get the index of the network interface where the service was found.
+ *
+ * This is only set when the service was found on an interface that does not have a usable
+ * Network, in which case {@link #getNetwork()} returns null.
+ * @return The interface index as per {@link java.net.NetworkInterface#getIndex}, or 0 if unset.
+ * @hide
+ */
+ public int getInterfaceIndex() {
+ return mInterfaceIndex;
+ }
+
+ /**
+ * Set the index of the network interface where the service was found.
+ * @hide
+ */
+ public void setInterfaceIndex(int interfaceIndex) {
+ mInterfaceIndex = interfaceIndex;
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
@@ -375,6 +400,7 @@
}
dest.writeParcelable(mNetwork, 0);
+ dest.writeInt(mInterfaceIndex);
}
/** Implement the Parcelable interface */
@@ -405,6 +431,7 @@
info.mTxtRecord.put(in.readString(), valueArray);
}
info.mNetwork = in.readParcelable(null, Network.class);
+ info.mInterfaceIndex = in.readInt();
return info;
}
diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java
index 6def44f..95e6114 100644
--- a/service-t/src/com/android/server/NsdService.java
+++ b/service-t/src/com/android/server/NsdService.java
@@ -23,6 +23,7 @@
import android.content.Intent;
import android.content.pm.PackageManager;
import android.net.ConnectivityManager;
+import android.net.INetd;
import android.net.LinkProperties;
import android.net.Network;
import android.net.mdns.aidl.DiscoveryInfo;
@@ -466,7 +467,7 @@
// interfaces that do not have an associated Network.
break;
}
- servInfo.setNetwork(new Network(foundNetId));
+ setServiceNetworkForCallback(servInfo, info.netId, info.interfaceIdx);
clientInfo.onServiceFound(clientId, servInfo);
break;
}
@@ -476,10 +477,11 @@
final String type = info.registrationType;
final int lostNetId = info.netId;
servInfo = new NsdServiceInfo(name, type);
- // The network could be null if it was torn down when the service is lost
- // TODO: avoid returning null in that case, possibly by remembering found
- // services on the same interface index and their network at the time
- servInfo.setNetwork(lostNetId == 0 ? null : new Network(lostNetId));
+ // The network could be set to null (netId 0) if it was torn down when the
+ // service is lost
+ // TODO: avoid returning null in that case, possibly by remembering
+ // found services on the same interface index and their network at the time
+ setServiceNetworkForCallback(servInfo, lostNetId, info.interfaceIdx);
clientInfo.onServiceLost(clientId, servInfo);
break;
}
@@ -557,7 +559,6 @@
final GetAddressInfo info = (GetAddressInfo) obj;
final String address = info.address;
final int netId = info.netId;
- final Network network = netId == NETID_UNSET ? null : new Network(netId);
InetAddress serviceHost = null;
try {
serviceHost = InetAddress.getByName(address);
@@ -568,9 +569,10 @@
// If the resolved service is on an interface without a network, consider it
// as a failure: it would not be usable by apps as they would need
// privileged permissions.
- if (network != null && serviceHost != null) {
+ if (netId != NETID_UNSET && serviceHost != null) {
clientInfo.mResolvedService.setHost(serviceHost);
- clientInfo.mResolvedService.setNetwork(network);
+ setServiceNetworkForCallback(clientInfo.mResolvedService,
+ netId, info.interfaceIdx);
clientInfo.onResolveServiceSucceeded(
clientId, clientInfo.mResolvedService);
} else {
@@ -590,6 +592,26 @@
}
}
+ private static void setServiceNetworkForCallback(NsdServiceInfo info, int netId, int ifaceIdx) {
+ switch (netId) {
+ case NETID_UNSET:
+ info.setNetwork(null);
+ break;
+ case INetd.LOCAL_NET_ID:
+ // Special case for LOCAL_NET_ID: Networks on netId 99 are not generally
+ // visible / usable for apps, so do not return it. Store the interface
+ // index instead, so at least if the client tries to resolve the service
+ // with that NsdServiceInfo, it will be done on the same interface.
+ // If they recreate the NsdServiceInfo themselves, resolution would be
+ // done on all interfaces as before T, which should also work.
+ info.setNetwork(null);
+ info.setInterfaceIndex(ifaceIdx);
+ break;
+ default:
+ info.setNetwork(new Network(netId));
+ }
+ }
+
// The full service name is escaped from standard DNS rules on mdnsresponder, making it suitable
// for passing to standard system DNS APIs such as res_query() . Thus, make the service name
// unescape for getting right service address. See "Notes on DNS Name Escaping" on
@@ -767,9 +789,8 @@
String type = service.getServiceType();
int port = service.getPort();
byte[] textRecord = service.getTxtRecord();
- final Network network = service.getNetwork();
- final int registerInterface = getNetworkInterfaceIndex(network);
- if (network != null && registerInterface == IFACE_IDX_ANY) {
+ final int registerInterface = getNetworkInterfaceIndex(service);
+ if (service.getNetwork() != null && registerInterface == IFACE_IDX_ANY) {
Log.e(TAG, "Interface to register service on not found");
return false;
}
@@ -781,10 +802,9 @@
}
private boolean discoverServices(int discoveryId, NsdServiceInfo serviceInfo) {
- final Network network = serviceInfo.getNetwork();
final String type = serviceInfo.getServiceType();
- final int discoverInterface = getNetworkInterfaceIndex(network);
- if (network != null && discoverInterface == IFACE_IDX_ANY) {
+ final int discoverInterface = getNetworkInterfaceIndex(serviceInfo);
+ if (serviceInfo.getNetwork() != null && discoverInterface == IFACE_IDX_ANY) {
Log.e(TAG, "Interface to discover service on not found");
return false;
}
@@ -798,9 +818,8 @@
private boolean resolveService(int resolveId, NsdServiceInfo service) {
final String name = service.getServiceName();
final String type = service.getServiceType();
- final Network network = service.getNetwork();
- final int resolveInterface = getNetworkInterfaceIndex(network);
- if (network != null && resolveInterface == IFACE_IDX_ANY) {
+ final int resolveInterface = getNetworkInterfaceIndex(service);
+ if (service.getNetwork() != null && resolveInterface == IFACE_IDX_ANY) {
Log.e(TAG, "Interface to resolve service on not found");
return false;
}
@@ -816,8 +835,17 @@
* this is to support the legacy mdnsresponder implementation, which historically resolved
* services on an unspecified network.
*/
- private int getNetworkInterfaceIndex(Network network) {
- if (network == null) return IFACE_IDX_ANY;
+ private int getNetworkInterfaceIndex(NsdServiceInfo serviceInfo) {
+ final Network network = serviceInfo.getNetwork();
+ if (network == null) {
+ // Fallback to getInterfaceIndex if present (typically if the NsdServiceInfo was
+ // provided by NsdService from discovery results, and the service was found on an
+ // interface that has no app-usable Network).
+ if (serviceInfo.getInterfaceIndex() != 0) {
+ return serviceInfo.getInterfaceIndex();
+ }
+ return IFACE_IDX_ANY;
+ }
final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class);
if (cm == null) {
diff --git a/tests/common/Android.bp b/tests/common/Android.bp
index 509e881..58731e0 100644
--- a/tests/common/Android.bp
+++ b/tests/common/Android.bp
@@ -140,6 +140,30 @@
],
}
+// defaults for tests that need to build against framework-connectivity's @hide APIs, but also
+// using fully @hide classes that are jarjared (because they have no API member). Similar to
+// framework-connectivity-test-defaults above but uses pre-jarjar class names.
+// Only usable from targets that have visibility on framework-connectivity-pre-jarjar, and apply
+// connectivity jarjar rules so that references to jarjared classes still match: this is limited to
+// connectivity internal tests only.
+java_defaults {
+ name: "framework-connectivity-internal-test-defaults",
+ sdk_version: "core_platform", // tests can use @CorePlatformApi's
+ libs: [
+ // order matters: classes in framework-connectivity are resolved before framework,
+ // meaning @hide APIs in framework-connectivity are resolved before @SystemApi
+ // stubs in framework
+ "framework-connectivity-pre-jarjar",
+ "framework-connectivity-t-pre-jarjar",
+ "framework-tethering.impl",
+ "framework",
+
+ // if sdk_version="" this gets automatically included, but here we need to add manually.
+ "framework-res",
+ ],
+ defaults_visibility: ["//packages/modules/Connectivity/tests:__subpackages__"],
+}
+
// Defaults for tests that want to run in mainline-presubmit.
// Not widely used because many of our tests have AndroidTest.xml files and
// use the mainline-param config-descriptor metadata in AndroidTest.xml.
diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp
index c9a41ba..18ace4e 100644
--- a/tests/unit/Android.bp
+++ b/tests/unit/Android.bp
@@ -112,7 +112,7 @@
name: "FrameworksNetTestsDefaults",
min_sdk_version: "30",
defaults: [
- "framework-connectivity-test-defaults",
+ "framework-connectivity-internal-test-defaults",
],
srcs: [
"java/**/*.java",
diff --git a/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java b/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java
index e5e7ebc..892e140 100644
--- a/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java
+++ b/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java
@@ -125,6 +125,7 @@
fullInfo.setPort(4242);
fullInfo.setHost(LOCALHOST);
fullInfo.setNetwork(new Network(123));
+ fullInfo.setInterfaceIndex(456);
checkParcelable(fullInfo);
NsdServiceInfo noHostInfo = new NsdServiceInfo();
@@ -175,6 +176,7 @@
assertEquals(original.getHost(), result.getHost());
assertTrue(original.getPort() == result.getPort());
assertEquals(original.getNetwork(), result.getNetwork());
+ assertEquals(original.getInterfaceIndex(), result.getInterfaceIndex());
// Assert equality of attribute map.
Map<String, byte[]> originalMap = original.getAttributes();
diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java
index 3c228d0..ed9e930 100644
--- a/tests/unit/java/com/android/server/NsdServiceTest.java
+++ b/tests/unit/java/com/android/server/NsdServiceTest.java
@@ -19,7 +19,9 @@
import static libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges;
import static libcore.junit.util.compat.CoreCompatChangeRule.EnableCompatChanges;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.any;
@@ -36,6 +38,12 @@
import android.compat.testing.PlatformCompatChangeRule;
import android.content.ContentResolver;
import android.content.Context;
+import android.net.INetd;
+import android.net.InetAddresses;
+import android.net.mdns.aidl.DiscoveryInfo;
+import android.net.mdns.aidl.GetAddressInfo;
+import android.net.mdns.aidl.IMDnsEventListener;
+import android.net.mdns.aidl.ResolutionInfo;
import android.net.nsd.INsdManagerCallback;
import android.net.nsd.INsdServiceConnector;
import android.net.nsd.MDnsManager;
@@ -63,6 +71,7 @@
import org.junit.rules.TestRule;
import org.junit.runner.RunWith;
import org.mockito.AdditionalAnswers;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -275,6 +284,105 @@
verify(mMockMDnsM, never()).stopDaemon();
}
+ @Test
+ public void testDiscoverOnTetheringDownstream() throws Exception {
+ NsdService service = makeService();
+ NsdManager client = connectClient(service);
+
+ final String serviceType = "a_type";
+ final String serviceName = "a_name";
+ final String domainName = "mytestdevice.local";
+ final int interfaceIdx = 123;
+ final NsdManager.DiscoveryListener discListener = mock(NsdManager.DiscoveryListener.class);
+ client.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discListener);
+ waitForIdle();
+
+ final ArgumentCaptor<IMDnsEventListener> listenerCaptor =
+ ArgumentCaptor.forClass(IMDnsEventListener.class);
+ verify(mMockMDnsM).registerEventListener(listenerCaptor.capture());
+ final ArgumentCaptor<Integer> discIdCaptor = ArgumentCaptor.forClass(Integer.class);
+ verify(mMockMDnsM).discover(discIdCaptor.capture(), eq(serviceType),
+ eq(0) /* interfaceIdx */);
+ // NsdManager uses a separate HandlerThread to dispatch callbacks (on ServiceHandler), so
+ // this needs to use a timeout
+ verify(discListener, timeout(TIMEOUT_MS)).onDiscoveryStarted(serviceType);
+
+ final DiscoveryInfo discoveryInfo = new DiscoveryInfo(
+ discIdCaptor.getValue(),
+ IMDnsEventListener.SERVICE_FOUND,
+ serviceName,
+ serviceType,
+ domainName,
+ interfaceIdx,
+ INetd.LOCAL_NET_ID); // LOCAL_NET_ID (99) used on tethering downstreams
+ final IMDnsEventListener eventListener = listenerCaptor.getValue();
+ eventListener.onServiceDiscoveryStatus(discoveryInfo);
+ waitForIdle();
+
+ final ArgumentCaptor<NsdServiceInfo> discoveredInfoCaptor =
+ ArgumentCaptor.forClass(NsdServiceInfo.class);
+ verify(discListener, timeout(TIMEOUT_MS)).onServiceFound(discoveredInfoCaptor.capture());
+ final NsdServiceInfo foundInfo = discoveredInfoCaptor.getValue();
+ assertEquals(serviceName, foundInfo.getServiceName());
+ assertEquals(serviceType, foundInfo.getServiceType());
+ assertNull(foundInfo.getHost());
+ assertNull(foundInfo.getNetwork());
+ assertEquals(interfaceIdx, foundInfo.getInterfaceIndex());
+
+ // After discovering the service, verify resolving it
+ final NsdManager.ResolveListener resolveListener = mock(NsdManager.ResolveListener.class);
+ client.resolveService(foundInfo, resolveListener);
+ waitForIdle();
+
+ final ArgumentCaptor<Integer> resolvIdCaptor = ArgumentCaptor.forClass(Integer.class);
+ verify(mMockMDnsM).resolve(resolvIdCaptor.capture(), eq(serviceName), eq(serviceType),
+ eq("local.") /* domain */, eq(interfaceIdx));
+
+ final int servicePort = 10123;
+ final String serviceFullName = serviceName + "." + serviceType;
+ final ResolutionInfo resolutionInfo = new ResolutionInfo(
+ resolvIdCaptor.getValue(),
+ IMDnsEventListener.SERVICE_RESOLVED,
+ null /* serviceName */,
+ null /* serviceType */,
+ null /* domain */,
+ serviceFullName,
+ domainName,
+ servicePort,
+ new byte[0] /* txtRecord */,
+ interfaceIdx);
+
+ doReturn(true).when(mMockMDnsM).getServiceAddress(anyInt(), any(), anyInt());
+ eventListener.onServiceResolutionStatus(resolutionInfo);
+ waitForIdle();
+
+ final ArgumentCaptor<Integer> getAddrIdCaptor = ArgumentCaptor.forClass(Integer.class);
+ verify(mMockMDnsM).getServiceAddress(getAddrIdCaptor.capture(), eq(domainName),
+ eq(interfaceIdx));
+
+ final String serviceAddress = "192.0.2.123";
+ final GetAddressInfo addressInfo = new GetAddressInfo(
+ getAddrIdCaptor.getValue(),
+ IMDnsEventListener.SERVICE_GET_ADDR_SUCCESS,
+ serviceFullName,
+ serviceAddress,
+ interfaceIdx,
+ INetd.LOCAL_NET_ID);
+ eventListener.onGettingServiceAddressStatus(addressInfo);
+ waitForIdle();
+
+ final ArgumentCaptor<NsdServiceInfo> resInfoCaptor =
+ ArgumentCaptor.forClass(NsdServiceInfo.class);
+ verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(resInfoCaptor.capture());
+ final NsdServiceInfo resolvedService = resInfoCaptor.getValue();
+ assertEquals(serviceName, resolvedService.getServiceName());
+ assertEquals("." + serviceType, resolvedService.getServiceType());
+ assertEquals(InetAddresses.parseNumericAddress(serviceAddress), resolvedService.getHost());
+ assertEquals(servicePort, resolvedService.getPort());
+ assertNull(resolvedService.getNetwork());
+ assertEquals(interfaceIdx, resolvedService.getInterfaceIndex());
+ }
+
private void waitForIdle() {
HandlerUtils.waitForIdle(mHandler, TIMEOUT_MS);
}