Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in DnsPinMiddleware.php, unable to resolve hostname #27870

Closed
giacomo892 opened this issue Jul 8, 2021 · 15 comments · Fixed by #27922
Closed

Bug in DnsPinMiddleware.php, unable to resolve hostname #27870

giacomo892 opened this issue Jul 8, 2021 · 15 comments · Fixed by #27922
Labels
1. to develop Accepted and waiting to be taken care of bug

Comments

@giacomo892
Copy link

Steps to reproduce

I'm running Nextcloud 22 on freebsd PHP8 , just upgraded from 21, and it failed

Checking for update of app activity in appstore
An unhandled exception has been thrown:
TypeError: count(): Argument #1 ($value) must be of type Countable|array, bool given in /usr/local/www/owncloud/lib/private/Http/Client/DnsPinMiddleware.php:68
Stack trace:
#0 /usr/local/www/owncloud/lib/private/Http/Client/DnsPinMiddleware.php(68): count()
#1 /usr/local/www/owncloud/lib/private/Http/Client/DnsPinMiddleware.php(111): OC\Http\Client\DnsPinMiddleware->dnsResolve()
#2 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php(35): OC\Http\Client\DnsPinMiddleware->OC\Http\Client{closure}()
#3 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php(31): GuzzleHttp\PrepareBodyMiddleware->__invoke()
#4 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/RedirectMiddleware.php(71): GuzzleHttp\Middleware::GuzzleHttp{closure}()
#5 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php(63): GuzzleHttp\RedirectMiddleware->__invoke()
#6 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/HandlerStack.php(75): GuzzleHttp\Middleware::GuzzleHttp{closure}()
#7 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/Client.php(331): GuzzleHttp\HandlerStack->__invoke()
#8 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/Client.php(168): GuzzleHttp\Client->transfer()
#9 /usr/local/www/owncloud/3rdparty/guzzlehttp/guzzle/src/Client.php(187): GuzzleHttp\Client->requestAsync()
#10 /usr/local/www/owncloud/lib/private/Http/Client/Client.php(223): GuzzleHttp\Client->request()
#11 /usr/local/www/owncloud/lib/private/App/AppStore/Fetcher/Fetcher.php(108): OC\Http\Client\Client->get()
#12 /usr/local/www/owncloud/lib/private/App/AppStore/Fetcher/AppFetcher.php(79): OC\App\AppStore\Fetcher\Fetcher->fetch()
#13 /usr/local/www/owncloud/lib/private/App/AppStore/Fetcher/Fetcher.php(180): OC\App\AppStore\Fetcher\AppFetcher->fetch()
#14 /usr/local/www/owncloud/lib/private/Installer.php(434): OC\App\AppStore\Fetcher\Fetcher->get()
#15 /usr/local/www/owncloud/lib/private/Updater.php(420): OC\Installer->isUpdateAvailable()
#16 /usr/local/www/owncloud/lib/private/Updater.php(268): OC\Updater->upgradeAppStoreApps()
#17 /usr/local/www/owncloud/lib/private/Updater.php(130): OC\Updater->doUpgrade()
#18 /usr/local/www/owncloud/core/Command/Upgrade.php(241): OC\Updater->upgrade()
#19 /usr/local/www/owncloud/3rdparty/symfony/console/Command/Command.php(255): OC\Core\Command\Upgrade->execute()
#20 /usr/local/www/owncloud/3rdparty/symfony/console/Application.php(1009): Symfony\Component\Console\Command\Command->run()
#21 /usr/local/www/owncloud/3rdparty/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand()
#22 /usr/local/www/owncloud/3rdparty/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#23 /usr/local/www/owncloud/lib/private/Console/Application.php(209): Symfony\Component\Console\Application->run()
#24 /usr/local/www/owncloud/console.php(99): OC\Console\Application->run()
#25 /usr/local/www/owncloud/occ(11): require_once('/usr/local/www/...')

Turns out the issue is in DnsPinMiddleware.php.
Basically there

$dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME];
foreach ($dnsTypes as $key => $dnsType) {
if ($this->negativeDnsCache->isNegativeCached($target, $dnsType)) {
unset($dnsTypes[$key]);
continue;
}

assuming the $target is apps.nextcloud.com after that part of the code the $target will be apps.nextcloud.comapps.nextcloud.comapps.nextcloud.com which will make the dns resolver to fail.

Sorry for the short report but I am running out of time.

@giacomo892 giacomo892 added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Jul 8, 2021
@kesselb
Copy link
Contributor

kesselb commented Jul 9, 2021

assuming the $target is apps.nextcloud.com after that part of the code the $target will be apps.nextcloud.comapps.nextcloud.comapps.nextcloud.com which will make the dns resolver to fail.

Are you sure? I don't see that we modify $target in the above code.

@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jul 9, 2021
@kesselb
Copy link
Contributor

kesselb commented Jul 9, 2021

$dnsResponses = dns_get_record($target, $dnsType);
$canHaveCnameRecord = true;
if (count($dnsResponses) > 0) {

dns_get_record may return false (if for whatever reason resolving the dns record is not possible). In the above code a check if $dnsResponses is false is missing. count(false) will raise a "must be of type Countable|array," warning because false is not countable.

But there is still something different broken as resolving a dns record failed. #27873 is the same error also for FreeBSD. I remember some issues with OpenSSL due some security hardening on FreeBSD. Is it possible that resolving dns records is forbidden on your setup (for php applications).

To test if getting dns records work in general with: php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"

@giacomo892
Copy link
Author

@kesselb Your command executes just fine. Infact that's how I manage to finish the upgrade, I've changed
$dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME];
to
$dnsTypes = [DNS_A];

and it worked

$ php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"
array(1) {
  [0]=>
  array(5) {
    ["host"]=>
    string(18) "apps.nextcloud.com"
    ["class"]=>
    string(2) "IN"
    ["ttl"]=>
    int(3600)
    ["type"]=>
    string(1) "A"
    ["ip"]=>
    string(12) "176.9.217.53"
  }
}

assuming the $target is apps.nextcloud.com after that part of the code the $target will be apps.nextcloud.comapps.nextcloud.comapps.nextcloud.com which will make the dns resolver to fail.

Are you sure? I don't see that we modify $target in the above code.

Quite, I added a print statement in the code.

@kesselb
Copy link
Contributor

kesselb commented Jul 9, 2021

php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"
php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_AAAA));"
php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_CNAME));"
php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A | DNS_AAAA | DNS_CNAME));"

Would you mind to do another test round? ;)

@giacomo892
Copy link
Author

@kesselb Sure!

php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"
array(1) {
  [0]=>
  array(5) {
    ["host"]=>
    string(18) "apps.nextcloud.com"
    ["class"]=>
    string(2) "IN"
    ["ttl"]=>
    int(3600)
    ["type"]=>
    string(1) "A"
    ["ip"]=>
    string(12) "176.9.217.53"
  }
}
$ php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_AAAA));"
array(1) {
  [0]=>
  array(5) {
    ["host"]=>
    string(18) "apps.nextcloud.com"
    ["class"]=>
    string(2) "IN"
    ["ttl"]=>
    int(3600)
    ["type"]=>
    string(4) "AAAA"
    ["ipv6"]=>
    string(21) "2a01:4f8:130:32f1::53"
  }
}
$ php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_CNAME));"
PHP Warning:  dns_get_record(): DNS Query failed in Command line code on line 1
bool(false)
$ php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A | DNS_AAAA | DNS_CNAME));"
PHP Warning:  dns_get_record(): DNS Query failed in Command line code on line 1
bool(false)

@ntinti
Copy link

ntinti commented Jul 10, 2021

Same here on FreeBSD 12.2. with NC 22.0 - Problem is "CNAME"

php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"
array(1) {
[0]=>
array(5) {
["host"]=>
string(18) "apps.nextcloud.com"
["class"]=>
string(2) "IN"
["ttl"]=>
int(1789)
["type"]=>
string(1) "A"
["ip"]=>
string(12) "176.9.217.53"
}
}
php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_AAAA));"
array(1) {
[0]=>
array(5) {
["host"]=>
string(18) "apps.nextcloud.com"
["class"]=>
string(2) "IN"
["ttl"]=>
int(104)
["type"]=>
string(4) "AAAA"
["ipv6"]=>
string(21) "2a01:4f8:130:32f1::53"
}
}
php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_CNAME));"
bool(false)

@l1gi
Copy link

l1gi commented Jul 12, 2021

Same problem. Quick workaround:

--- /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php.orig  2021-07-12 09:23:37.206156000 +0200
+++ /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php       2021-07-12 09:19:34.353092000 +0200
@@ -65,7 +65,7 @@
 
                        $dnsResponses = dns_get_record($target, $dnsType);
                        $canHaveCnameRecord = true;
