check_ssh: Handle non-alpha software versions

This patch fixes a bug where we would reject version control strings
that do not contain letters, because the assumption is made that they
always do. This is not required by the RFC however, and there exist
implementations that do not contain letters.

I've also added a few references to the RFC to make the process of
parsing the control string more apparent.

This fixes op5#8716 (https://bugs.op5.com/view.php?id=8716)

Signed-off-by: Anton Lofgren <alofgren@op5.com>

2 files changed, 70 insertions, 32 deletions

diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c
index 8a3abb03..b4bfab45 100644
--- a/plugins/check_ssh.c
+++ b/plugins/check_ssh.c
@@ -278,11 +278,35 @@ ssh_connect (char *haddr, int hport, char *remote_version, char *remote_protocol
                 printf("SSH CRITICAL - No version control string received");
                 exit(STATE_CRITICAL);
         }
+        /*
+         * "When the connection has been established, both sides MUST send an
+         * identification string.  This identification string MUST be
+         *
+         * SSH-protoversion-softwareversion SP comments CR LF"
+         *              - RFC 4253:4.2
+         */
         strip (version_control_string);
         if (verbose)
                 printf ("%s\n", version_control_string);
         ssh_proto = version_control_string + 4;
-        ssh_server = ssh_proto + strspn (ssh_proto, "-0123456789.");
+
+        /*
+         * We assume the protoversion is of the form Major.Minor, although
+         * this is not _strictly_ required. See
+         *
+         * "Both the 'protoversion' and 'softwareversion' strings MUST consist of
+         * printable US-ASCII characters, with the exception of whitespace
+         * characters and the minus sign (-)"
+         *              - RFC 4253:4.2
+         * and,
+         *
+         * "As stated earlier, the 'protoversion' specified for this protocol is
+         * "2.0".  Earlier versions of this protocol have not been formally
+         * documented, but it is widely known that they use 'protoversion' of
+         * "1.x" (e.g., "1.5" or "1.3")."
+         *              - RFC 4253:5
+         */
+        ssh_server = ssh_proto + strspn (ssh_proto, "0123456789.") + 1; /* (+1 for the '-' separating protoversion from softwareversion) */
 
         /* If there's a space in the version string, whatever's after the space is a comment
          * (which is NOT part of the server name/version)*/
diff --git a/plugins/t/check_ssh.t b/plugins/t/check_ssh.t
index 3e1824df..7df62651 100644
--- a/plugins/t/check_ssh.t
+++ b/plugins/t/check_ssh.t
@@ -8,10 +8,13 @@ use strict;
 use Test::More;
 use NPTest;
 
+my $res;
+
 # Required parameters
 my $ssh_host           = getTestParameter("NP_SSH_HOST",
                                           "A host providing SSH service",
                                           "localhost");
+
 my $host_nonresponsive = getTestParameter("NP_HOST_NONRESPONSIVE",
                                           "The hostname of system not responsive to network requests",
                                           "10.0.0.1" );
@@ -20,13 +23,37 @@ my $hostname_invalid   = getTestParameter("NP_HOSTNAME_INVALID",
                                           "An invalid (not known to DNS) hostname",
                                           "nosuchhost" );
 
-my $res;
 
+plan tests => 14 + 6;
 
