Issues on OpenWRT 22.03.4

[carstencodes]

Hi,

thank you for your tremendous work. This was, what I was looking for.

Thanks to #18 by @LLdaniel I was also able to install it on OpenWrt 22.03.4.

Unfortunately, I found some issues:

1. Missing r-permission on config
   /etc/init.d/hetzner_ddns starts process as nobody. After installation, the config file has o600 permissions only
2. Missing write-permissions on log-file and pid-file
   /etc/init.d/hetzner_ddns starts process as nobody. /var/run and /var/log are by default only root-writable
3. Wrong link to start script
   /etc/init.d/hetzner_ddns wants to start /usr/bin/hetzner_ddns, but only /usr/bin/hetzner_ddns.sh exists
4. Issue with config-file as parameter
   After solving all issues above, the following issue remains: The start-up script tries to acquire /var/log/hetzner_ddns./etc/config/hetzner_ddns.conf.log

My current workarounds to this issue:

1. After installation: chmod a+r /etc/config/hetzner_dds.conf
2. After installation: touch /var/run/hetzner_ddns.pid /var/log/hetzner_ddns.pid && chown nobody:nogroup /var/run/hetzner_ddns.pid /var/log/hetzner_ddns.pid
3. After installation: ln -s /usr/bin/hetzner_ddns.sh /usr/bin/hetzner_ddns
4. After installation: sed -i 's@command /usr/bin/hetzner_ddns -d /etc/config/hetzner_ddns.conf@command /usr/bin/hetzner_ddns -d@' /etc/init.d/hetzner_ddns

Step four is something I really did not like. And I think, that 1-3 should be trivial steps I can also provide a fix for.

But I have no idea what goes wrong in 4.

Regards

Carsten

[filiparag]

Hi, thanks for sending the report.

I invite @LLdaniel to chime in, if they want, as I have no prior experience with OpenWrt packaging and services.

Missing [...]permission on [...]

The permissions should be as restrictive as possible. To me it looks like the process should be started as a user like daemon on various Linux distributions, who is also the owner of those files.

But I have no idea what goes wrong in 4.

The [CONFIG] parameter is supposed to represent infix in the configuration path filename, which can be seen here: [1] [2]. It is not an absolute path for the configuration file, so removing it makes sense if you don't have multiple configuration files.

Example: /usr/local/etc/hetzner_ddns.foo.conf /usr/local/etc/hetzner_ddns.bar.conf /usr/local/etc/hetzner_ddns.baz.conf. In this case, the [CONFIG] parameter would be either foo, bar, or baz. Hope this clears things up.

The infix configuration file parameter might not be the most fortunate, or intuitive way to use multiple files, but if it's changed now, it would break existing behaviour.

[LLdaniel]

Hi,

thanks for the report! I will try to address your points and have to admit that I tested it for OpenWrt version 23.05 (At the moment I suspect no difference for 22.03 and 23.05 regarding hetzner_ddns).

Just for me as a question: Did you install the .ipk package or did you try to create it directly on your OpenWrt machine with the Makefile in the repo's root directory? (Unfortunately I went the first way and created the package)

1. Missing r-permission on config
   /etc/init.d/hetzner_ddns starts process as nobody. After installation, the config file has o600 permissions only

I tried to follow closely the OpenWrt doc and tried to use the a non-root user. See:

hetzner_ddns/hetzner_ddns.openwrt.rc

Line 21 in ce69db6

procd_set_param user nobody

Maybe we can change the default permissions for the config file as you already suggested. Or as @filiparag suggested: Switch to the daemon user:

# my available openwrt users (without creating one manually)
 # cat /etc/passwd
root:x:0:0:root:/root:/bin/ash
daemon:*:1:1:daemon:/var:/bin/false
ftp:*:55:55:ftp:/home/ftp:/bin/false
network:*:101:101:network:/var:/bin/false
nobody:*:65534:65534:nobody:/var:/bin/false
ntp:x:123:123:ntp:/var/run/ntp:/bin/false
dnsmasq:x:453:453:dnsmasq:/var/run/dnsmasq:/bin/false
logd:x:514:514:logd:/var/run/logd:/bin/false
ubus:x:81:81:ubus:/var/run/ubus:/bin/false

2. Missing write-permissions on log-file and pid-file
   /etc/init.d/hetzner_ddns starts process as nobody. /var/run and /var/log are by default only root-writable

same answer like for 1.

3. Wrong link to start script
   /etc/init.d/hetzner_ddns wants to start /usr/bin/hetzner_ddns, but only /usr/bin/hetzner_ddns.sh exists

Seems I overlooked that, probably you are referring to this line:

hetzner_ddns/hetzner_ddns.openwrt.rc

Line 9 in ce69db6

procd_set_param command /usr/bin/hetzner_ddns -d /etc/config/hetzner_ddns.conf

4. Issue with config-file as parameter
   After solving all issues above, the following issue remains: The start-up script tries to acquire /var/log/hetzner_ddns./etc/config/hetzner_ddns.conf.log

