[mdns] Skip conflict check for incoming mDNS answer records which are
probed in the repository
There's a scenario where an undesired conflict happens:
1. Register a service and custom host with addresses and public key.
2. Register another service using the same hostname and public key.
While the second registartion is being probed, its KEY RR is considered
as conflicting with the address record of the first registraiton. This
shouldn't be a reasonable behavior because the address record itself is
from the repository itself.
Bug: 336936158
Bug: 336967571
Test: atest CtsNetTestCases:NsdManagerTest FrameworksNetTests:android.net.connectivity.com.android.server.connectivity.mdns.MdnsRecordRepositoryTest
Change-Id: Ibe6c8dd845540eba8f08152be6693be1dbc42ce9
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
index 39e8bcc..36f3982 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
@@ -34,6 +34,7 @@
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.SparseArray;
+import android.util.SparseIntArray;
import com.android.internal.annotations.VisibleForTesting;
import com.android.net.module.util.CollectionUtils;
@@ -87,6 +88,12 @@
private static final String[] DNS_SD_SERVICE_TYPE =
new String[] { "_services", "_dns-sd", "_udp", LOCAL_TLD };
+ private enum RecordConflictType {
+ NO_CONFLICT,
+ CONFLICT,
+ IDENTICAL
+ }
+
@NonNull
private final Random mDelayGenerator = new Random();
// Map of service unique ID -> records for service
@@ -1172,38 +1179,49 @@
* {@link MdnsInterfaceAdvertiser#CONFLICT_HOST}.
*/
public Map<Integer, Integer> getConflictingServices(MdnsPacket packet) {
- // Avoid allocating a new set for each incoming packet: use an empty set by default.
- Map<Integer, Integer> conflicting = Collections.emptyMap();
+ Map<Integer, Integer> conflicting = new ArrayMap<>();
for (MdnsRecord record : packet.answers) {
+ SparseIntArray conflictingWithRecord = new SparseIntArray();
for (int i = 0; i < mServices.size(); i++) {
final ServiceRegistration registration = mServices.valueAt(i);
if (registration.exiting) continue;
- int conflictType = 0;
+ final RecordConflictType conflictForService =
+ conflictForService(record, registration);
+ final RecordConflictType conflictForHost = conflictForHost(record, registration);
- if (conflictForService(record, registration)) {
- conflictType |= CONFLICT_SERVICE;
+ // Identical record is found in the repository so there won't be a conflict.
+ if (conflictForService == RecordConflictType.IDENTICAL
+ || conflictForHost == RecordConflictType.IDENTICAL) {
+ conflictingWithRecord.clear();
+ break;
}
- if (conflictForHost(record, registration)) {
+ int conflictType = 0;
+ if (conflictForService == RecordConflictType.CONFLICT) {
+ conflictType |= CONFLICT_SERVICE;
+ }
+ if (conflictForHost == RecordConflictType.CONFLICT) {
conflictType |= CONFLICT_HOST;
}
if (conflictType != 0) {
- if (conflicting.isEmpty()) {
- // Conflict was found: use a mutable set
- conflicting = new ArrayMap<>();
- }
final int serviceId = mServices.keyAt(i);
- conflicting.put(serviceId, conflictType);
+ conflictingWithRecord.put(serviceId, conflictType);
}
}
+ for (int i = 0; i < conflictingWithRecord.size(); i++) {
+ final int serviceId = conflictingWithRecord.keyAt(i);
+ final int conflictType = conflictingWithRecord.valueAt(i);
+ final int oldConflictType = conflicting.getOrDefault(serviceId, 0);
+ conflicting.put(serviceId, oldConflictType | conflictType);
+ }
}
return conflicting;
}
- private static boolean conflictForService(
+ private static RecordConflictType conflictForService(
@NonNull MdnsRecord record, @NonNull ServiceRegistration registration) {
String[] fullServiceName;
if (registration.srvRecord != null) {
@@ -1211,75 +1229,75 @@
} else if (registration.serviceKeyRecord != null) {
fullServiceName = registration.serviceKeyRecord.record.getName();
} else {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
if (!MdnsUtils.equalsDnsLabelIgnoreDnsCase(record.getName(), fullServiceName)) {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
// As per RFC6762 9., it's fine if the "conflict" is an identical record with same
// data.
if (record instanceof MdnsServiceRecord && equals(record, registration.srvRecord)) {
- return false;
+ return RecordConflictType.IDENTICAL;
}
if (record instanceof MdnsTextRecord && equals(record, registration.txtRecord)) {
- return false;
+ return RecordConflictType.IDENTICAL;
}
if (record instanceof MdnsKeyRecord && equals(record, registration.serviceKeyRecord)) {
- return false;
+ return RecordConflictType.IDENTICAL;
}
- return true;
+ return RecordConflictType.CONFLICT;
}
- private boolean conflictForHost(
+ private RecordConflictType conflictForHost(
@NonNull MdnsRecord record, @NonNull ServiceRegistration registration) {
// Only custom hosts are checked. When using the default host, the hostname is derived from
// a UUID and it's supposed to be unique.
if (registration.serviceInfo.getHostname() == null) {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
- // It cannot be a hostname conflict because not record is registered with the hostname.
+ // It cannot be a hostname conflict because no record is registered with the hostname.
if (registration.addressRecords.isEmpty() && registration.hostKeyRecord == null) {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
// The record's name cannot be registered by NsdManager so it's not a conflict.
if (record.getName().length != 2 || !record.getName()[1].equals(LOCAL_TLD)) {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
// Different names. There won't be a conflict.
if (!MdnsUtils.equalsIgnoreDnsCase(
record.getName()[0], registration.serviceInfo.getHostname())) {
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
// As per RFC6762 9., it's fine if the "conflict" is an identical record with same
// data.
if (record instanceof MdnsInetAddressRecord
&& hasInetAddressRecord(registration, (MdnsInetAddressRecord) record)) {
- return false;
+ return RecordConflictType.IDENTICAL;
}
if (record instanceof MdnsKeyRecord && equals(record, registration.hostKeyRecord)) {
- return false;
+ return RecordConflictType.IDENTICAL;
}
// Per RFC 6762 8.1, when a record is being probed, any answer containing a record with that
// name, of any type, MUST be considered a conflicting response.
if (registration.isProbing) {
- return true;
+ return RecordConflictType.CONFLICT;
}
if (record instanceof MdnsInetAddressRecord && !registration.addressRecords.isEmpty()) {
- return true;
+ return RecordConflictType.CONFLICT;
}
if (record instanceof MdnsKeyRecord && registration.hostKeyRecord != null) {
- return true;
+ return RecordConflictType.CONFLICT;
}
- return false;
+ return RecordConflictType.NO_CONFLICT;
}
private List<RecordInfo<MdnsInetAddressRecord>> getInetAddressRecordsForHostname(
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
index d735dc6..2cb97c9 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
@@ -1625,6 +1625,44 @@
}
@Test
+ fun testGetConflictingServices_multipleRegistrationsForHostKey_noConflict() {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
+
+ repository.addServiceAndFinishProbing(TEST_SERVICE_ID_1, NsdServiceInfo().apply {
+ hostname = "MyHost"
+ hostAddresses = listOf(
+ parseNumericAddress("2001:db8::1"),
+ parseNumericAddress("2001:db8::2"))
+ publicKey = TEST_PUBLIC_KEY
+ })
+ repository.addService(TEST_SERVICE_ID_2, NsdServiceInfo().apply {
+ serviceType = "_testservice._tcp"
+ serviceName = "MyTestService"
+ port = TEST_PORT
+ hostname = "MyHost"
+ publicKey = TEST_PUBLIC_KEY
+ }, null /* ttl */)
+
+ // Although there's a KEY RR in the second registration being probed, it shouldn't conflict
+ // with an address record which is from a probed registration in the repository.
+ val otherTtlMillis = 1234L
+ val packet = MdnsPacket(
+ 0 /* flags */,
+ emptyList() /* questions */,
+ listOf(
+ MdnsInetAddressRecord(
+ arrayOf("MyHost", "local"),
+ 0L /* receiptTimeMillis */, true /* cacheFlush */,
+ otherTtlMillis,
+ parseNumericAddress("2001:db8::1"))
+ ) /* answers */,
+ emptyList() /* authorityRecords */,
+ emptyList() /* additionalRecords */)
+
+ assertEquals(mapOf(), repository.getConflictingServices(packet))
+ }
+
+ @Test
fun testGetConflictingServices_IdenticalService() {
val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* ttl */)