[Buildroot] [PATCH] inadyn: fix init script and default config file

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Thomas Petazzoni]

This commit does a number of fixes to the inadyn package to make it
work properly "out of the box":

 * inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path
   in the init script

 * Use "echo -n" for the Starting and Stopping messages, so that the
   OK / FAIL stay on the same line.

 * There is no need to pass the inadyn binary path during the stop
   sequence, as long as we pass the appropriate -p option with the
   path to the PID file created by inadyn, so we do this.

 * Pass the -q option to the start sequence, since it's passed in the
   stop sequence.

 * Fix the configuration file to use an existing dyndns_system and
   avoid a failure at startup.

Cc: Alex Suykov <alex.suykov@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/inadyn/S70inadyn   | 10 +++++-----
 package/inadyn/inadyn.conf |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/inadyn/S70inadyn b/package/inadyn/S70inadyn
index b20048c..b2aaa0a 100644
--- a/package/inadyn/S70inadyn
+++ b/package/inadyn/S70inadyn
@@ -14,15 +14,15 @@ VR_INADYN=/var/run/inadyn
 
 case "$1" in
 	start)
-		echo "Starting inadyn: "
-		start-stop-daemon -S -x /usr/bin/inadyn
+		echo -n "Starting inadyn: "
+		start-stop-daemon -q -S -p ${VR_INADYN}/inadyn.pid -x /usr/sbin/inadyn
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
-		echo  "Stopping inadyn: "
-		start-stop-daemon -q -K -x /usr/bin/inadyn
+		echo -n "Stopping inadyn: "
+		start-stop-daemon -q -K -p ${VR_INADYN}/inadyn.pid
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		rm -f /var/run/inadyn/inadyn.pid
+		rm -f ${VR_INADYN}/inadyn.pid
 		;;
 	restart)
 		"$0" stop
diff --git a/package/inadyn/inadyn.conf b/package/inadyn/inadyn.conf
index b5877f7..93ad0b5 100644
--- a/package/inadyn/inadyn.conf
+++ b/package/inadyn/inadyn.conf
@@ -5,7 +5,7 @@ background
 update_period_sec 600 # Check for a new IP every 600 seconds
 username test		# replace 'test' with your username
 password test		# replace 'test' with your password
-dyndns_system dyndns at dyndns.org   # replace w/ your provider
+dyndns_system default at dyndns.org   # replace w/ your provider
 
 #  uncomment the alias statement below to test it on your system
 alias test.homeip.net
-- 
2.4.5

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Arnout Vandecappelle]

On 07/05/15 17:12, Thomas Petazzoni wrote:
> This commit does a number of fixes to the inadyn package to make it
> work properly "out of the box":
> 
>  * inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path
>    in the init script
> 
>  * Use "echo -n" for the Starting and Stopping messages, so that the
>    OK / FAIL stay on the same line.
> 
>  * There is no need to pass the inadyn binary path during the stop
>    sequence, as long as we pass the appropriate -p option with the
>    path to the PID file created by inadyn, so we do this.

 Actually there is a reason to pass it: if inadyn has already died and another
process has taken over the PID, then start-stop-daemon can check that both the
PID and the process executable match. So keep it (if it works).

 With that fixed:
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> 
>  * Pass the -q option to the start sequence, since it's passed in the
>    stop sequence.
> 
>  * Fix the configuration file to use an existing dyndns_system and
>    avoid a failure at startup.
> 
> Cc: Alex Suykov <alex.suykov@gmail.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/inadyn/S70inadyn   | 10 +++++-----
>  package/inadyn/inadyn.conf |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/package/inadyn/S70inadyn b/package/inadyn/S70inadyn
> index b20048c..b2aaa0a 100644
> --- a/package/inadyn/S70inadyn
> +++ b/package/inadyn/S70inadyn
> @@ -14,15 +14,15 @@ VR_INADYN=/var/run/inadyn
>  
>  case "$1" in
>  	start)
> -		echo "Starting inadyn: "
> -		start-stop-daemon -S -x /usr/bin/inadyn
> +		echo -n "Starting inadyn: "
> +		start-stop-daemon -q -S -p ${VR_INADYN}/inadyn.pid -x /usr/sbin/inadyn
>  		[ $? = 0 ] && echo "OK" || echo "FAIL"
>  		;;
>  	stop)
> -		echo  "Stopping inadyn: "
> -		start-stop-daemon -q -K -x /usr/bin/inadyn
> +		echo -n "Stopping inadyn: "
> +		start-stop-daemon -q -K -p ${VR_INADYN}/inadyn.pid
>  		[ $? = 0 ] && echo "OK" || echo "FAIL"
> -		rm -f /var/run/inadyn/inadyn.pid
> +		rm -f ${VR_INADYN}/inadyn.pid
>  		;;
>  	restart)
>  		"$0" stop
> diff --git a/package/inadyn/inadyn.conf b/package/inadyn/inadyn.conf
> index b5877f7..93ad0b5 100644
> --- a/package/inadyn/inadyn.conf
> +++ b/package/inadyn/inadyn.conf
> @@ -5,7 +5,7 @@ background
>  update_period_sec 600 # Check for a new IP every 600 seconds
>  username test		# replace 'test' with your username
>  password test		# replace 'test' with your password
> -dyndns_system dyndns at dyndns.org   # replace w/ your provider
> +dyndns_system default at dyndns.org   # replace w/ your provider
>  
>  #  uncomment the alias statement below to test it on your system
>  alias test.homeip.net
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Gustavo Zacarias]