Thanks @filiparag explanation. First I wondered where the intermediate part of the path comes from: /etc/config/hetzner_ddns.conf.
Maybe I missunderstood the concept here? Then it seems I should change the command in hetzner_ddns.openwrt.rc as @carstencodes already did with his sed command.

[carstencodes]

Hi,

thanks for your quick replies.

The permissions should be as restrictive as possible. To me it looks like the process should be started as a user like daemon on various Linux distributions, who is also the owner of those files.

After installation on my OpenWRT, these files belong to root:root. The /var/run and /var/log folders also with do755, hence only root is permitted to add files here.

The infix configuration file parameter might not be the most fortunate, or intuitive way to use multiple files, but if it's changed now, it would break existing behaviour.

Thanks for your explanation @filiparag . I totally agree.

Just for me as a question: Did you install the .ipk package or did you try to create it directly on your OpenWrt machine with the Makefile in the repo's root directory? (Unfortunately I went the first way and created the package)

I installed the .ipkg by downloading it from the github release and installing it via LuCI.

Afterwards I added my token to the /etc/config config file and adjusted the domain / zone.

Unfortunately, due to the permissions of the directory, changing to daemon doesn't work either.

# service hetzner_ddns stop
# service hetzner_ddns start
# service hetzner_ddns info
{
	"hetzner_ddns": {
		"instances": {
			"[hetzner_ddns]": {
				"running": false,
				"command": [
					"/usr/bin/hetzner_ddns",
					"-d"
				],
				"term_timeout": 5,
				"exit_code": 2,
				"limits": {
					"core": "0"
				},
				"respawn": {
					"threshold": 3600,
					"timeout": 5,
					"retry": 5
				},
				"pidfile": "/var/run/hetzner_ddns.pid",
				"user": "daemon"
			}
		}
	}
}

But it starts as root

# service hetzner_ddns stop
# service hetzner_ddns start
# service hetzner_ddns info
{
	"hetzner_ddns": {
		"instances": {
			"[hetzner_ddns]": {
				"running": false,
				"command": [
					"/usr/bin/hetzner_ddns",
					"-d"
				],
				"term_timeout": 5,
				"exit_code": 0,
				"limits": {
					"core": "0"
				},
				"respawn": {
					"threshold": 3600,
					"timeout": 5,
					"retry": 5
				},
				"pidfile": "/var/run/hetzner_ddns.pid",
				"user": "root"
			}
		}
	}
}

This is something I don't like either.

@LLdaniel Let me dig into some documentation, maybe I'll find a solution here as well

[LLdaniel]

Maybe we can do it similar to bind:
https://github.com/openwrt/packages/blob/master/net/bind/files/named.init
Just setting the permissions and owner within the init script?

[carstencodes]

@LLdaniel Great idea.

Please have a look at #22

[carstencodes]

On the other hand: All data is present to add this to a ddns-scripts add-in, but this would include major refactorings

[filiparag]

On the other hand: All data is present to add this to a ddns-scripts add-in, but this would include major refactorings

I am not against supporting OpenWrt's universal format as long as adding it doesn't spill over to issues on other target platforms.

What do you think about adding an intermediate service script that parses, translates ddns-scripts format to configuration format this script understands, and then runs it as a child?

[carstencodes]

Yeah, but as far as I've seen it, the current functions must be split up into more atomic functions and a library-script that is dot-sourced must be created from it. I'm not sure, if this is wise.

[filiparag]

The hetzner_ddns.sh script is sourcable as-is, if the ifs on the bottom get changed a bit, akin to this:

if [ -z "$HETZNER_DDNS_NOEXEC" ]; then
    if [ "$daemon" = '1' ]; then
        # Deamonize and write PID to file
        if touch "/var/run/$self.pid";
        then
            run_ddns &
            echo $! > "/var/run/$self.pid"
        else
            >&2 echo 'unable to daemonize'
            exit 2
        fi
    else
        # Run in foreground
        run_ddns
    fi
fi

And when sourcing the script we add the HETZNER_DDNS_NOEXEC flag:

HETZNER_DDNS_NOEXEC=1 . /usr/local/bin/hetzner_ddns

This is just a quick and dirty modification idea, it should be thought through.

As for more granular functions, they are already segmented enough in my opinion; it's just unfortunate that global variables have to be used for storing state. Maybe IPv4/IPv6 functionality can be split further, but it would add a lot of code duplication noise.

[LLdaniel]

Sorry, that I was a bit quiet about the follow-up discussion, because I have not a really strong feeling to push it into either one or into another direction.
From my perspective I am happy with the .ipk of hetzner_ddns and that you, filiparag, allowed to include the OpenWrt support.

So far I had no chance to have a deeper look into the ddns client from openwrt. Hence I cannot say what are the differences and commonalities.
At the moment it seems for me that both the ddns client and hetzner_ddns have their use case and initially I searched for something specific for Hetzner and came to this project. Maybe I could have also searched for the ddns client in openwrt in the first place and could have configured it accordingly to match the Hetzner API.