-plan tests => 18;
 SKIP: {
+        skip "SSH_HOST must be defined", 6 unless $ssh_host;
+        my $result = NPTest->testCmd(
+                "./check_ssh -H $ssh_host"
+                );
+        cmp_ok($result->return_code, '==', 0, "Exit with return code 0 (OK)");
+        like($result->output, '/^SSH OK - /', "Status text if command returned none (OK)");
 
-        skip "No netcat available", 12 unless (system("which nc > /dev/null") == 0);
+
+        $result = NPTest->testCmd(
+                "./check_ssh -H $host_nonresponsive -t 2"
+                );
+        cmp_ok($result->return_code, '==', 2, "Exit with return code 0 (OK)");
+        like($result->output, '/^CRITICAL - Socket timeout after 2 seconds/', "Status text if command returned none (OK)");
+
+
+
+        $result = NPTest->testCmd(
+                "./check_ssh -H $hostname_invalid -t 2"
+                );
+        cmp_ok($result->return_code, '==', 3, "Exit with return code 0 (OK)");
+        like($result->output, '/^check_ssh: Invalid hostname/', "Status text if command returned none (OK)");
+
+
+}
+SKIP: {
+
+        skip "No netcat available", 14 unless (system("which nc > /dev/null") == 0);
 
         my $nc_flags = "-l 5003 -i 1";
         #A valid protocol version control string has the form
@@ -41,6 +68,13 @@ SKIP: {
         like( $res->output, '/^SSH OK - nagiosplug.ssh.0.1 \(protocol 2.0\)/', "Output OK");
         close NC;
 
+        open(NC, "echo 'SSH-2.0-3.2.9.1' | nc ${nc_flags}|");
+        sleep 1;
+        $res = NPTest->testCmd( "./check_ssh -H localhost -p 5003" );
+        cmp_ok( $res->return_code, "==", 0, "Got SSH protocol version control string with non-alpha softwareversion string");
+        like( $res->output, '/^SSH OK - 3.2.9.1 \(protocol 2.0\)/', "Output OK for non-alpha softwareversion string");
+        close NC;
+
         open(NC, "echo 'SSH-2.0-nagiosplug.ssh.0.1 this is a comment' | nc ${nc_flags} |");
         sleep 1;
         $res = NPTest->testCmd( "./check_ssh -H localhost -p 5003 -r nagiosplug.ssh.0.1" );
@@ -48,7 +82,6 @@ SKIP: {
         like( $res->output, '/^SSH OK - nagiosplug.ssh.0.1 \(protocol 2.0\)/', "Output OK");
         close NC;
 
-
         open(NC, "echo 'SSH-' | nc ${nc_flags}|");
         sleep 1;
         $res = NPTest->testCmd( "./check_ssh -H localhost -p 5003" );
@@ -72,36 +105,17 @@ SKIP: {
 
 
         #RFC 4253 permits servers to send any number of data lines prior to sending the protocol version control string
-        open(NC, "echo 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n
-                BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB\n
-                CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC\n
-                DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD\n
-                Some\nPrepended\nData\nLines\nSSH-2.0-nagiosplug.ssh.0.2' | nc ${nc_flags}|");
+        open(NC, "{ echo 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'; sleep 1;
+                echo 'BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB'; sleep 1;
+                echo 'CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC'; sleep 1;
+                echo 'DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD'; sleep 1;
+                printf 'EEEEEEEEEEEEEEEEEE'; sleep 1;
+                printf 'EEEEEEEEEEEEEEEEEE\n'; sleep 1;
+                echo 'Some\nPrepended\nData\nLines\n'; sleep 1;
+                echo 'SSH-2.0-nagiosplug.ssh.0.2';} | nc ${nc_flags}|");
         sleep 1;
         $res = NPTest->testCmd( "./check_ssh -H localhost -p 5003" );
         cmp_ok( $res->return_code, '==', 0, "Got delayed SSH protocol version control string");
         like( $res->output, '/^SSH OK - nagiosplug.ssh.0.2 \(protocol 2.0\)/', "Output OK");
         close NC;
 }
-
-SKIP {
-        skip "SSH_HOST must be defined", 6 unless $ssh_host;
-        $res = NPTest->testCmd(
-            "./check_ssh -H $ssh_host"
-            );
-        cmp_ok($result->return_code, '==', 0, "Exit with return code 0 (OK)");
-        like($result->output, '/^SSH OK - /', "Status text if command returned none (OK)");
-
-        $res = NPTest->testCmd(
-            "./check_ssh -H $host_nonresponsive -t 2"
-            );
-        cmp_ok($result->return_code, '==', 2, "Exit with return code 2 (CRITICAL)");
-        like($result->output, '/^CRITICAL - Socket timeout after 2 seconds/', "Status text if command returned none (OK)");
-
-        $res = NPTest->testCmd(
-            "./check_ssh -H $hostname_invalid -t 2"
-            );
-        cmp_ok($result->return_code, '==', 3, "Exit with return code 3 (UNKNOWN)");
-        like($result->output, '/^check_ssh: Invalid hostname/', "Status text if command returned none (OK)");
-
-}