On 05/07/15 12:12, Thomas Petazzoni wrote:

>   * Fix the configuration file to use an existing dyndns_system and
>     avoid a failure at startup.

You definitely don't want this, if it's not configured and requires 
configuration for any sane usage let it be so.
You don't want to do this kind of thing by default:
https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse

Also trusting that 'background' is in the config doesn't seem foolproof 
either, dropping it and adding -b to the initscript sounds better to me.

Regards.

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Thomas Petazzoni]

Gustavo,

On Tue, 14 Jul 2015 11:24:50 -0300, Gustavo Zacarias wrote:
> 
> 
> On 05/07/15 12:12, Thomas Petazzoni wrote:
> 
> >   * Fix the configuration file to use an existing dyndns_system and
> >     avoid a failure at startup.
> 
> You definitely don't want this, if it's not configured and requires 
> configuration for any sane usage let it be so.
> You don't want to do this kind of thing by default:
> https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse

Hu? What's the relation between NTP server misuse and Inadyn which is a
tool that updates a DynDNS entry with your IP address? Am I missing
something?

> Also trusting that 'background' is in the config doesn't seem foolproof 
> either, dropping it and adding -b to the initscript sounds better to me.

But then if 'background' is in the config file, we will background it
twice. Since inadyn generates its own PID file, it might be OK, but
it's not that nice either.

I think our init scripts are anyway done to work fine with our default
configuration files. If you change the configuration file, you have to
be ready to adjust init scripts as well I'd say, no?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Gustavo Zacarias]

On 14/07/15 12:19, Thomas Petazzoni wrote:

>> You definitely don't want this, if it's not configured and requires
>> configuration for any sane usage let it be so.
>> You don't want to do this kind of thing by default:
>> https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse
>
> Hu? What's the relation between NTP server misuse and Inadyn which is a
> tool that updates a DynDNS entry with your IP address? Am I missing
> something?

If you ship a config that starts up inadyn with user=test pass=test with 
some X dyndns service that could potentially flood said service/server 
with a bogus/useless setting.
It might not matter with small usage scenarios, but if a firmware image 
is shipped for some networking appliance which doesn't sanitize the 
config then you've got a similar scenario to the NTP flood.

> But then if 'background' is in the config file, we will background it
> twice. Since inadyn generates its own PID file, it might be OK, but
> it's not that nice either.
>
> I think our init scripts are anyway done to work fine with our default
> configuration files. If you change the configuration file, you have to
> be ready to adjust init scripts as well I'd say, no?

I haven't tested the detail, but -b is probably the same as "background" 
in the config and won't background twice (i'm talking of the inadyn -b 
option, not backgrounding from start-stop-daemon).
Regards.

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Thomas Petazzoni]

Dear Gustavo Zacarias,

On Tue, 14 Jul 2015 12:24:06 -0300, Gustavo Zacarias wrote:

> If you ship a config that starts up inadyn with user=test pass=test with 
> some X dyndns service that could potentially flood said service/server 
> with a bogus/useless setting.
> It might not matter with small usage scenarios, but if a firmware image 
> is shipped for some networking appliance which doesn't sanitize the 
> config then you've got a similar scenario to the NTP flood.

Ah, yes, I see. So maybe we should instead have:

ENABLED="no" 

in /etc/default/inadyn

and in the SXXinadyn, if ENABLED=="no", do something like:

Starting inadyn: SKIPPED

or something like that.

Thoughts?

> > But then if 'background' is in the config file, we will background it
> > twice. Since inadyn generates its own PID file, it might be OK, but
> > it's not that nice either.
> >
> > I think our init scripts are anyway done to work fine with our default
> > configuration files. If you change the configuration file, you have to
> > be ready to adjust init scripts as well I'd say, no?
> 
> I haven't tested the detail, but -b is probably the same as "background" 
> in the config and won't background twice (i'm talking of the inadyn -b 
> option, not backgrounding from start-stop-daemon).

Ah, you're talking about inadyn -b option, while I was thinking of the
start-stop-daemon option. Then it definitely makes sense, and I can get
rid of the background option in the example config file.

Sounds OK?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] inadyn: fix init script and default config file

[Gustavo Zacarias]

On 14/07/15 12:59, Thomas Petazzoni wrote:

> Ah, yes, I see. So maybe we should instead have:
>
> ENABLED="no"
>
> in /etc/default/inadyn
>
> and in the SXXinadyn, if ENABLED=="no", do something like:
>
> Starting inadyn: SKIPPED
>
> or something like that.
>
> Thoughts?

Yes, the initscripts enable/disable logic is pending (my fault probably, 
though i didn't follow the conclusions with detail).

> Ah, you're talking about inadyn -b option, while I was thinking of the
> start-stop-daemon option. Then it definitely makes sense, and I can get
> rid of the background option in the example config file.
>
> Sounds OK?

Yup, it's mostly covering for user omission since it costs nothing.
With the addition of -- -b :)
It also makes the systemd service more resilient, since it normally 
doesn't expect a background (at least via BR default config).
Regards.