Merge pull request #5 from noglitchyo/bugfix/std_client_fix

Fixed non working StdClient not able to fetch from UDP upstreams

composer.json

@@ -26,14 +26,16 @@
     "psr/log": "^1.1",
     "psr/http-server-middleware": "^1.0",
     "php-http/guzzle6-adapter": "^2.0",
-    "react/dns": "^0.4.17"
+    "react/dns": "^1.3.0"
   },
   "require-dev": {
+    "react/datagram": "^1.5",
     "phpunit/phpunit": "^8.1",
     "mockery/mockery": "^1.2",
     "monolog/monolog": "^1.24",
-    "squizlabs/php_codesniffer": "*",
-    "phpstan/phpstan": "^0.11.8"
+    "squizlabs/php_codesniffer": "3.*",
+    "phpstan/phpstan": "^0.11.8",
+    "symfony/process": "^5.1"
   },
   "autoload": {
     "psr-4": {
@@ -47,7 +49,8 @@
   },
   "scripts": {
     "phpstan": "phpstan analyse -l max src",
-    "phpcs": "phpcs --standard=PSR2 ./src/",
+    "phpcs": "phpcs --standard=PSR12 ./src/ ./tests",
+    "phpcbf": "phpcbf --standard=PSR12 ./src/ ./tests",
     "test": "phpunit phpunit.dist.xml"
   }
 }

examples/docker-firefox/docker/src/public/index.php

@@ -3,6 +3,8 @@
 use Http\Adapter\Guzzle6\Client;
 use NoGlitchYo\Dealdoh\Client\DohClient;
 use NoGlitchYo\Dealdoh\Client\StdClient;
+use NoGlitchYo\Dealdoh\Client\Transport\DnsOverTcpTransport;
+use NoGlitchYo\Dealdoh\Client\Transport\DnsOverUdpTransport;
 use NoGlitchYo\Dealdoh\DohProxy;
 use NoGlitchYo\Dealdoh\Entity\DnsUpstreamPool;
 use NoGlitchYo\Dealdoh\Factory\Dns\MessageFactory;
@@ -11,7 +13,6 @@
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Slim\App;
-use Socket\Raw\Factory;
 
 require __DIR__ . '/../vendor/autoload.php';
 
@@ -37,8 +38,9 @@ function (ServerRequestInterface $request, ResponseInterface $response, $args) {
                     $dnsMessageFactory
                 ),
                 new StdClient(
-                    new Factory(),
-                    $dnsMessageFactory
+                    $dnsMessageFactory,
+                    new DnsOverTcpTransport(),
+                    new DnsOverUdpTransport()
                 ),
             ]
         );

examples/slim-integration/public/index.php

@@ -3,13 +3,14 @@
 use Http\Adapter\Guzzle6\Client;
 use NoGlitchYo\Dealdoh\Client\DohClient;
 use NoGlitchYo\Dealdoh\Client\StdClient;
+use NoGlitchYo\Dealdoh\Client\Transport\DnsOverTcpTransport;
+use NoGlitchYo\Dealdoh\Client\Transport\DnsOverUdpTransport;
 use NoGlitchYo\Dealdoh\DohProxy;
 use NoGlitchYo\Dealdoh\Entity\DnsUpstreamPool;
 use NoGlitchYo\Dealdoh\Factory\Dns\MessageFactory;
 use NoGlitchYo\Dealdoh\Factory\DohHttpMessageFactory;
 use NoGlitchYo\Dealdoh\Service\DnsPoolResolver;
 use Slim\App;
-use Socket\Raw\Factory;
 
 require __DIR__ . '/../vendor/autoload.php';
 
@@ -34,8 +35,9 @@ function (ServerRequestInterface $request, ResponseInterface $response, $args) {
                     $dnsMessageFactory
                 ),
                 new StdClient(
-                    new Factory(),
-                    $dnsMessageFactory
+                    $dnsMessageFactory,
+                    new DnsOverTcpTransport(),
+                    new DnsOverUdpTransport()
                 ),
             ]
         );

src/Client/DnsClientInterface.php

@@ -1,4 +1,6 @@
-<?php declare(strict_types=1);
+<?php
+
+declare(strict_types=1);
 
 namespace NoGlitchYo\Dealdoh\Client;
 
