From 4edecf3dc1ee680d31396a638e8ae66079cc2dfd Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 25 Jun 2020 23:17:17 -0400 Subject: [PATCH] Turn `fw-banlocal` into a no-op and mark it as deprecated `fw-banlocal` is problematic: * There's not much point to it. Regardless of whether it is enabled, the end result is a DNS record that is not being updated to a useful value. It does cause a warning to be logged, but because it is not enabled by default it doesn't help the poor user who is trying to figure out why they can't reach their machine. By the time they realize that enabling this option would have saved them hours of troubleshooting, they no longer need to enable it because they already know what the problem is. * It's a misnomer: `fw-banlocal` doesn't just filter out local IP addresses from `use=fw`, it also filters them out of all other address sources except `use=ip`. * It doesn't filter out local IPv6 addresses. * The resulting warning ("unable to determine IP address") is misleading. We might want to add a warning whenever a non-global address is discovered (along with an option to silence the warning), but that should be done in a future commit if at all. --- ChangeLog.md | 1 + ddclient | 39 +++++++++++---------------------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 854c9b7..6c67763 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -34,6 +34,7 @@ repository history](https://github.com/ddclient/ddclient/commits/master). * The way ddclient chooses the default for the `use` option has changed. Rather than rely on the default, users should explicitly set the `use` option. + * The `fw-banlocal` option is deprecated and no longer does anything. ## 2020-01-08 v3.9.1 diff --git a/ddclient b/ddclient index f844598..f1452d5 100755 --- a/ddclient +++ b/ddclient @@ -336,7 +336,6 @@ my %variables = ( 'web-skip' => setv(T_STRING,1, 0, 1, '', undef), 'fw' => setv(T_ANY, 0, 0, 1, '', undef), 'fw-skip' => setv(T_STRING,1, 0, 1, '', undef), - 'fw-banlocal' => setv(T_BOOL, 0, 0, 1, 0, undef), 'fw-login' => setv(T_LOGIN, 1, 0, 1, '', undef), 'fw-password' => setv(T_PASSWD,1, 0, 1, '', undef), 'cmd' => setv(T_PROG, 0, 0, 1, '', undef), @@ -376,7 +375,6 @@ my %variables = ( 'web-skip' => setv(T_STRING,0, 0, 1, '', undef), 'fw' => setv(T_ANY, 0, 0, 1, '', undef), 'fw-skip' => setv(T_STRING,0, 0, 1, '', undef), - 'fw-banlocal' => setv(T_BOOL, 0, 0, 1, 0, undef), 'fw-login' => setv(T_LOGIN, 0, 0, 1, '', undef), 'fw-password' => setv(T_PASSWD,0, 0, 1, '', undef), 'cmd' => setv(T_PROG, 0, 0, 1, '', undef), @@ -734,6 +732,7 @@ $variables{'merged'} = merge($variables{'global-defaults'}, # This will hold the processed args. my %opt = (); +$opt{'fw-banlocal'} = sub { warning("'-fw-banlocal' is deprecated and does nothing") }; my @opt = ( "usage: ${program} [options]", @@ -760,7 +759,6 @@ my @opt = ( "", [ "fw", "=s", "-fw address|url : obtain IP address from firewall at 'address'" ], [ "fw-skip", "=s", "-fw-skip pattern : skip any IP addresses before 'pattern' on the firewall address|url" ], - [ "fw-banlocal", "!", "-fw-banlocal : ignore local IP addresses on the firewall address|url" ], [ "fw-login", "=s", "-fw-login login : use 'login' when getting IP from fw" ], [ "fw-password", "=s", "-fw-password secret : use password 'secret' when getting IP from fw" ], "", @@ -792,6 +790,7 @@ my @opt = ( [ "postscript", "", "-postscript : script to run after updating ddclient, has new IP as param" ], [ "query", "!", "-{no}query : print {no} ip addresses and exit" ], + [ "fw-banlocal", "!", "" ], ## deprecated [ "test", "!", "" ], ## hidden [ "geturl", "=s", "" ], ## hidden "", @@ -1080,6 +1079,10 @@ sub parse_assignments { $rest =~ s/^\s+//; ($name, $value, $rest) = parse_assignment($rest); if (defined $name) { + if ($name eq 'fw-banlocal') { + warning("'fw-banlocal' is deprecated and does nothing"); + next; + } $variables{$name} = $value; } else { last; @@ -1308,6 +1311,10 @@ sub init_config { my %options = (); foreach my $opt (split_by_comma($opt{'options'})) { my ($name, $var) = split /\s*=\s*/, $opt; + if ($name eq 'fw-banlocal') { + warning("'fw-banlocal' is deprecated and does nothing"); + next; + } $options{$name} = $var; } ## determine hosts specified with -host @@ -1436,7 +1443,7 @@ sub process_args { push @spec, $key . $specifier; ## define the default value which can be overwritten later - $opt{$key} = undef; + $opt{$key} = undef unless exists($opt{$key}); next unless $arg_usage; @@ -2167,28 +2174,6 @@ sub un_zero_pad { return join('.', @out_str); } ###################################################################### -## filter_local -###################################################################### -sub filter_local { - my $in_ip = shift(@_); - - if ($in_ip eq '0.0.0.0') { - return $in_ip; - } - - my @guess_local = ( - '^10\.', - '^172\.(?:1[6-9]|2[0-9]|3[01])\.', - '^192\.168' - ); - foreach my $block (@guess_local) { - if ($in_ip =~ /$block/) { - return '0.0.0.0'; - } - } - return $in_ip; -} -###################################################################### ## get_ip ###################################################################### sub get_ip { @@ -2299,10 +2284,8 @@ sub get_ip { # no need to parse $reply } elsif ($ip = ipv4_match($reply)) { $ip = un_zero_pad($ip); - $ip = filter_local($ip) if opt('fw-banlocal', $h); } elsif ($ip = ipv6_match($reply)) { $ip = un_zero_pad($ip); - $ip = filter_local($ip) if opt('fw-banlocal', $h); } else { warning("found neither ipv4 nor ipv6 address"); }