Plugins: DynDNS: Invalid argument supplied for foreach()

[Dunky13]

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:

[x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md

[x] I have searched the existing issues and I'm convinced that mine is new.

Describe the bug
I added a DynDNS to cloudflare, and when reloading DynDNS from the lobby I get an PHP error:

PHP Warning: Invalid argument supplied for foreach() in /usr/local/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc on line 739

I'm running the latest 19.7.6.

I have 1 domain set-up in dyndns

Expected behavior
Update the Cached IP from the WAN address without throwing this error

Screenshots
n/a

Relevant log files
User-Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.121 Safari/537.36 Vivaldi/2.8.1664.44
FreeBSD 11.2-RELEASE-p14-HBSD 07680caafe9(stable/19.7) amd64
OPNsense 19.7.6 32dbc73f4
Plugins os-dyndns-1.17 os-theme-rebellion-1.8.2
Time Tue, 05 Nov 2019 12:41:31 +0100
OpenSSL 1.0.2t 10 Sep 2019
PHP 7.2.24

Additional context
Add any other context about the problem here.

Environment
Had this issue on bare-metal as well as in VM under proxmox
Hardware:
Intel i5-7200k
16GB DDR4
32GB SSD

Clean install

[moonman]

There are a couple of threads about this on the forum.
Here is one of them (the latest): https://forum.opnsense.org/index.php?PHPSESSID=mbmv9aepvaknkcjvnitpoq6qpk&topic=15110.msg68875#msg68875
It only seems to happen with cloudflare and DynDNS.

[richard-parker]

There are a couple of threads about this on the forum.
Here is one of them (the latest): https://forum.opnsense.org/index.php?PHPSESSID=mbmv9aepvaknkcjvnitpoq6qpk&topic=15110.msg68875#msg68875
It only seems to happen with cloudflare and DynDNS.

Cloudflare here; exact same issue.

[afontenot]

Seeing the same issue here. Anything any of us can do to help debug this?

[moonman]

Removing "Description" from all Dynamic DNS entries seems to fix this problem.

[afontenot]

This problem has proven difficult to debug. I don't know if @moonman's solution works for me because this problem doesn't seem to occur for me every time I force a refresh. I only notice the error reports once a week or so. What I suggest is this: modify the file reported in the error like so:

$output = curl_exec($ch);

should become

$cloudflare_json_resp = curl_exec($ch);
file_put_contents("/tmp/dyndnsbug.txt", date(DATE_RFC2822) . "\n", FILE_APPEND);
file_put_contents("/tmp/dyndnsbug.txt", $cloudflare_json_resp, FILE_APPEND);
$output = json_decode($cloudflare_json_resp);

Next time you get an error pull the relevant json out of the log file /tmp/dyndnsbug.txt and after editing it for privacy, post it here. I'll be doing that myself. (Note that printing the JSON to the system log appears to fail silently, probably because the string is very long.)

My current best guess is that this is a intermittent problem with Cloudflare. I don't see any way for the Description field to affect what we send to them. That said, I'm also logging the curl request itself on my end so that if it turns out we're sending bad data, it should show up in my logs next time I get the error.

[afontenot]

Okay, got a fail. Here are the logs:

JSON log:

Tue, 07 Apr 2020 21:56:06 -0700

Warning: Invalid argument supplied for foreach() in /usr/local/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc on line 782

CURL log:

* Could not resolve host: api.cloudflare.com
* Closing connection 0
* Could not resolve host: api.cloudflare.com
* Closing connection 1

So here's what's happening: Curl is unable to resolve the domain it needs to connect to in order to update the dynamic DNS records, so it bails out. The JSON response is therefore empty (notice the blank line after the date), and the foreach fails trying to access the result item in the parsed JSON.

It seems that there are likely several overlapping problems here:

1. The DNS lookup should not be failing. I don't have any noticeable connection issues, and this error is happening much too frequently. I'm using Cloudflare's DNS over TLS via Unbound, so the DNS error is rather surprising. According to the logs, OPNsense is trying again almost immediately (8 seconds later), and it succeeds this time right away. Maybe there's a very short timeout on DNS responses??

2. When the CURL request indicates a broken connection, DynDNS shouldn't explode with a "bug report" style error message. Rather, it should try again after a short period of time and save the error to the log.

3. When CURL returns an empty string (e.g. because the Cloudflare server is down), DynDNS should bail out and log an error, similar to if the connection was down, rather than trying to parse the results.

4. Even after the retry succeeds, my logs show that DynDNS tries again five seconds after that. Not sure why this is happening. Some kind of race condition?

[joydashy]

Removing "Description" from all Dynamic DNS entries seems to fix this problem.

I had this error, and my entries have no Description.

[12-May-2020 09:10:14 Europe/Amsterdam] PHP Warning: Invalid argument supplied for foreach() in /usr/local/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc on line 775

[moonman]

Removing "Description" from all Dynamic DNS entries seems to fix this problem.

I had this error, and my entries have no Description.

[12-May-2020 09:10:14 Europe/Amsterdam] PHP Warning: Invalid argument supplied for foreach() in /usr/local/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc on line 775

Fair enough. Somehow it fixed it for me, but I haven't gone indepth as afontenot.

[AdSchellevis]

This issue has been automatically timed-out (after 180 days of inactivity).

For more information about the policies for this repository,
please read https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md for further details.

If someone wants to step up and work on this issue,
just let us know, so we can reopen the issue and assign an owner to it.

[seitzbg]

This is still broken, why id you close it!

[afontenot]

I'm still seeing this too. I think I provided enough information that this should be trivially fixable assuming this plugin is actively maintained. Barring that, if someone following this can write decent PHP, you can probably provide a patch for this by working out what the correct way to report a non-fatal error which doesn't trigger a bug report in this code base is, and use that to solve the following issues:

1. When the CURL request indicates a broken connection, DynDNS shouldn't explode with a "bug report" style error message. Rather, it should try again after a short period of time and save the error to the log.

2. When CURL returns an empty string (e.g. because the Cloudflare server is down), DynDNS should bail out and log an error, similar to if the connection was down, rather than trying to parse the results.

Basically, there's no error checking at all - the code JSON decodes some data it gets directly from the dynamic DNS provider, and if it can't be parsed correctly or doesn't contain the expected data structures, the code just explodes. It's very bad. Just providing some basic sanity checking would be enough to stop this problem from happening.

For anyone else like me who doesn't have the ability to write a good enough patch, you could help by providing the following information:

1. What's your dynamic DNS service provider? Mine is Cloudflare, and I believe that's the only one for which this specific bug arises. (Very similar bugs are likely to exist with other providers, assuming those parts of the code similarly lack sanity checking.)

2. What's your DNS setup? As I indicated in my April comment, this bug occurs for me when curl can't resolve the Cloudflare API hostname. (There are definitely other situations that can cause it, but I haven't run into them.) I'm using Cloudflare's DNS over TLS with Unbound. If others with this problem have the same DNS setup (or are at least using Cloudflare as their DNS provider), that would be very telling.

[MBfromOK]

The closing post says that this issue was closed due to timeout, until this gets fixed I suppose one of us should log a post to the effect that we are still getting this error and refer to @afontenot's submission regarding the curl results every day, week, month or 90 days.

_...squeeky wheel and all that_

[MBfromOK]

What I want to know is why @fichtner re-opened it and removed the help wanted tag only for @AdSchellevis to come breezing in a couple of days later and re-close it after telling #1857 that they should report in this open issue the exact same problem instead of creating a duplicate.

[AdSchellevis]

@MBfromOK The "automatically timed-out" is exactly that, if nobody offered to own the item in 180days it will be closed automatically (it doesn't make sense to keep issues open for longer than that if nobody is actually willing to work on it). My account is just used to enforce the project policy, that's all.

The DynDNS is a very old plugin, which isn't actively maintained by us, it needs a rewrite, which isn't on our roadmap. Issues like this are really for the community to pick up. If you plan to work on it, just let us know and we can discuss moving the work item to you.

[fichtner]

breezing in a couple of days later and re-close

err what now... this ticket is open according to github...

[MBfromOK]

Thanks for that detail @AdSchellevis, I just re-read my posts and I acknowledged that it was closed due to timeout and suggested we do our part to keep it open by causing new activity. If status is determined by ownership (or lack thereof) then my suggestion wouldn't help.

Regarding the breezing in: It says @fichtner self assigned, added cleanup, removed help wanted, then reopened the ticket. I must have mistakenly thought the the reference that followed (and said closed) was referring to this request and not that one... My appologies! (especially wasting your time to answer my drivel) :-P

As soon as I figure out where & how to work on things like this in OPNSense I wouldn't mind taking a crack (seems like it should be fairly simple), but I am a programmer in class, so it will be a while if it is waiting on me.

[seitzbg]

Still seeing this issue with CloudFlare as of 20.7.2.

[mannp]

I've submitted 3 or 4 bug reports via the gui for this one :)

FYI It is still happening on 20.7.3 which I guess was likely, with discussions still ongoing.

[juantxorena]

Almost exactly one year later, the bug is still on.

@AdSchellevis you claim that DynDNS is a very old plugin and not maintained by you. Is this still true? I would think that a dynamic DNS update is important-ish in a firewall like this.

[AdSchellevis]

@juantxorena https://github.com/opnsense/plugins/issues/1611#issuecomment-706129596

[AdSchellevis]

I guess most other distributions use https://github.com/ddclient/ddclient (which is also included in our package repository), which might be a better path then trying to resurrect (nearly) dead code.

[giggio]

In the latest plugin version it is failing at line 838. This is the latest code at this moment, in this repo:

https://github.com/opnsense/plugins/blob/e0c09fa3570858bfd02bf391fb15f2ff0b8883b1/dns/dyndns/src/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc#L832-L848

Anything that goes wrong on line 834 will produce an error on line 838. An error in curl might do it, an error in parsing might also do it.
A warning should be enough, no need for the crash report. The internet link might be down temporarily, or something else might have failed (dns etc IDK).

I'd give it a go, but I am not a php dev, and I could mess it up more. The file seems to have no error handling whatsover, and the pattern $output = json_decode(curl_exec($ch)) shows up in other parts of the same file, so it probably fails for other providers as well.

I'm pretty sure a php dev should be able to fix it in no time.

[fichtner]

With a JSON dump perhaps.

On 8. Nov 2020, at 21:02, Giovanni Bassi [e-mail] wrote:


In the latest plugin version it is failing at line 838. This is the latest code at this moment, in this repo:

https://github.com/opnsense/plugins/blob/e0c09fa3570858bfd02bf391fb15f2ff0b8883b1/dns/dyndns/src/etc/inc/plugins.inc.d/dyndns/phpDynDNS.inc#L832-L848

Anything that goes wrong on line 834 will produce an error on line 838. An error in curl might do it, an error in parsing might also do it.
A warning should be enough, no need for the crash report. The internet link might be down temporarily, or something else might have failed (dns etc IDK).

I'd give it a go, but I am not a php dev, and I could mess it up more. The file seems to have no error handling whatsover, and the pattern $output = json_decode(curl_exec($ch)) shows up in other parts of the same file, so it probably fails for other providers as well.

I'm pretty sure a php dev should be able to fix it in no time.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[giggio]

There is no need for a json dump to fix such a simple error. Some exception handling should be enough, and clearly both json_decode and curl_exec are executing without any treatment.
I don't even program in PHP and I'm pretty sure that the cases where these functions fail are very well documented.
Actually, I just googled it and the docs even show how to do some exception handling, it should be simple enough for an experienced and willing dev.

[fichtner]

If we iterate over unknown context in the foreach of the returned data we just need a magic spell to fix this then. To reiterate: either we get a JSON dump to see what's going on or we will have to keep waiting for a different kind of miracle.

[giggio]

I really don't care if this is properly fixed (and I don't know if it is possible), I just want the error report to stop showing up, as this simply resolves itself after a while. As I mentioned earlier, this is probably nothing more than a network glitch, and no error reporting will fix a link problem.

For the time being, until someone sends a json dump, if anyone reading this knows how to write a try/catch block on PHP and knows how to make sure it works well enough, please simply add some simple error handling, so the plugin stops raising false errors. It should be a simple PR. I'd do it, but PHP is not one of the languages I work on.

[fichtner]

I really don't care if this is properly fixed (and I don't know if it is possible), I just want the error report to stop showing up, as this simply resolves itself after a while. As I mentioned earlier, this is probably nothing more than a network glitch, and no error reporting will fix a link problem.

Again: too much sentiment, too many assumptions.

[giggio]

This is a one year bug, people are switching away from the plugin. If it is in the interest of the maintainers to keep their users, it would be a nice idea to at least try to stop nagging them with unhelpful error messages.
The link problem is an assumption. The nagging is a fact.

Also, the json dump will come for free if someone adds error handling to that one PHP file, and let it crash with more info. Then people would submit the error to the Opnsense project with the json dump.
Asking people who don't know the language to write code is not going to help, as it's been shown, it's been an year without the dump.

[fichtner]

If it is in the interest of the maintainers to keep their users

Cloudflare support, like many other third party services in this plugin are maintained by the respective users. If there is a runtime error someone with Cloudflare support will have to help fix it. Ideally the original author would take care of it but I am not joining the blame game. It is what it is. Your move. ;)