@@ -7,10 +9,18 @@
 
 interface DnsClientInterface
 {
+    /**
+     * Resolve a DNS message using the provided upstream
+     *
+     * @param DnsUpstream      $dnsUpstream
+     * @param MessageInterface $dnsRequestMessage
+     *
+     * @return MessageInterface
+     */
     public function resolve(DnsUpstream $dnsUpstream, MessageInterface $dnsRequestMessage): MessageInterface;
 
     /**
-     * Indicate whether or not the given DNS upstream can be used by the client
+     * Indicate whether or not the provided $dnsUpstream can be used by the client
      *
      * @param  DnsUpstream $dnsUpstream
      * @return bool

src/Client/DohClient.php

@@ -1,4 +1,6 @@
-<?php declare(strict_types=1);
+<?php
+
+declare(strict_types=1);
 
 namespace NoGlitchYo\Dealdoh\Client;
 

src/Client/GoogleDnsClient.php

@@ -1,4 +1,6 @@
-<?php declare(strict_types=1);
+<?php
+
+declare(strict_types=1);
 
 namespace NoGlitchYo\Dealdoh\Client;
 
@@ -50,10 +52,14 @@ public function resolve(DnsUpstream $dnsUpstream, MessageInterface $dnsRequestMe
             throw new InvalidArgumentException('Query type must be in range [1, 65535]');
         }
 
-        $uri = $uri->withQuery(http_build_query([
-            'name' => $query->getQname(),
-            'type' => $query->getQtype()
-        ]));
+        $uri = $uri->withQuery(
+            http_build_query(
+                [
+                'name' => $query->getQname(),
+                'type' => $query->getQtype()
+                ]
+            )
+        );
 
         try {
             $response = $this->client->sendRequest(new Request('GET', $uri));

src/Client/StdClient.php

@@ -1,14 +1,16 @@
-<?php declare(strict_types=1);
+<?php
+
+declare(strict_types=1);
 
 namespace NoGlitchYo\Dealdoh\Client;
 
+use Exception;
+use LogicException;
+use NoGlitchYo\Dealdoh\Client\Transport\DnsTransportInterface;
+use NoGlitchYo\Dealdoh\Entity\Dns\Message\Header;
 use NoGlitchYo\Dealdoh\Entity\Dns\MessageInterface;
 use NoGlitchYo\Dealdoh\Entity\DnsUpstream;
 use NoGlitchYo\Dealdoh\Factory\Dns\MessageFactoryInterface;
-use Socket\Raw\Factory;
-use const MSG_WAITALL;
-use const SO_RCVTIMEO;
-use const SOL_SOCKET;
 
 /**
  * Standard DNS client making request over UDP (& TCP as a fallback)
@@ -18,41 +20,139 @@ class StdClient implements DnsClientInterface
 {
     public const EDNS_SIZE = 4096;
 
-    /**
-     * @var Factory
-     */
-    private $factory;
-
     /**
      * @var MessageFactoryInterface
      */
     private $dnsMessageFactory;
+    /**
+     * @var DnsTransportInterface
+     */
+    private $tcpTransport;
+    /**
+     * @var DnsTransportInterface
+     */
+    private $udpTransport;
 
-    public function __construct(Factory $factory, MessageFactoryInterface $dnsMessageFactory)
-    {
-        $this->factory = $factory;
+    public function __construct(
+        MessageFactoryInterface $dnsMessageFactory,
+        DnsTransportInterface $tcpTransport,
+        DnsTransportInterface $udpTransport
+    ) {
         $this->dnsMessageFactory = $dnsMessageFactory;
+        $this->tcpTransport = $tcpTransport;
+        $this->udpTransport = $udpTransport;
     }
 
+    /**
+     * Resolve message using regular UDP/TCP queries towards DNS upstream
+     *
+     * @param DnsUpstream      $dnsUpstream
+     * @param MessageInterface $dnsRequestMessage
+     *
+     * @return MessageInterface
+     * @throws Exception
+     */
     public function resolve(DnsUpstream $dnsUpstream, MessageInterface $dnsRequestMessage): MessageInterface
     {
-        $socket = $this->getClientSocket($dnsUpstream);
-        $remote = $dnsUpstream->getUri();
-        $socket->sendTo($this->dnsMessageFactory->createDnsWireMessageFromMessage($dnsRequestMessage), 0, $remote);
-        $socket->setOption(SOL_SOCKET, SO_RCVTIMEO, ['sec' => 5, 'usec' => 0]);
-        // TODO: Need to be improved: usage of tcp, handle truncated query, retry, etc...
-        $dnsWireResponseMessage = $socket->recvFrom(static::EDNS_SIZE, MSG_WAITALL, $remote);
+        $dnsRequestMessage = $this->enableRecursionForDnsMessage($dnsRequestMessage);
+        $address = $this->getSanitizedUpstreamAddress($dnsUpstream);
 
-        return $this->dnsMessageFactory->createMessageFromDnsWireMessage($dnsWireResponseMessage);
+        if ($this->isUdp($dnsUpstream)) {
+            $dnsWireResponseMessage = $this->sendWith('udp', $address, $dnsRequestMessage);
+        } elseif ($this->isTcp($dnsUpstream)) {
+            $dnsWireResponseMessage = $this->sendWith('tcp', $address, $dnsRequestMessage);
+        } else {
+            throw new LogicException(sprintf('Scheme `%s` is not supported', $dnsUpstream->getScheme()));
+        }
+
+        return $dnsWireResponseMessage;
     }
 
     public function supports(DnsUpstream $dnsUpstream): bool
     {
-        return in_array($dnsUpstream->getScheme(), ['udp', 'tcp', 'dns']) || $dnsUpstream->getScheme() === null;
+        return $this->isUdp($dnsUpstream) || $this->isTcp($dnsUpstream);
+    }
+
+    private function isUdp($dnsUpstream): bool
+    {
+        return in_array($dnsUpstream->getScheme(), ['udp', 'dns']) || $dnsUpstream->getScheme() === null;
+    }
+
+    private function isTcp($dnsUpstream): bool
+    {
+        return $dnsUpstream->getScheme() === 'tcp';
     }
 
-    private function getClientSocket(DnsUpstream $dnsUpstream)
+    /**
+     * Send DNS message using socket with the chosen protocol: `udp` or `tcp`
+     * Allow a sender to force usage of a specific protocol (e.g. protocol blocked by network/firewall)
+     *
+     * @param string           $protocol          Protocol to use to send the message
+     * @param string           $address
+     * @param MessageInterface $dnsRequestMessage
+     *
+     * @return MessageInterface
+     * @throws Exception
+     */
+    private function sendWith(
+        string $protocol,
+        string $address,
+        MessageInterface $dnsRequestMessage
+    ): MessageInterface {
+        $dnsWireMessage = $this->dnsMessageFactory->createDnsWireMessageFromMessage($dnsRequestMessage);
+
+        if ($protocol === 'udp') {
+            if (strlen($dnsWireMessage) <= static::EDNS_SIZE) { // Must use TCP if message is bigger
+                $dnsWireResponseMessage = $this->udpTransport->send($address, $dnsWireMessage);
+
+                $message = $this->dnsMessageFactory->createMessageFromDnsWireMessage($dnsWireResponseMessage);
+                // Only if message is not truncated response is returned, otherwise retry with TCP
+                if (!$message->getHeader()->isTc()) {
+                    return $message;
+                }
+            }
+        }
+
+        $dnsWireResponseMessage = $this->tcpTransport->send($address, $dnsWireMessage);
+
+        $message = $this->dnsMessageFactory->createMessageFromDnsWireMessage($dnsWireResponseMessage);
+
+        return $message;
+    }
+
+    /**
+     * Clean up the protocol from URI supported by the client but which can not be used with transport (e.g. dns://).
+     *
+     * @param DnsUpstream $dnsUpstream
+     *
+     * @return string
+     */
+    private function getSanitizedUpstreamAddress(DnsUpstream $dnsUpstream): string
+    {
+        return str_replace($dnsUpstream->getScheme() . '://', '', $dnsUpstream->getUri());
+    }
+
+    /**
+     * Enable recursion for the given DNS message
+     *
+     * @param MessageInterface $dnsRequestMessage
+     *
+     * @return MessageInterface
+     */
+    private function enableRecursionForDnsMessage(MessageInterface $dnsRequestMessage): MessageInterface
     {
-        return $this->factory->createClient('udp://' . $dnsUpstream->getUri());
+        return $dnsRequestMessage->withHeader(
+            new Header(
+                $dnsRequestMessage->getHeader()->getId(),
+                $dnsRequestMessage->getHeader()->isQr(),
+                $dnsRequestMessage->getHeader()->getOpcode(),
+                $dnsRequestMessage->getHeader()->isAa(),
+                $dnsRequestMessage->getHeader()->isTc(),
+                true, // Enable recursion (RD = 1)
+                $dnsRequestMessage->getHeader()->isRa(),
+                $dnsRequestMessage->getHeader()->getZ(),
+                $dnsRequestMessage->getHeader()->getRcode()
+            )
+        );
     }
 }