8030731: Improve name service robustness

Reviewed-by: weijun, michaelm, skoivu

jdk/src/share/classes/com/sun/jndi/dns/DnsClient.java

@@ -30,13 +30,14 @@ import java.net.DatagramSocket;
 import java.net.DatagramPacket;
 import java.net.InetAddress;
 import java.net.Socket;
+import java.security.SecureRandom;
 import javax.naming.*;
 
 import java.util.Collections;
 import java.util.Map;
 import java.util.HashMap;
-import java.util.Set;
-import java.util.HashSet;
+
+import sun.security.jca.JCAUtil;
 
 // Some of this code began life as part of sun.javaos.net.DnsClient
 // originally by sritchie@eng 1/96.  It was first hacked up for JNDI
@@ -77,6 +78,8 @@ public class DnsClient {
     };
 
     private static final int DEFAULT_PORT = 53;
+    private static final int TRANSACTION_ID_BOUND = 0x10000;
+    private static final SecureRandom random = JCAUtil.getSecureRandom();
     private InetAddress[] servers;
     private int[] serverPorts;
     private int timeout;                // initial timeout on UDP queries in ms
@@ -85,7 +88,7 @@ public class DnsClient {
     private DatagramSocket udpSocket;
 
     // Requests sent
-    private Set<Integer> reqs;
+    private Map<Integer, ResourceRecord> reqs;
 
     // Responses received
     private Map<Integer, byte[]> resps;
@@ -134,7 +137,8 @@ public class DnsClient {
                 throw ne;
             }
         }
-        reqs = Collections.synchronizedSet(new HashSet<Integer>());
+        reqs = Collections.synchronizedMap(
+            new HashMap<Integer, ResourceRecord>());
         resps = Collections.synchronizedMap(new HashMap<Integer, byte[]>());
     }
 
@@ -153,10 +157,6 @@ public class DnsClient {
         }
     }
 
-
-    private int ident = 0;              // used to set the msg ID field
-    private Object identLock = new Object();
-
     /*
      * If recursion is true, recursion is requested on the query.
      * If auth is true, only authoritative responses are accepted; other
@@ -167,15 +167,19 @@ public class DnsClient {
             throws NamingException {
 
         int xid;
-        synchronized (identLock) {
-            ident = 0xFFFF & (ident + 1);
-            xid = ident;
-        }
+        Packet pkt;
+        ResourceRecord collision;
 
-        // enqueue the outstanding request
-        reqs.add(xid);
+        do {
+            // Generate a random transaction ID
+            xid = random.nextInt(TRANSACTION_ID_BOUND);
+            pkt = makeQueryPacket(fqdn, xid, qclass, qtype, recursion);
 
-        Packet pkt = makeQueryPacket(fqdn, xid, qclass, qtype, recursion);
+            // enqueue the outstanding request
+            collision = reqs.putIfAbsent(xid, new ResourceRecord(pkt.getData(),
+                pkt.length(), Header.HEADER_SIZE, true, false));
+
+        } while (collision != null);
 
         Exception caughtException = null;
         boolean[] doNotRetry = new boolean[servers.length];
@@ -305,11 +309,8 @@ public class DnsClient {
     ResourceRecords queryZone(DnsName zone, int qclass, boolean recursion)
             throws NamingException {
 
-        int xid;
-        synchronized (identLock) {
-            ident = 0xFFFF & (ident + 1);
-            xid = ident;
-        }
+        int xid = random.nextInt(TRANSACTION_ID_BOUND);
+
         Packet pkt = makeQueryPacket(zone, xid, qclass,
                                      ResourceRecord.QTYPE_AXFR, recursion);
         Exception caughtException = null;
@@ -390,6 +391,7 @@ public class DnsClient {
             DatagramPacket opkt = new DatagramPacket(
                     pkt.getData(), pkt.length(), server, port);
             DatagramPacket ipkt = new DatagramPacket(new byte[8000], 8000);
+            // Packets may only be sent to or received from this server address
             udpSocket.connect(server, port);
             int pktTimeout = (timeout * (1 << retry));
             try {
@@ -542,6 +544,9 @@ public class DnsClient {
      * Checks the header of an incoming DNS response.
      * Returns true if it matches the given xid and throws a naming
      * exception, if appropriate, based on the response code.
+     *
+     * Also checks that the domain name, type and class in the response
+     * match those in the original query.
      */
     private boolean isMatchResponse(byte[] pkt, int xid)
                 throws NamingException {
@@ -551,7 +556,7 @@ public class DnsClient {
             throw new CommunicationException("DNS error: expecting response");
         }
 
-        if (!reqs.contains(xid)) { // already received, ignore the response
+        if (!reqs.containsKey(xid)) { // already received, ignore the response
             return false;
         }
 
@@ -560,14 +565,47 @@ public class DnsClient {
             if (debug) {
                 dprint("XID MATCH:" + xid);
             }
-
             checkResponseCode(hdr);
-            // remove the response for the xid if received by some other thread.
-            synchronized (queuesLock) {
-                resps.remove(xid);
-                reqs.remove(xid);
+            if (!hdr.query && hdr.numQuestions == 1) {
+
+                ResourceRecord rr = new ResourceRecord(pkt, pkt.length,
+                    Header.HEADER_SIZE, true, false);
+
+                // Retrieve the original query
+                ResourceRecord query = reqs.get(xid);
+                int qtype = query.getType();
+                int qclass = query.getRrclass();
+                DnsName qname = query.getName();
+
+                // Check that the type/class/name in the query section of the
+                // response match those in the original query
+                if ((qtype == ResourceRecord.QTYPE_STAR ||
+                    qtype == rr.getType()) &&
+                    (qclass == ResourceRecord.QCLASS_STAR ||
+                    qclass == rr.getRrclass()) &&
+                    qname.equals(rr.getName())) {
+
+                    if (debug) {
+                        dprint("MATCH NAME:" + qname + " QTYPE:" + qtype +
+                            " QCLASS:" + qclass);
+                    }
+
+                    // Remove the response for the xid if received by some other
+                    // thread.
+                    synchronized (queuesLock) {
+                        resps.remove(xid);
+                        reqs.remove(xid);
+                    }
+                    return true;
+
+                } else {
+                    if (debug) {
+                        dprint("NO-MATCH NAME:" + qname + " QTYPE:" + qtype +
+                            " QCLASS:" + qclass);
+                    }
+                }
             }
-            return true;
+            return false;
         }
 
         //
@@ -576,7 +614,7 @@ public class DnsClient {
         // enqueue only the first response, responses for retries are ignored.
         //
         synchronized (queuesLock) {
-            if (reqs.contains(hdr.xid)) { // enqueue only the first response
+            if (reqs.containsKey(hdr.xid)) { // enqueue only the first response
                 resps.put(hdr.xid, pkt);
             }
         }