[AdSchellevis]

@giggio have you tried switching to https://github.com/ddclient/ddclient as suggested earlier, it probably wouldn't be very difficult to create a plugin for, just waiting for someone to start I guess. if there is an alternative for the old DynDNS code, it's highly likely we will deprecate it in a future release for reasons mentioned earlier.

[giggio]

@AdSchellevis I'm trying to keep as little customizations out of the OpnSense radar as possible. I want to be able to quickly restore a backup if I need to. If I customize too much I'll need to restore a backup and run a series of post restore procedures, and I want to avoid that.
If this issue keeps up for too long I'll probably find some other way to solve the issue. I already had a solution using a container in a Pi: https://hub.docker.com/r/oznu/cloudflare-ddns
I'd expect such a large project to have a solution for this problem, and I was happy when I found it. It's a pity the original author is not maintaining the plugin anymore. Solving the issue with cloudflare is cost free and super cool. I have a backup on noip, but they nag me every month on the free plan, asking me to confirm I'm still using the plan.

[afontenot]

As I mentioned in my previous comment, I already did log the JSON response, and it was empty. There is no reason at all to think that the problem with this is something on Cloudflare's end and that they need to help. This is an issue with this code lacking any kind of error recovery, full stop.

Your DNS server is down? Well, the curl request is going to fail to pull the JSON, so you get this error. Probably the same thing if your Internet connection goes down for long enough. This doesn't necessarily have anything to do with Cloudflare. The code needs error handling.

[fichtner]

No, the alleged fix needs a contributor and a tester or two. 😉