Project

General

Profile

Bug #9362

rc.dyndns.update: Cloudflare DDNS with proxy enabled doesn't work at all

Added by Nico Schneider 9 months ago. Updated about 2 months ago.

Status:
Feedback
Priority:
Normal
Category:
Dynamic DNS
Target version:
Start date:
03/03/2019
Due date:
% Done:

100%

Estimated time:
Affected Version:
Affected Architecture:

Description

When updating the DNS record via services_dyndns_edit.php it works normally, but when it tries to update it automatically (e.g. after the interface IP changes), it does not work and throws this error:

/rc.dyndns.update: phpDynDNS (@): PAYLOAD: {"success":false,"errors":[{"code":1004,"message":"DNS Validation Error","error_chain":[{"code":9003,"message":"Invalid 'proxied' value, must be a boolean"}]}],"messages":[],"result":null}

it seems like services_dyndns_edit.php sends the request the right way, however rc.dyndns.update doesnt.

Associated revisions

Revision 88863533 (diff)
Added by Robert Resch about 2 months ago

Fixes #9362: proxied value must be a boolean

Revision 77e4d068 (diff)
Added by Robert Resch about 2 months ago

Fixes #9362: proxied value must be a boolean

(cherry picked from commit 888635338d63e8b21297e3b25f0ff545fe9c4c41)

History

#1 Updated by Berzerker Berzerker 6 months ago

Chiming in that this is broken for me as well, as described in the original description.

2.4.4-p2

#2 Updated by Arian K. 6 months ago

Berzerker Berzerker wrote:

Chiming in that this is broken for me as well, as described in the original description.

2.4.4-p2

exactly same problem ... 2.4.4-p3

#3 Updated by Nathan Hand 4 months ago

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

  $hostData = array(
    "content" => "{$this->_dnsIP}",
    "type" => "{$recordType}",
    "proxied" => $this->_dnsProxied,
    "name" => "{$this->_dnsHost}" 
  );

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

  $dnsProxied = $conf['proxied'],   

The value is taken from /conf/config.xml which is

  <proxied></proxied>

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

--- services.inc.orig    2019-07-29 10:06:14.249341000 +1000
+++ services.inc    2019-07-29 10:31:42.415104000 +1000
@@ -1878,7 +1878,7 @@
         $dnsUser = $conf['username'],
         $dnsPass = $conf['password'],
         $dnsWildcard = $conf['wildcard'],
-        $dnsProxied = $conf['proxied'],
+        $dnsProxied = isset($conf['proxied']),
         $dnsMX = $conf['mx'],
         $dnsIf = "{$conf['interface']}",
         $dnsBackMX = NULL,

#4 Updated by Arian K. 4 months ago

Nathan Hand wrote:

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

[...]

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

[...]

The value is taken from /conf/config.xml which is

[...]

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

[...]

Nathan Hand wrote:

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

[...]

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

[...]

The value is taken from /conf/config.xml which is

[...]

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

[...]

Can't believe it's finally solved ... Unlimited thanks to you Nathan ...

#5 Updated by Jim Pingle 3 months ago

Can you submit that change as a pull request on github?

Thanks.

#6 Updated by Emmanuel Cardenas 3 months ago

Nathan Hand wrote:

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

[...]

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

[...]

The value is taken from /conf/config.xml which is

[...]

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

[...]

Seriously was going crazy! Thanks so much, Nathan, changing the configuration worked immediately after a reboot. Can't believe after so much frustration and scouring of the internet, this was able to be fixed. Seriously a thousand thanks! :D

#7 Updated by Berzerker Berzerker 3 months ago

Nathan Hand wrote:

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

[...]

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

[...]

The value is taken from /conf/config.xml which is

[...]

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

[...]

This fix does not work 100%. Changing this to the above causes the entries to be always set to proxied. The fact that the stock code works fine when manually updating vs an automatic update from an IP change leads me to believe the issue is not in the service itself, but the function that runs an update when the service is called.

#8 Updated by Robert R. about 2 months ago

Berzerker Berzerker wrote:

Nathan Hand wrote:

Underlying problem is /etc/inc/dyndns.class line 799. The value of dnsProxied is passed directly to Cloudflare.

[...]

However $this->_dnsProxied is neither true nor false, it's an empty string. The mistake is in /etc/inc/services.inc line 1881

[...]

The value is taken from /conf/config.xml which is

[...]

when true and missing when false. However the conf is taking the contents of the tag, not the existence of the tag. The fix is

[...]

This fix does not work 100%. Changing this to the above causes the entries to be always set to proxied. The fact that the stock code works fine when manually updating vs an automatic update from an IP change leads me to believe the issue is not in the service itself, but the function that runs an update when the service is called.

You are right. The bug was not in the method services_dyndns_configure_client but in the method services_dyndns_configure. The later one is reading the required parameter from the config. If the checkbox proxied is checked, then the xml tag _proxied' is present, otherwise not. So if we read the properties from the config, we must map it back to a boolean.

The manual update was working as pfsense was passing the required parameter directly to the method services_dyndns_configure_client.

I have open a PR (https://github.com/pfsense/pfsense/pull/4093).

For everyone, who need to fix it in the meantime, below the patch.

Cheers,
Robert

From 6c12cb864199fb360aeb3deb2587af0b58ba2758 Mon Sep 17 00:00:00 2001
From: Robert Resch <edenhaus@users.noreply.github.com>
Date: Thu, 26 Sep 2019 15:21:40 +0200
Subject: [PATCH] Fixes #9362: proxied value must be a boolean

---
 src/etc/inc/services.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/etc/inc/services.inc b/src/etc/inc/services.inc
index d73fe3265d..8ae8db448a 100644
--- a/src/etc/inc/services.inc
+++ b/src/etc/inc/services.inc
@@ -1960,6 +1960,7 @@ function services_dyndns_configure($int = "") {
                 $dyndns['verboselog'] = isset($dyndns['verboselog']);
                 $dyndns['curl_ipresolve_v4'] = isset($dyndns['curl_ipresolve_v4']);
                 $dyndns['curl_ssl_verifypeer'] = isset($dyndns['curl_ssl_verifypeer']);
+                $dyndns['proxied'] = isset($dyndns['proxied']);
                 services_dyndns_configure_client($dyndns);
                 sleep(1);
             }
-- 
2.20.1

#9 Updated by Jim Pingle about 2 months ago

  • Status changed from New to Pull Request Review

#10 Updated by Renato Botelho about 2 months ago

  • Status changed from Pull Request Review to Feedback
  • Assignee set to Renato Botelho
  • Target version set to 2.5.0
  • % Done changed from 0 to 100

PR has been merged. Thanks!

Also available in: Atom PDF