Add DatagramTransportExecutor, validate incoming DNS response messages to avoid cache poisoning attacks and deprecate legacy Executor

README.md

@@ -14,6 +14,7 @@ easily be used to create a DNS server.
 * [Caching](#caching)
   * [Custom cache adapter](#custom-cache-adapter)
 * [Advanced usage](#advanced-usage)
+  * [DatagramTransportExecutor](#datagramtransportexecutor)
   * [HostsFileExecutor](#hostsfileexecutor)
 * [Install](#install)
 * [Tests](#tests)
@@ -117,13 +118,20 @@ See also the wiki for possible [cache implementations](https://github.com/reactp
 
 ## Advanced Usage
 
-For more advanced usages one can utilize the `React\Dns\Query\Executor` directly.
+### DatagramTransportExecutor
+
+The `DatagramTransportExecutor` can be used to
+send DNS queries over a datagram transport such as UDP.
+
+This is the main class that sends a DNS query to your DNS server and is used
+internally by the `Resolver` for the actual message transport.
+
+For more advanced usages one can utilize this class directly.
 The following example looks up the `IPv6` address for `igor.io`.
 
 ```php
 $loop = Factory::create();
-
-$executor = new Executor($loop, new Parser(), new BinaryDumper(), null);
+$executor = new DatagramTransportExecutor($loop);
 
 $executor->query(
     '8.8.8.8:53', 
@@ -135,11 +143,41 @@ $executor->query(
 }, 'printf');
 
 $loop->run();
-
 ```
 
 See also the [fourth example](examples).
 
+Note that this executor does not implement a timeout, so you will very likely
+want to use this in combination with a `TimeoutExecutor` like this:
+
+```php
+$executor = new TimeoutExecutor(
+    new DatagramTransportExecutor($loop),
+    3.0,
+    $loop
+);
+```
+
+Also note that this executor uses an unreliable UDP transport and that it
+does not implement any retry logic, so you will likely want to use this in
+combination with a `RetryExecutor` like this:
+
+```php
+$executor = new RetryExecutor(
+    new TimeoutExecutor(
+        new DatagramTransportExecutor($loop),
+        3.0,
+        $loop
+    )
+);
+```
+
+> Internally, this class uses PHP's UDP sockets and does not take advantage
+  of [react/datagram](https://github.com/reactphp/datagram) purely for
+  organizational reasons to avoid a cyclic dependency between the two
+  packages. Higher-level components should take advantage of the Datagram
+  component instead of reimplementing this socket logic from scratch.
+
 ### HostsFileExecutor
 
 Note that the above `Executor` class always performs an actual DNS query.

examples/04-query-a-and-aaaa.php

@@ -1,17 +1,14 @@
 <?php
 
 use React\Dns\Model\Message;
-use React\Dns\Protocol\BinaryDumper;
-use React\Dns\Protocol\Parser;
-use React\Dns\Query\Executor;
+use React\Dns\Query\DatagramTransportExecutor;
 use React\Dns\Query\Query;
 use React\EventLoop\Factory;
 
 require __DIR__ . '/../vendor/autoload.php';
 
 $loop = Factory::create();
-
-$executor = new Executor($loop, new Parser(), new BinaryDumper(), null);
+$executor = new DatagramTransportExecutor($loop);
 
 $name = isset($argv[1]) ? $argv[1] : 'www.google.com';
 

src/Model/Message.php

@@ -3,7 +3,6 @@
 namespace React\Dns\Model;
 
 use React\Dns\Query\Query;
-use React\Dns\Model\Record;
 
 class Message
 {
@@ -73,8 +72,29 @@ public static function createResponseWithAnswersForQuery(Query $query, array $an
         return $response;
     }
 
+    /**
+     * generates a random 16 bit message ID
+     *
+     * This uses a CSPRNG so that an outside attacker that is sending spoofed
+     * DNS response messages can not guess the message ID to avoid possible
+     * cache poisoning attacks.
+     *
+     * The `random_int()` function is only available on PHP 7+ or when
+     * https://github.com/paragonie/random_compat is installed. As such, using
+     * the latest supported PHP version is highly recommended. This currently
+     * falls back to a less secure random number generator on older PHP versions
+     * in the hope that this system is properly protected against outside
+     * attackers, for example by using one of the common local DNS proxy stubs.
+     *
+     * @return int
+     * @see self::getId()
+     * @codeCoverageIgnore
+     */
     private static function generateId()
     {
+        if (function_exists('random_int')) {
+            return random_int(0, 0xffff);
+        }
         return mt_rand(0, 0xffff);
     }
 
@@ -99,6 +119,22 @@ public function __construct()
         $this->header = new HeaderBag();
     }
 
+    /**
+     * Returns the 16 bit message ID
+     *
+     * The response message ID has to match the request message ID. This allows
+     * the receiver to verify this is the correct response message. An outside
+     * attacker may try to inject fake responses by "guessing" the message ID,
+     * so this should use a proper CSPRNG to avoid possible cache poisoning.
+     *
+     * @return int
+     * @see self::generateId()
+     */
+    public function getId()
+    {
+        return $this->header->get('id');
+    }
+
     public function prepare()
     {
         $this->header->populateCounts($this);

src/Query/DatagramTransportExecutor.php

@@ -0,0 +1,161 @@
+<?php
+
+namespace React\Dns\Query;
+
+use React\Dns\Model\Message;
+use React\Dns\Protocol\BinaryDumper;
+use React\Dns\Protocol\Parser;
+use React\EventLoop\LoopInterface;
+use React\Promise\Deferred;
+
+/**
+ * Send DNS queries over a datagram transport such as UDP.
+ *
+ * This is the main class that sends a DNS query to your DNS server and is used
+ * internally by the `Resolver` for the actual message transport.
+ *
+ * For more advanced usages one can utilize this class directly.
+ * The following example looks up the `IPv6` address for `igor.io`.
+ *
+ * ```php
+ * $loop = Factory::create();
+ * $executor = new DatagramTransportExecutor($loop);
+ *
+ * $executor->query(
+ *     '8.8.8.8:53',
+ *     new Query($name, Message::TYPE_AAAA, Message::CLASS_IN, time())
+ * )->then(function (Message $message) {
+ *     foreach ($message->answers as $answer) {
+ *         echo 'IPv6: ' . $answer->data . PHP_EOL;
+ *     }
+ * }, 'printf');
+ *
+ * $loop->run();
+ * ```
+ *
+ * See also the [fourth example](examples).
+ *
+ * Note that this executor does not implement a timeout, so you will very likely
+ * want to use this in combination with a `TimeoutExecutor` like this:
+ *
+ * ```php
+ * $executor = new TimeoutExecutor(
+ *     new DatagramTransportExecutor($loop),
+ *     3.0,
+ *     $loop
+ * );
+ * ```
+ *
+ * Also note that this executor uses an unreliable UDP transport and that it
+ * does not implement any retry logic, so you will likely want to use this in
+ * combination with a `RetryExecutor` like this:
+ *
+ * ```php
+ * $executor = new RetryExecutor(
+ *     new TimeoutExecutor(
+ *         new DatagramTransportExecutor($loop),
+ *         3.0,
+ *         $loop
+ *     )
+ * );
+ * ```
+ *
+ * > Internally, this class uses PHP's UDP sockets and does not take advantage
+ *   of [react/datagram](https://github.com/reactphp/datagram) purely for
+ *   organizational reasons to avoid a cyclic dependency between the two
+ *   packages. Higher-level components should take advantage of the Datagram
+ *   component instead of reimplementing this socket logic from scratch.
+ */
+class DatagramTransportExecutor implements ExecutorInterface
+{
+    private $loop;
+    private $parser;
+    private $dumper;
+
+    /**
+     * @param LoopInterface     $loop
+     * @param null|Parser       $parser optional/advanced: DNS protocol parser to use
+     * @param null|BinaryDumper $dumper optional/advanced: DNS protocol dumper to use
+     */
+    public function __construct(LoopInterface $loop, Parser $parser = null, BinaryDumper $dumper = null)
+    {
+        if ($parser === null) {
+            $parser = new Parser();
+        }
+        if ($dumper === null) {
+            $dumper = new BinaryDumper();
+        }
+
+        $this->loop = $loop;
+        $this->parser = $parser;
+        $this->dumper = $dumper;
+    }
+
+    public function query($nameserver, Query $query)
+    {
+        $request = Message::createRequestForQuery($query);
+
+        $queryData = $this->dumper->toBinary($request);
+        if (isset($queryData[512])) {
+            return \React\Promise\reject(new \RuntimeException(
+                'DNS query for ' . $query->name . ' failed: Query too large for UDP transport'
+            ));
+        }
+
+        // UDP connections are instant, so try connection without a loop or timeout
+        $socket = @\stream_socket_client("udp://$nameserver", $errno, $errstr, 0);
+        if ($socket === false) {
+            return \React\Promise\reject(new \RuntimeException(
+                'DNS query for ' . $query->name . ' failed: Unable to connect to DNS server ('  . $errstr . ')',
+                $errno
+            ));
+        }
+
+        // set socket to non-blocking and immediately try to send (fill write buffer)
+        \stream_set_blocking($socket, false);
+        \fwrite($socket, $queryData);
+
+        $loop = $this->loop;
+        $deferred = new Deferred(function () use ($loop, $socket, $query) {
+            // cancellation should remove socket from loop and close socket
+            $loop->removeReadStream($socket);
+            \fclose($socket);
+
+            throw new CancellationException('DNS query for ' . $query->name . ' has been cancelled');
+        });
+
+        $parser = $this->parser;
+        $loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request) {
+            // try to read a single data packet from the DNS server
+            // ignoring any errors, this is uses UDP packets and not a stream of data
+            $data = @\fread($socket, 512);
+
+            try {
+                $response = $parser->parseMessage($data);
+            } catch (\Exception $e) {
+                // ignore and await next if we received an invalid message from remote server
+                // this may as well be a fake response from an attacker (possible DOS)
+                return;
+            }
+
+            // ignore and await next if we received an unexpected response ID
+            // this may as well be a fake response from an attacker (possible cache poisoning)
+            if ($response->getId() !== $request->getId()) {
+                return;
+            }
+
+            // we only react to the first valid message, so remove socket from loop and close
+            $loop->removeReadStream($socket);
+            \fclose($socket);
+
+            if ($response->header->isTruncated()) {
+                $deferred->reject(new \RuntimeException('DNS query for ' . $query->name . ' failed: The server returned a truncated result for a UDP query, but retrying via TCP is currently not supported'));
+                return;
+            }
+
+            $deferred->resolve($response);
+        });
+
+        return $deferred->promise();
+    }
+}

src/Query/Executor.php

@@ -11,6 +11,10 @@
 use React\Stream\DuplexResourceStream;
 use React\Stream\Stream;
 
+/**
+ * @deprecated unused, exists for BC only
+ * @see DatagramTransportExecutor
+ */
 class Executor implements ExecutorInterface
 {
     private $loop;

src/Resolver/Factory.php

@@ -5,10 +5,8 @@
 use React\Cache\ArrayCache;
 use React\Cache\CacheInterface;
 use React\Dns\Config\HostsFile;
-use React\Dns\Protocol\Parser;
-use React\Dns\Protocol\BinaryDumper;
 use React\Dns\Query\CachedExecutor;
-use React\Dns\Query\Executor;
+use React\Dns\Query\DatagramTransportExecutor;
 use React\Dns\Query\ExecutorInterface;
 use React\Dns\Query\HostsFileExecutor;
 use React\Dns\Query\RecordCache;
@@ -71,7 +69,7 @@ private function decorateHostsFileExecutor(ExecutorInterface $executor)
     protected function createExecutor(LoopInterface $loop)
     {
         return new TimeoutExecutor(
-            new Executor($loop, new Parser(), new BinaryDumper(), null),
+            new DatagramTransportExecutor($loop),
             5.0,
             $loop
         );