-                       if (count($dnsResponses) > 0) {
+                       if ($dnsResponse && count($dnsResponses) > 0) {
                                foreach ($dnsResponses as $key => $dnsResponse) {
                                        if (isset($dnsResponse['ip'])) {
                                                $targetIps[] = $dnsResponse['ip'];

@LukasReschke LukasReschke added this to the Nextcloud 22.0.1 milestone Jul 12, 2021
LukasReschke added a commit that referenced this issue Jul 12, 2021
`dns_get_record` can return false which results in exceptions such as
the ones shown in #27870.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke
Copy link
Member

Adding a non-false check here makes sense. Please check #27922

backportbot-nextcloud bot pushed a commit that referenced this issue Jul 12, 2021
`dns_get_record` can return false which results in exceptions such as
the ones shown in #27870.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@tronyx
Copy link

tronyx commented Jul 12, 2021

@kesselb Your command executes just fine. Infact that's how I manage to finish the upgrade, I've changed
$dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME];
to
$dnsTypes = [DNS_A];

and it worked

$ php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));"
array(1) {
  [0]=>
  array(5) {
    ["host"]=>
    string(18) "apps.nextcloud.com"
    ["class"]=>
    string(2) "IN"
    ["ttl"]=>
    int(3600)
    ["type"]=>
    string(1) "A"
    ["ip"]=>
    string(12) "176.9.217.53"
  }
}

assuming the $target is apps.nextcloud.com after that part of the code the $target will be apps.nextcloud.comapps.nextcloud.comapps.nextcloud.com which will make the dns resolver to fail.

Are you sure? I don't see that we modify $target in the above code.

Quite, I added a print statement in the code.

Thank you for this! I've been pulling what little hair I have left out for the last 3 days trying to get my Docker container to function again.

@gt2416
Copy link

gt2416 commented Jul 17, 2021

TrueNAS 12 - U4 (FreeBSD 12)
Even after applying the fix I still get a error in logs. It used to be 3 errors but now its just

Error PHP Error: dns_get_record(): DNS Query failed at /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php#66

Running php -r "var_dump(dns_get_record('apps.nextcloud.com', DNS_A));" I get

DNS_A));"
array(1) {
  [0]=>
  array(5) {
    ["host"]=>
    string(18) "apps.nextcloud.com"
    ["class"]=>
    string(2) "IN"
    ["ttl"]=>
    int(3022)
    ["type"]=>
    string(1) "A"
    ["ip"]=>
    string(12) "176.9.217.53"
  }
}

@moviuro
Copy link

moviuro commented Jul 22, 2021

I'm still getting errors in my logs when trying to open the dashboard: I don't get the background image, and the following (partial) log:

Jul 22 08:20:17 sco Nextcloud[43804]: {"reqId":"nCGGKCb7iXqFnVqsoR43","level":3,"time":"2021-07-22T06:20:17+00:00","remoteAddr":"10.21.10.12","user":"moviuro","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/weather_status/api/v1/forecast","message":"{\"Exception\":\"Error\",\"Message\":\"dns_get_record(): DNS Query failed at /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php#52\",\"Code\":0,\"Trace\":[{\"function\":\"onError\",\"class\":\"OC\\\\Log\\\\ErrorHandler\",\"type\":\"::\",\"args\":[2,\"dns_get_record(): DNS Query failed\",\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",52]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":52,\"function\":\"dns_get_record\",\"args\":[\"api.met.no\",32]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":113,\"function\":\"dnsResolve\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"api.met.no\",0]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php\",\"line\":35,\"function\":\"OC\\\\Http\\\\Client\\\\{closure}\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":31,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\PrepareBodyMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameter replaced ***\",\"*** sensitive parameter replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/RedirectMiddleware.php\",\"line\":71,\"function\":\"GuzzleHttp\\\\{closure}\",\"class\":\"GuzzleHttp\\\\Middleware\",\"type\":\"::\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":63,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\RedirectMiddleware\",
Jul 22 08:20:17 sco Nextcloud[43804]: {"reqId":"nCGGKCb7iXqFnVqsoR43","level":3,"time":"2021-07-22T06:20:17+00:00","remoteAddr":"10.21.10.12","user":"moviuro","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/weather_status/api/v1/forecast","message":"{\"Exception\":\"Error\",\"Message\":\"dns_get_record(): DNS Query failed at /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php#66\",\"Code\":0,\"Trace\":[{\"function\":\"onError\",\"class\":\"OC\\\\Log\\\\ErrorHandler\",\"type\":\"::\",\"args\":[2,\"dns_get_record(): DNS Query failed\",\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",66]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":66,\"function\":\"dns_get_record\",\"args\":[\"api.met.no\",134217728]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":113,\"function\":\"dnsResolve\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"api.met.no\",0]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php\",\"line\":35,\"function\":\"OC\\\\Http\\\\Client\\\\{closure}\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":31,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\PrepareBodyMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameter replaced ***\",\"*** sensitive parameter replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/RedirectMiddleware.php\",\"line\":71,\"function\":\"GuzzleHttp\\\\{closure}\",\"class\":\"GuzzleHttp\\\\Middleware\",\"type\":\"::\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":63,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\RedirectMiddle
Jul 22 08:20:17 sco Nextcloud[43804]: {"reqId":"nCGGKCb7iXqFnVqsoR43","level":3,"time":"2021-07-22T06:20:17+00:00","remoteAddr":"10.21.10.12","user":"moviuro","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/weather_status/api/v1/forecast","message":"{\"Exception\":\"Error\",\"Message\":\"dns_get_record(): DNS Query failed at /usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php#66\",\"Code\":0,\"Trace\":[{\"function\":\"onError\",\"class\":\"OC\\\\Log\\\\ErrorHandler\",\"type\":\"::\",\"args\":[2,\"dns_get_record(): DNS Query failed\",\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",66]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":66,\"function\":\"dns_get_record\",\"args\":[\"api.met.no\",16]},{\"file\":\"/usr/local/www/nextcloud/lib/private/Http/Client/DnsPinMiddleware.php\",\"line\":113,\"function\":\"dnsResolve\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"api.met.no\",0]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php\",\"line\":35,\"function\":\"OC\\\\Http\\\\Client\\\\{closure}\",\"class\":\"OC\\\\Http\\\\Client\\\\DnsPinMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":31,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\PrepareBodyMiddleware\",\"type\":\"->\",\"args\":[\"*** sensitive parameter replaced ***\",\"*** sensitive parameter replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/RedirectMiddleware.php\",\"line\":71,\"function\":\"GuzzleHttp\\\\{closure}\",\"class\":\"GuzzleHttp\\\\Middleware\",\"type\":\"::\",\"args\":[\"*** sensitive parameters replaced ***\"]},{\"file\":\"/usr/local/www/nextcloud/3rdparty/guzzlehttp/guzzle/src/Middleware.php\",\"line\":63,\"function\":\"__invoke\",\"class\":\"GuzzleHttp\\\\RedirectMiddleware\",

I had hand-edited my lib/private/Http/Client/DnsPinMiddleware.php file, same as in #27922 https://github.com/nextcloud/server/pull/27922/files

if ($dnsResponses !== false && count($dnsResponses) > 0) {
  foreach ($dnsResponses as $key => $dnsResponse) {
...

@LukasReschke
Copy link
Member

Please file a new bug and provide all necessary details. We sadly lack a 🔮

@bcurran3
Copy link

@kesselb Your command executes just fine. Infact that's how I manage to finish the upgrade, I've changed
$dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME];
to
$dnsTypes = [DNS_A];
and it worked

Allowed my upgrade to complete. THANK YOU!

@ajdunevent
Copy link

I'm going to risk exposing myself as an idiot in the spirit of trying to help others.

If you still seem to be struggling with your server claiming it has no internet connection after this was fixed in version 21.1.0 and you're running systemd-resolved on Arch Linux, make sure your /etc/resolv.conf isn't empty. It should probably be linked to /run/systemd/resolve/stub-resolv.conf per https://wiki.archlinux.org/title/Systemd-resolved#DNS 🤦‍♂️️

@sveeke
Copy link

sveeke commented Jan 24, 2022

A friend asked me to look at his Nextcloud server because it didn't want to update to 23. It had a absolutely insane amount of errors about /lib/private/Http/Client/DnsPinMiddleware.php#83. Like suggested in this issue, we first changed $dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME]; to $dnsTypes = [DNS_A]; so it stopped failing basic DNS requests. Then we updated it manually to 23.0.0 (which wasn't possible before the change).

Now 23.0.0 seems to run fine, except that the constant flood of error messages is back. So we again changed DnsPinMiddleware.php so the flood will stop. Now Nextcloud of course whines about file integrity (which is better than the flood of errors).

This band aid is fine for now, but does anyone have any suggestions how to fix this in a more robust and future proof way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.