From 380390c4b6dca9bda52707807b3ee577f758b107 Mon Sep 17 00:00:00 2001 From: Jon Simons Date: Tue, 11 Jul 2017 19:34:18 -0400 Subject: misc: relax fatal errors in ssh_analyze_banner Relax the cases where `ssh_analyze_banner` fails to extract a major and minor version from banners which appear like OpenSSH banners. Update the tests to demonstrate that now a banner as might be sent by `ssh-keyscan(1)` ("SSH-2.0-OpenSSH-keyscan") no longer returns failure. Signed-off-by: Jon Simons Reviewed-by: Andreas Schneider --- src/misc.c | 17 +++++++---------- tests/unittests/torture_misc.c | 40 +++++++++++++++++++--------------------- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/misc.c b/src/misc.c index 21276c68..5b260b15 100644 --- a/src/misc.c +++ b/src/misc.c @@ -844,6 +844,7 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) { return -1; } + /* Make a best-effort to extract OpenSSH version numbers. */ openssh = strstr(banner, "OpenSSH"); if (openssh != NULL) { char *tmp = NULL; @@ -861,11 +862,8 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) { ((errno == ERANGE) && (major == ULONG_MAX)) || ((errno != 0) && (major == 0)) || ((major < 1) || (major > 100))) { - ssh_set_error(session, - SSH_FATAL, - "Invalid major version number: %s", - banner); - return -1; + /* invalid major */ + goto done; } minor = strtoul(openssh + 10, &tmp, 10); @@ -873,12 +871,10 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) { ((errno == ERANGE) && (major == ULONG_MAX)) || ((errno != 0) && (major == 0)) || (minor > 100)) { - ssh_set_error(session, - SSH_FATAL, - "Invalid minor version number: %s", - banner); - return -1; + /* invalid minor */ + goto done; } + session->openssh = SSH_VERSION_INT(((int) major), ((int) minor), 0); SSH_LOG(SSH_LOG_RARE, @@ -887,6 +883,7 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) { } } +done: return 0; } diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c index f11f4488..1d0e0f5c 100644 --- a/tests/unittests/torture_misc.c +++ b/tests/unittests/torture_misc.c @@ -315,35 +315,33 @@ static void torture_ssh_analyze_banner(void **state) { assert_server_banner_accepted("SSH-2.0-OpenSSH_1.99", 0, 1); assert_int_equal(SSH_VERSION_INT(1, 99, 0), session->openssh); - /* OpenSSH banners: major, minor version limits */ - reset_banner_test(); - assert_client_banner_rejected("SSH-2.0-OpenSSH_0.99p1"); - reset_banner_test(); - assert_server_banner_rejected("SSH-2.0-OpenSSH_0.99p1"); - reset_banner_test(); - assert_client_banner_rejected("SSH-2.0-OpenSSH_1.101p1"); - reset_banner_test(); - assert_server_banner_rejected("SSH-2.0-OpenSSH_1.101p1"); + /* OpenSSH banners: major, minor version limits result in zero */ + assert_client_banner_accepted("SSH-2.0-OpenSSH_0.99p1", 0, 1); + assert_int_equal(0, session->openssh); + assert_server_banner_accepted("SSH-2.0-OpenSSH_0.99p1", 0, 1); + assert_int_equal(0, session->openssh); + assert_client_banner_accepted("SSH-2.0-OpenSSH_1.101p1", 0, 1); + assert_int_equal(0, session->openssh); + assert_server_banner_accepted("SSH-2.0-OpenSSH_1.101p1", 0, 1); + assert_int_equal(0, session->openssh); - /* OpenSSH banners: bogus major */ - reset_banner_test(); - assert_client_banner_rejected("SSH-2.0-OpenSSH_X.9p1"); - reset_banner_test(); - assert_server_banner_rejected("SSH-2.0-OpenSSH_X.9p1"); + /* OpenSSH banners: bogus major results in zero */ + assert_client_banner_accepted("SSH-2.0-OpenSSH_X.9p1", 0, 1); + assert_int_equal(0, session->openssh); + assert_server_banner_accepted("SSH-2.0-OpenSSH_X.9p1", 0, 1); + assert_int_equal(0, session->openssh); - /* OpenSSH banners: bogus minor */ - reset_banner_test(); - assert_server_banner_rejected("SSH-2.0-OpenSSH_5.Yp1"); - reset_banner_test(); - assert_client_banner_rejected("SSH-2.0-OpenSSH_5.Yp1"); + /* OpenSSH banners: bogus minor results in zero */ + assert_server_banner_accepted("SSH-2.0-OpenSSH_5.Yp1", 0, 1); + assert_int_equal(0, session->openssh); + assert_client_banner_accepted("SSH-2.0-OpenSSH_5.Yp1", 0, 1); + assert_int_equal(0, session->openssh); /* OpenSSH banners: ssh-keyscan(1) */ - #if 0 /* these don't pass */ assert_client_banner_accepted("SSH-2.0-OpenSSH-keyscan", 0, 1); assert_int_equal(0, session->openssh); assert_server_banner_accepted("SSH-2.0-OpenSSH-keyscan", 0, 1); assert_int_equal(0, session->openssh); - #endif /* these don't pass */ ssh_free(session); } -- cgit v1.2.3