Merge pull request #664 from rhansen/cisco

Move `--use=cisco` and `--use=cisco-asa` to `%builtinfw`

Makefile.am

@@ -62,6 +62,7 @@ AM_PL_LOG_FLAGS = -Mstrict -w \
 	-I'$(abs_top_srcdir)'/t/lib \
 	-MDevel::Autoflush
 handwritten_tests = \
+	t/builtinfw_query.pl \
 	t/get_ip_from_if.pl \
 	t/geturl_connectivity.pl \
 	t/is-and-extract-ipv4.pl \

ddclient.in

@@ -189,6 +189,32 @@ our %builtinweb = (
     'nsupdate.info-ipv6' => {'url' => 'https://ipv6.nsupdate.info/myip'},
     'zoneedit' => {'url' => 'https://dynamic.zoneedit.com/checkip.html'},
 );
+
+sub query_cisco {
+    my ($h, $asa, $v4) = @_;
+    warning("'--if' is deprecated for '--usev4=ifv4; use '--ifv4' instead")
+        if ($v4 && !defined(opt('ifv4')) && defined(opt('if', $h)));
+    warning("'--fw' is deprecated for '--usev4=fwv4; use '--fwv4' instead")
+        if ($v4 && !defined(opt('fwv4')) && defined(opt('fw', $h)));
+    my $if = ($v4 ? opt('ifv4', $h) : undef) // opt('if', $h);
+    my $fw = ($v4 ? opt('fwv4', $h) : undef) // opt('fw', $h);
+    # Convert slashes to protected value "\/"
+    $if =~ s%\/%\\\/%g;
+    # Protect special HTML characters (like '?')
+    $if =~ s/([\?&= ])/sprintf("%%%02x", ord($1))/ge;
+    my $url = ($asa)
+        ? "https://$fw/exec/show%20interface%20$if"
+        : "http://$fw/level/1/exec/show/ip/interface/brief/$if/CR";
+    my $reply = geturl(
+        url => $url,
+        login => opt('fw-login', $h),
+        password => opt('fw-password', $h),
+        ignore_ssl_option => 1,
+        ssl_validate => opt('fw-ssl-validate', $h),
+    ) // '';
+    return ($url, $reply);
+}
+
 our %builtinfw = (
     '2wire' => {
         'name' => '2Wire 1701HG Gateway',
@@ -230,6 +256,18 @@ our %builtinfw = (
         'url'  => '/shell/show+ip+interfaces',
         'skip' => '.*inet',
     },
+    'cisco' => {
+        'name' => 'Cisco FW',
+        'query' => sub { return query_cisco($_[0], 0, 0); },
+        'queryv4' => sub { return query_cisco($_[0], 0, 1); },
+        'help' => sub { return " at the host given by --fw$_[0]=<host> and interface given by --if$_[0]=<interface>"; },
+    },
+    'cisco-asa' => {
+        'name' => 'Cisco ASA',
+        'query' => sub { return query_cisco($_[0], 1, 0); },
+        'queryv4' => sub { return query_cisco($_[0], 1, 1); },
+        'help' => sub { return " at the host given by --fw$_[0]=<host> and interface given by --if$_[0]=<interface>"; },
+    },
     'dlink-524' => {
         'name' => 'D-Link DI-524',
         'url'  => '/st_device.html',
@@ -435,15 +473,16 @@ my %ip_strategies = (
     'fw'        => ": deprecated, see '--usev4=fwv4' and '--usev6=fwv6'",
     'if'        => ": deprecated, see '--usev4=ifv4' and '--usev6=ifv6'",
     'cmd'       => ": deprecated, see '--usev4=cmdv4' and '--usev6=cmdv6'",
-    'cisco'     => ": deprecated, see '--usev4=cisco'",
+    map({
-    'cisco-asa' => ": deprecated, see '--usev4=cisco-asa'",
+        my $fw = $builtinfw{$_};
-    map({ $_ => ": deprecated, see '--usev4=$_'"; } keys(%builtinfw)),
+        $_ => ": deprecated, see '--usev4=$_'" .
+            (defined($fw->{queryv6}) ? " and '--usev6=$_'" : '');
+    } keys(%builtinfw)),
 );
 
 sub ip_strategies_usage {
     return map({ sprintf("    --use=%-22s %s.", $_, $ip_strategies{$_}) }
-               'disabled', 'no', 'ip', 'web', 'if', 'cmd', 'fw',
+               'disabled', 'no', 'ip', 'web', 'if', 'cmd', 'fw', sort(keys(%builtinfw)));
-               sort('cisco', 'cisco-asa', keys(%builtinfw)));
 }
 
 my %ipv4_strategies = (
@@ -455,7 +494,9 @@ my %ipv4_strategies = (
     'fwv4'        => ": obtain IPv4 from the URL given by --fwv4=<URL>",
     map({
         my $fw = $builtinfw{$_};
-        $_ => ": obtain IPv4 from $fw->{'name'} at the host or URL given by --fwv4=<host|URL>";
+        $_ => defined($fw->{queryv4})
+            ? ": obtain IPv4 from $fw->{name}@{[($fw->{help} // sub {})->('v4') // '']}"
+            : ": obtain IPv4 from $fw->{name} at the host or URL given by --fwv4=<host|URL>";
     } keys(%builtinfw)),
 );
 sub ipv4_strategies_usage {
@@ -475,11 +516,17 @@ my %ipv6_strategies = (
     'cmdv6'       => ": obtain IPv6 from the command given by --cmdv6=<command>",
     'cmd'         => ": deprecated, use '--usev6=cmdv6'",
     'fwv6'        => ": obtain IPv6 from the URL given by --fwv6=<URL>",
+    map({
+        my $fw = $builtinfw{$_};
+        defined($fw->{queryv6})
+            ? ($_ => ": obtain IPv6 from $fw->{name}@{[($fw->{help} // sub {})->('v6') // '']}")
+            : ();
+    } keys(%builtinfw)),
 );
 sub ipv6_strategies_usage {
     return map({ sprintf("    --usev6=%-22s %s.", $_, $ipv6_strategies{$_}) }
                'disabled', 'no', 'ipv6', 'ip', 'webv6', 'web', 'ifv6', 'if', 'cmdv6', 'cmd',
-               'fwv6');
+               'fwv6', sort(map({exists($ipv6_strategies{$_}) ? ($_) : ()} keys(%builtinfw))));
 }
 
 sub setv {
@@ -2841,65 +2888,23 @@ sub get_ip {
             ) // '';
         }
 
-    } elsif (($use eq 'cisco')) {
-        # Stuff added to support Cisco router ip http daemon
-        # User fw-login should only have level 1 access to prevent
-        # password theft.  This is pretty harmless.
-        my $queryif = opt('if', $h);
-        $skip = opt('fw-skip', $h);
-
-        # Convert slashes to protected value "\/"
-        $queryif =~ s%\/%\\\/%g;
-
-        # Protect special HTML characters (like '?')
-        $queryif =~ s/([\?&= ])/sprintf("%%%02x", ord($1))/ge;
-
-        $url = "http://" . opt('fw', $h) . "/level/1/exec/show/ip/interface/brief/${queryif}/CR";
-        $reply = geturl(
-            url => $url,
-            login => opt('fw-login', $h),
-            password => opt('fw-password', $h),
-            ignore_ssl_option => 1,
-            ssl_validate => opt('fw-ssl-validate', $h),
-        ) // '';
-        $arg = $url;
-
-    } elsif (($use eq 'cisco-asa')) {
-        # Stuff added to support Cisco ASA ip https daemon
-        # User fw-login should only have level 1 access to prevent
-        # password theft.  This is pretty harmless.
-        my $queryif = opt('if', $h);
-        $skip = opt('fw-skip', $h);
-
-        # Convert slashes to protected value "\/"
-        $queryif =~ s%\/%\\\/%g;
-
-        # Protect special HTML characters (like '?')
-        $queryif =~ s/([\?&= ])/sprintf("%%%02x", ord($1))/ge;
-
-        $url = "https://" . opt('fw', $h) . "/exec/show%20interface%20${queryif}";
-        $reply = geturl(
-            url => $url,
-            login => opt('fw-login', $h),
-            password => opt('fw-password', $h),
-            ignore_ssl_option => 1,
-            ssl_validate => opt('fw-ssl-validate', $h),
-        ) // '';
-        $arg = $url;
-
     } elsif ($use eq 'disabled') {
         ## This is a no-op... Do not get an IP address for this host/service
         $reply = '';
 
-    } else {
+    } elsif ($use eq 'fw' || defined(my $fw = $builtinfw{$use})) {
         # Note that --use=firewallname uses --fw=arg, not --firewallname=arg.
         $arg = opt('fw', $h) // '';
         $url = $arg;
         $skip = opt('fw-skip', $h);
-
+        if ($fw) {
-        if (exists $builtinfw{$use}) {
+            $skip //= $fw->{'skip'};
-            $skip //= $builtinfw{$use}->{'skip'};
+            if (defined(my $query = $fw->{'query'})) {
-            $url  = "http://${url}" . $builtinfw{$use}->{'url'} unless $url =~ /\//;
+                $url = undef;
+                ($arg, $reply) = $query->($h);
+            } else {
+                $url = "http://$url$fw->{'url'}" unless $url =~ /\//;
+            }
         }
 
         if ($url) {
@@ -2911,6 +2916,9 @@ sub get_ip {
                 ssl_validate => opt('fw-ssl-validate', $h),
             ) // '';
         }
+
+    } else {
+        warning("ignoring unsupported '--use=$use'");
     }
     if (!defined $reply) {
         $reply = '';
@@ -3284,42 +3292,11 @@ sub get_ipv4 {
             ) // '';
         }
 
-    } elsif ($usev4 eq 'cisco' || $usev4 eq 'cisco-asa') {
-        # Stuff added to support Cisco router ip http or ASA https daemon
-        # User fw-login should only have level 1 access to prevent
-        # password theft.  This is pretty harmless.
-        warning("'--if' is deprecated for '--usev4=$usev4'; use '--ifv4' instead")
-            if (!defined(opt('ifv4', $h)) && defined(opt('if', $h)));
-        warning("'--fw' is deprecated for '--usev4=$usev4'; use '--fwv4' instead")
-            if (!defined(opt('fwv4', $h)) && defiend(opt('fw', $h)));
-        warning("'--fw-skip' is deprecated for '--usev4=$usev4'; use '--fwv4-skip' instead")
-            if (!defined(opt('fwv4-skip', $h)) && defined(opt('fw-skip', $h)));
-        my $queryif = opt('ifv4', $h) // opt('if', $h);
-        $skip = opt('fwv4-skip', $h) // opt('fw-skip', $h);
-        # Convert slashes to protected value "\/"
-        $queryif =~ s%\/%\\\/%g;
-        # Protect special HTML characters (like '?')
-        $queryif =~ s/([\?&= ])/sprintf("%%%02x", ord($1))/ge;
-        if ($usev4 eq 'cisco') {
-            $url = "http://" . (opt('fwv4', $h) // opt('fw', $h)) . "/level/1/exec/show/ip/interface/brief/${queryif}/CR";
-        } else {
-            $url = "https://" . (opt('fwv4', $h) // opt('fw', $h))  . "/exec/show%20interface%20${queryif}";
-        }
-        $arg = $url;
-        $reply = geturl(
-            url => $url,
-            login => opt('fw-login', $h),
-            password => opt('fw-password', $h),
-            ipversion => 4, # when using a URL to find IPv4 address we should force use of IPv4
-            ignore_ssl_option => 1,
-            ssl_validate => opt('fw-ssl-validate', $h),
-        ) // '';
-
     } elsif ($usev4 eq 'disabled') {
         ## This is a no-op... Do not get an IPv4 address for this host/service
         $reply = '';
 
-    } else {
+    } elsif ($usev4 eq 'fwv4' || defined(my $fw = $builtinfw{$usev4})) {
         warning("'--fw' is deprecated for '--usev4=$usev4'; use '--fwv4' instead")
             if (!defined(opt('fwv4', $h)) && defined(opt('fw', $h)));
         warning("'--fw-skip' is deprecated for '--usev4=$usev4'; use '--fwv4-skip' instead")
@@ -3328,10 +3305,14 @@ sub get_ipv4 {
         $arg = opt('fwv4', $h) // opt('fw', $h) // '';
         $url = $arg;
         $skip = opt('fwv4-skip', $h) // opt('fw-skip', $h);
-
+        if ($fw) {
-        if (exists $builtinfw{$usev4}) {
+            $skip //= $fw->{'skip'};
-            $skip //= $builtinfw{$usev4}->{'skip'};
+            if (defined(my $query = $fw->{'queryv4'})) {
-            $url  = "http://${url}" . $builtinfw{$usev4}->{'url'} unless $url =~ /\//;
+                $url = undef;
+                ($arg, $reply) = $query->($h);
+            } else {
+                $url = "http://$url$fw->{'url'}" unless $url =~ /\//;
+            }
         }
         if ($url) {
             $reply = geturl(
@@ -3343,6 +3324,9 @@ sub get_ipv4 {
                 ssl_validate => opt('fw-ssl-validate', $h),
             ) // '';
         }
+
+    } else {
+        warning("ignoring unsupported '--usev4=$usev4'");
     }
 
     ## Set to loopback address if no text set yet
@@ -3436,9 +3420,17 @@ sub get_ipv6 {
     } elsif ($usev6 eq 'disabled') {
         $reply = '';
 
+    } elsif ($usev6 eq 'fwv6' || defined(my $fw = $builtinfw{$usev6})) {
+        $skip = opt('fwv6-skip', $h) // $fw->{'skip'};
+        if ($fw && defined(my $query = $fw->{'queryv6'})) {
+            $skip //= $fw->{'skip'};
+            ($arg, $reply) = $query->($h);
+        } else {
+            warning("'--usev6=%s' is not implemented and does nothing", $usev6);
+        }
+
     } else {
-        warning("'--usev6=%s' is not implemented and does nothing", $usev6);
+        warning("ignoring unsupported '--usev6=$usev6'");
-        $reply = '';
 
     }
 

t/builtinfw_query.pl

@@ -0,0 +1,93 @@
+use Test::More;
+eval { require Test::MockModule; } or plan(skip_all => $@);
+SKIP: { eval { require Test::Warnings; } or skip($@, 1); }
+eval { require 'ddclient'; } or BAIL_OUT($@);
+
+my $debug_msg;
+my $module = Test::MockModule->new('ddclient');
+# Note: 'mock' is used instead of 'redefine' because 'redefine' is not available in the versions of
+# Test::MockModule distributed with old Debian and Ubuntu releases.
+$module->mock('debug', sub {
+    my $msg = sprintf(shift, @_);
+    return unless ($msg =~ qr/^get_ip(v[46])?:/);
+    BAIL_OUT("debug already called") if defined($debug_msg);
+    $debug_msg = $msg;
+});
+my $got_host;
+my $builtinfw = 't/builtinfw_query.pl';
+$ddclient::builtinfw{$builtinfw} = {
+    name => 'dummy device for testing',
+    query => sub {
+        ($got_host) = @_;
+        return ($got_host, "192.0.2.1 skip1 192.0.2.2 skip2 192.0.2.3");
+    },
+    queryv4 => sub {
+        ($got_host) = @_;
+        return ($got_host, "192.0.2.4 skip1 192.0.2.5 skip3 192.0.2.6");
+    },
+    queryv6 => sub {
+        ($got_host) = @_;
+        return ($got_host, "2001:db8::1 skip1 2001:db8::2 skip4 2001:db8::3");
+    },
+};
+%ddclient::builtinfw if 0;  # suppress spurious warning "Name used only once: possible typo"
+
+my @test_cases = (
+    {
+        desc => 'query',
+        getip => \&ddclient::get_ip,
+        useopt => 'use',
+        cfgxtra => {},
+        want => '192.0.2.2',
+    },
+    {
+        desc => 'queryv4',
+        getip => \&ddclient::get_ipv4,
+        useopt => 'usev4',
+        cfgxtra => {'fwv4-skip' => 'skip3'},
+        want => '192.0.2.6',
+    },
+    {
+        desc => 'queryv4 with fw-skip fallback',
+        getip => \&ddclient::get_ipv4,
+        useopt => 'usev4',
+        cfgxtra => {},
+        want => '192.0.2.5',
+    },
+    {
+        desc => 'queryv6',
+        getip => \&ddclient::get_ipv6,
+        useopt => 'usev6',
+        cfgxtra => {'fwv6-skip' => 'skip4'},
+        want => '2001:db8::3',
+    },
+    {
+        # Support for --usev6=<builtin> wasn't added until after --fwv6-skip was added, so fallback
+        # to the deprecated --fw-skip option was never needed.
+        desc => 'queryv6 ignores fw-skip',
+        getip => \&ddclient::get_ipv6,
+        useopt => 'usev6',
+        cfgxtra => {},
+        want => '2001:db8::1',
+    },
+);
+
+for my $tc (@test_cases) {
+    subtest $tc->{desc} => sub {
+        my $h = "t/builtinfw_query.pl $tc->{desc}";
+        $ddclient::config{$h} = {
+            $tc->{useopt} => $builtinfw,
+            'fw-skip' => 'skip1',
+            %{$tc->{cfgxtra}},
+        };
+        %ddclient::config if 0;  # suppress spurious warning "Name used only once: possible typo"
+        undef($debug_msg);
+        undef($got_host);
+        my $got = $tc->{getip}($builtinfw, $h);
+        is($got_host, $h, "host is passed through");
+        is($got, $tc->{want}, "returned IP matches");
+        like($debug_msg, qr/\b\Q$h\E\b/, "returned arg is properly handled");
+    };
+}
+
+done_testing();