rpms/libssh
7
0
Fork 0
Code Issues Pull Requests Releases Activity
c8
libssh/SOURCES/CVE-2023-6004.patch

1115 lines
37 KiB
Diff
Raw Permalink Blame History

From 11bd6e6ad926a38cd7b9f8308a4c2fd8dfd9200c Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Sun, 5 Nov 2023 13:12:47 +0100
Subject: [PATCH 01/12] CVE-2023-6004: torture_config: Allow multiple '@' in
usernames
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
tests/unittests/torture_config.c | 44 ++++++++++++++++++--------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c
index f91112a9..3a5a74bf 100644
--- a/tests/unittests/torture_config.c
+++ b/tests/unittests/torture_config.c
@@ -671,24 +671,40 @@ static void torture_config_proxyjump(void **state) {
assert_string_equal(session->opts.ProxyCommand,
"ssh -W [%h]:%p 2620:52:0::fed");
- /* Try to create some invalid configurations */
- /* Non-numeric port */
+ /* Multiple @ is allowed in second jump */
torture_write_file(LIBSSH_TESTCONFIG11,
- "Host bad-port\n"
- "\tProxyJump jumpbox:22bad22\n"
+ "Host allowed-hostname\n"
+ "\tProxyJump localhost,user@principal.com@jumpbox:22\n"
"");
torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port");
+ ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname");
ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG11);
- assert_ssh_return_code_equal(session, ret, SSH_ERROR);
+ assert_ssh_return_code(session, ret);
+ assert_string_equal(session->opts.ProxyCommand,
+ "ssh -J user@principal.com@jumpbox:22 -W [%h]:%p localhost");
- /* Too many @ */
+ /* Multiple @ is allowed */
torture_write_file(LIBSSH_TESTCONFIG11,
- "Host bad-hostname\n"
+ "Host allowed-hostname\n"
"\tProxyJump user@principal.com@jumpbox:22\n"
"");
torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname");
+ ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname");
+ ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG11);
+ assert_ssh_return_code(session, ret);
+ assert_string_equal(session->opts.ProxyCommand,
+ "ssh -l user@principal.com -p 22 -W [%h]:%p jumpbox");
+
+ /* In this part, we try various other config files and strings. */
+
+ /* Try to create some invalid configurations */
+ /* Non-numeric port */
+ torture_write_file(LIBSSH_TESTCONFIG11,
+ "Host bad-port\n"
+ "\tProxyJump jumpbox:22bad22\n"
+ "");
+ torture_reset_config(session);
+ ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port");
ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG11);
assert_ssh_return_code_equal(session, ret, SSH_ERROR);
@@ -752,16 +768,6 @@ static void torture_config_proxyjump(void **state) {
ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG11);
assert_ssh_return_code_equal(session, ret, SSH_ERROR);
- /* Too many @ in second jump */
- torture_write_file(LIBSSH_TESTCONFIG11,
- "Host bad-hostname\n"
- "\tProxyJump localhost,user@principal.com@jumpbox:22\n"
- "");
- torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname");
- ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG11);
- assert_ssh_return_code_equal(session, ret, SSH_ERROR);
-
/* Braces mismatch in second jump */
torture_write_file(LIBSSH_TESTCONFIG11,
"Host mismatch\n"
--
2.41.0
From c3234e5f94b96d6e29f0c1c82821c1e3ebb181ed Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Wed, 1 Nov 2023 11:24:43 +0100
Subject: [PATCH 02/12] CVE-2023-6004: config_parser: Allow multiple '@' in
usernames
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
src/config_parser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/config_parser.c b/src/config_parser.c
index ae2aa2c8..76cca224 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -152,7 +152,7 @@ int ssh_config_parse_uri(const char *tok,
}
/* Username part (optional) */
- endp = strchr(tok, '@');
+ endp = strrchr(tok, '@');
if (endp != NULL) {
/* Zero-length username is not valid */
if (tok == endp) {
--
2.41.0
From a5b8bd0d8841296cf71d927824d60f576581243f Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 31 Oct 2023 09:48:52 +0100
Subject: [PATCH 03/12] CVE-2023-6004: options: Simplify the hostname parsing
in ssh_options_set
Using ssh_config_parse_uri can simplify the parsing of the host
parsing inside the function of ssh_options_set
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
src/options.c | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/src/options.c b/src/options.c
index b5f951ac..7c03e7ab 100644
--- a/src/options.c
+++ b/src/options.c
@@ -36,6 +36,7 @@
#include "libssh/session.h"
#include "libssh/misc.h"
#include "libssh/options.h"
+#include "libssh/config_parser.h"
#ifdef WITH_SERVER
#include "libssh/server.h"
#include "libssh/bind.h"
@@ -490,33 +491,24 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
ssh_set_error_invalid(session);
return -1;
} else {
- q = strdup(value);
- if (q == NULL) {
- ssh_set_error_oom(session);
+ char *username = NULL, *hostname = NULL, *port = NULL;
+ rc = ssh_config_parse_uri(value, &username, &hostname, &port);
+ if (rc != SSH_OK) {
return -1;
}
- p = strchr(q, '@');
-
- SAFE_FREE(session->opts.host);
-
- if (p) {
- *p = '\0';
- session->opts.host = strdup(p + 1);
- if (session->opts.host == NULL) {
- SAFE_FREE(q);
- ssh_set_error_oom(session);
- return -1;
- }
-
+ if (port != NULL) {
+ SAFE_FREE(username);
+ SAFE_FREE(hostname);
+ SAFE_FREE(port);
+ return -1;
+ }
+ if (username != NULL) {
SAFE_FREE(session->opts.username);
- session->opts.username = strdup(q);
- SAFE_FREE(q);
- if (session->opts.username == NULL) {
- ssh_set_error_oom(session);
- return -1;
- }
- } else {
- session->opts.host = q;
+ session->opts.username = username;
+ }
+ if (hostname != NULL) {
+ SAFE_FREE(session->opts.host);
+ session->opts.host = hostname;
}
}
break;
--
2.41.0
From efb24b6472e8b87c5832c0590f14e99e82fcdeeb Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 10 Oct 2023 12:44:16 +0200
Subject: [PATCH 04/12] CVE-2023-6004: misc: Add function to check allowed
characters of a hostname
The hostname can be a domain name or an ip address. The colon has to be
allowed because of IPv6 even it is prohibited in domain names.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
include/libssh/misc.h | 2 ++
src/misc.c | 68 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/include/libssh/misc.h b/include/libssh/misc.h
index 3cc3b113..a5bee930 100644
--- a/include/libssh/misc.h
+++ b/include/libssh/misc.h
@@ -97,4 +97,6 @@ int ssh_mkdirs(const char *pathname, mode_t mode);
int ssh_quote_file_name(const char *file_name, char *buf, size_t buf_len);
int ssh_newline_vis(const char *string, char *buf, size_t buf_len);
+int ssh_check_hostname_syntax(const char *hostname);
+
#endif /* MISC_H_ */
diff --git a/src/misc.c b/src/misc.c
index 149eb85e..e4239e81 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -94,6 +94,8 @@
#define ZLIB_STRING ""
#endif
+#define ARPA_DOMAIN_MAX_LEN 63
+
/**
* @defgroup libssh_misc The SSH helper functions.
* @ingroup libssh
@@ -1734,4 +1736,70 @@ int ssh_newline_vis(const char *string, char *buf, size_t buf_len)
return out - buf;
}
+/**
+ * @brief Checks syntax of a domain name
+ *
+ * The check is made based on the RFC1035 section 2.3.1
+ * Allowed characters are: hyphen, period, digits (0-9) and letters (a-zA-Z)
+ *
+ * The label should be no longer than 63 characters
+ * The label should start with a letter and end with a letter or number
+ * The label in this implementation can start with a number to allow virtual
+ * URLs to pass. Note that this will make IPv4 addresses to pass
+ * this check too.
+ *
+ * @param hostname The domain name to be checked, has to be null terminated
+ *
+ * @return SSH_OK if the hostname passes syntax check
+ * SSH_ERROR otherwise or if hostname is NULL or empty string
+ */
+int ssh_check_hostname_syntax(const char *hostname)
+{
+ char *it = NULL, *s = NULL, *buf = NULL;
+ size_t it_len;
+ char c;
+
+ if (hostname == NULL || strlen(hostname) == 0) {
+ return SSH_ERROR;
+ }
+
+ /* strtok_r writes into the string, keep the input clean */
+ s = strdup(hostname);
+ if (s == NULL) {
+ return SSH_ERROR;
+ }
+
+ it = strtok_r(s, ".", &buf);
+ /* if the token has 0 length */
+ if (it == NULL) {
+ free(s);
+ return SSH_ERROR;
+ }
+ do {
+ it_len = strlen(it);
+ if (it_len > ARPA_DOMAIN_MAX_LEN ||
+ /* the first char must be a letter, but some virtual urls start
+ * with a number */
+ isalnum(it[0]) == 0 ||
+ isalnum(it[it_len - 1]) == 0) {
+ free(s);
+ return SSH_ERROR;
+ }
+ while (*it != '\0') {
+ c = *it;
+ /* the "." is allowed too, but tokenization removes it from the
+ * string */
+ if (isalnum(c) == 0 && c != '-') {
+ free(s);
+ return SSH_ERROR;
+ }
+ it++;
+ }
+ } while ((it = strtok_r(NULL, ".", &buf)) != NULL);
+
+ free(s);
+
+ return SSH_OK;
+}
+
/** @} */
--
2.41.0
From 234ecdf4d9705efa3727a54dcc1ddfe6377c7bf6 Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 10 Oct 2023 12:45:28 +0200
Subject: [PATCH 05/12] CVE-2023-6004: torture_misc: Add test for
ssh_check_hostname_syntax
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
tests/unittests/torture_misc.c | 73 ++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c
index 0a48abbe..d14f4254 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -656,6 +656,78 @@ static void torture_ssh_newline_vis(UNUSED_PARAM(void **state))
assert_string_equal(buffer, "a\\nb\\n");
}
+static void torture_ssh_check_hostname_syntax(void **state)
+{
+ int rc;
+ (void)state;
+
+ rc = ssh_check_hostname_syntax("duckduckgo.com");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("www.libssh.org");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("Some-Thing.com");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123.ok");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("lavabo-inter.innocentes-manus-meas");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("localhost");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("a");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("a-0.b-b");
+ assert_int_equal(rc, SSH_OK);
+ rc = ssh_check_hostname_syntax("libssh.");
+ assert_int_equal(rc, SSH_OK);
+
+ rc = ssh_check_hostname_syntax(NULL);
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("/");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("@");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("[");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("`");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("{");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("&");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("|");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("\"");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("`");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax(" ");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("*the+giant&\"rooks\".c0m");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("!www.libssh.org");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("--.--");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("libssh.a234567890123456789012345678901234567890123456789012345678901234");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("libssh.a234567890123456789012345678901234567890123456789012345678901234.a234567890123456789012345678901234567890123456789012345678901234");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("libssh-");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("fe80::9656:d028:8652:66b6");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax(".");
+ assert_int_equal(rc, SSH_ERROR);
+ rc = ssh_check_hostname_syntax("..");
+ assert_int_equal(rc, SSH_ERROR);
+}
+
int torture_run_tests(void) {
int rc;
struct CMUnitTest tests[] = {
@@ -678,6 +750,7 @@ int torture_run_tests(void) {
cmocka_unit_test(torture_ssh_newline_vis),
cmocka_unit_test(torture_ssh_mkdirs),
cmocka_unit_test(torture_ssh_quote_file_name),
+ cmocka_unit_test(torture_ssh_check_hostname_syntax),
};
ssh_init();
--
2.41.0
From 4d7ae19e9cd8c407012b40f3f2eaf480bfb1da7d Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 10 Oct 2023 18:33:56 +0200
Subject: [PATCH 06/12] CVE-2023-6004: config_parser: Check for valid syntax of
a hostname if it is a domain name
This prevents code injection.
The domain name syntax checker is based on RFC1035.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
src/config_parser.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/config_parser.c b/src/config_parser.c
index 76cca224..87bac5d4 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -30,6 +30,7 @@
#include "libssh/config_parser.h"
#include "libssh/priv.h"
+#include "libssh/misc.h"
char *ssh_config_get_cmd(char **str)
{
@@ -139,6 +140,7 @@ int ssh_config_parse_uri(const char *tok,
{
char *endp = NULL;
long port_n;
+ int rc;
/* Sanitize inputs */
if (username != NULL) {
@@ -196,6 +198,14 @@ int ssh_config_parse_uri(const char *tok,
if (*hostname == NULL) {
goto error;
}
+ /* if not an ip, check syntax */
+ rc = ssh_is_ipaddr(*hostname);
+ if (rc == 0) {
+ rc = ssh_check_hostname_syntax(*hostname);
+ if (rc != SSH_OK) {
+ goto error;
+ }
+ }
}
/* Skip also the closing bracket */
if (*endp == ']') {
--
2.41.0
From 8cf4f4bfda968ab526c1a601ea1030bbaccaba17 Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 10 Oct 2023 10:28:47 +0200
Subject: [PATCH 07/12] CVE-2023-6004: torture_proxycommand: Add test for
proxycommand injection
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
tests/client/torture_proxycommand.c | 53 +++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tests/client/torture_proxycommand.c b/tests/client/torture_proxycommand.c
index c04ff2ab..dc17f3d8 100644
--- a/tests/client/torture_proxycommand.c
+++ b/tests/client/torture_proxycommand.c
@@ -161,6 +161,56 @@ static void torture_options_set_proxycommand_ssh_stderr(void **state)
assert_int_equal(rc & O_RDWR, O_RDWR);
}
+static void torture_options_proxycommand_injection(void **state)
+{
+ struct torture_state *s = *state;
+ struct passwd *pwd = NULL;
+ const char *malicious_host = "`echo foo > mfile`";
+ const char *command = "nc %h %p";
+ char *current_dir = NULL;
+ char *malicious_file_path = NULL;
+ int mfp_len;
+ int verbosity = torture_libssh_verbosity();
+ struct stat sb;
+ int rc;
+
+ pwd = getpwnam("bob");
+ assert_non_null(pwd);
+
+ rc = setuid(pwd->pw_uid);
+ assert_return_code(rc, errno);
+
+ s->ssh.session = ssh_new();
+ assert_non_null(s->ssh.session);
+
+ ssh_options_set(s->ssh.session, SSH_OPTIONS_LOG_VERBOSITY, &verbosity);
+ // if we would be checking the rc, this should fail
+ ssh_options_set(s->ssh.session, SSH_OPTIONS_HOST, malicious_host);
+
+ ssh_options_set(s->ssh.session, SSH_OPTIONS_USER, TORTURE_SSH_USER_ALICE);
+
+ rc = ssh_options_set(s->ssh.session, SSH_OPTIONS_PROXYCOMMAND, command);
+ assert_int_equal(rc, 0);
+ rc = ssh_connect(s->ssh.session);
+ assert_ssh_return_code_equal(s->ssh.session, rc, SSH_ERROR);
+
+ current_dir = torture_get_current_working_dir();
+ assert_non_null(current_dir);
+ mfp_len = strlen(current_dir) + 6;
+ malicious_file_path = malloc(mfp_len);
+ assert_non_null(malicious_file_path);
+ rc = snprintf(malicious_file_path, mfp_len,
+ "%s/mfile", current_dir);
+ assert_int_equal(rc, mfp_len);
+ free(current_dir);
+ rc = stat(malicious_file_path, &sb);
+ assert_int_not_equal(rc, 0);
+
+ // cleanup
+ remove(malicious_file_path);
+ free(malicious_file_path);
+}
+
int torture_run_tests(void) {
int rc;
struct CMUnitTest tests[] = {
@@ -176,6 +226,9 @@ int torture_run_tests(void) {
cmocka_unit_test_setup_teardown(torture_options_set_proxycommand_ssh_stderr,
session_setup,
session_teardown),
+ cmocka_unit_test_setup_teardown(torture_options_proxycommand_injection,
+ NULL,
+ session_teardown),
};
--
2.41.0
From a0dbe0d556e073804cc549802569577bb24757d9 Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Mon, 6 Nov 2023 20:11:38 +0100
Subject: [PATCH 08/12] CVE-2023-6004: torture_misc: Add test for ssh_is_ipaddr
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
tests/unittests/torture_misc.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c
index d14f4254..073bc54c 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -728,6 +728,31 @@ static void torture_ssh_check_hostname_syntax(void **state)
assert_int_equal(rc, SSH_ERROR);
}
+static void torture_ssh_is_ipaddr(void **state) {
+ int rc;
+ (void)state;
+
+ rc = ssh_is_ipaddr("201.255.3.69");
+ assert_int_equal(rc, 1);
+ rc = ssh_is_ipaddr("::1");
+ assert_int_equal(rc, 1);
+ rc = ssh_is_ipaddr("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+ assert_int_equal(rc, 1);
+
+ rc = ssh_is_ipaddr("..");
+ assert_int_equal(rc, 0);
+ rc = ssh_is_ipaddr(":::");
+ assert_int_equal(rc, 0);
+ rc = ssh_is_ipaddr("1.1.1.1.1");
+ assert_int_equal(rc, 0);
+ rc = ssh_is_ipaddr("1.1");
+ assert_int_equal(rc, 0);
+ rc = ssh_is_ipaddr("caesar");
+ assert_int_equal(rc, 0);
+ rc = ssh_is_ipaddr("::xa:1");
+ assert_int_equal(rc, 0);
+}
+
int torture_run_tests(void) {
int rc;
struct CMUnitTest tests[] = {
@@ -751,6 +776,7 @@ int torture_run_tests(void) {
cmocka_unit_test(torture_ssh_mkdirs),
cmocka_unit_test(torture_ssh_quote_file_name),
cmocka_unit_test(torture_ssh_check_hostname_syntax),
+ cmocka_unit_test(torture_ssh_is_ipaddr),
};
ssh_init();
--
2.41.0
From cdaec0d6273243a03f460cc5ba1a2265b4afb93a Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 28 Nov 2023 15:26:45 +0100
Subject: [PATCH 09/12] CVE-2023-6004: misc: Add ipv6 link-local check for an
ip address
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
src/CMakeLists.txt | 17 ++++++++++-------
src/connect.c | 2 +-
src/misc.c | 44 ++++++++++++++++++++++++++++++++++++++------
3 files changed, 49 insertions(+), 14 deletions(-)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index a576cf71..fc401793 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -9,13 +9,6 @@ set(LIBSSH_LINK_LIBRARIES
${LIBSSH_REQUIRED_LIBRARIES}
)
-if (WIN32)
- set(LIBSSH_LINK_LIBRARIES
- ${LIBSSH_LINK_LIBRARIES}
- ws2_32
- )
-endif (WIN32)
-
if (OPENSSL_CRYPTO_LIBRARIES)
set(LIBSSH_PRIVATE_INCLUDE_DIRS
${LIBSSH_PRIVATE_INCLUDE_DIRS}
@@ -93,6 +86,16 @@ if (MINGW AND Threads_FOUND)
)
endif()
+# This needs to be last for mingw to build
+# https://gitlab.com/libssh/libssh-mirror/-/issues/84
+if (WIN32)
+ set(LIBSSH_LINK_LIBRARIES
+ ${LIBSSH_LINK_LIBRARIES}
+ iphlpapi
+ ws2_32
+ )
+endif (WIN32)
+
if (BUILD_STATIC_LIB)
set(LIBSSH_STATIC_LIBRARY
ssh_static
diff --git a/src/connect.c b/src/connect.c
index ce4d58df..ca62dcf0 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -136,7 +136,7 @@ static int getai(const char *host, int port, struct addrinfo **ai)
#endif
}
- if (ssh_is_ipaddr(host)) {
+ if (ssh_is_ipaddr(host) == 1) {
/* this is an IP address */
SSH_LOG(SSH_LOG_PACKET, "host %s matches an IP address", host);
hints.ai_flags |= AI_NUMERICHOST;
diff --git a/src/misc.c b/src/misc.c
index e4239e81..6f5d2d60 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -32,6 +32,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <net/if.h>
#endif /* _WIN32 */
@@ -59,6 +60,7 @@
#include <ws2tcpip.h>
#include <shlobj.h>
#include <direct.h>
+#include <netioapi.h>
#ifdef HAVE_IO_H
#include <io.h>
@@ -216,22 +218,37 @@ int ssh_is_ipaddr_v4(const char *str) {
int ssh_is_ipaddr(const char *str) {
int rc = SOCKET_ERROR;
+ char *s = strdup(str);
- if (strchr(str, ':')) {
+ if (s == NULL) {
+ return -1;
+ }
+ if (strchr(s, ':')) {
struct sockaddr_storage ss;
int sslen = sizeof(ss);
+ char *network_interface = strchr(s, '%');
- /* TODO link-local (IP:v6:addr%ifname). */
- rc = WSAStringToAddressA((LPSTR) str,
+ /* link-local (IP:v6:addr%ifname). */
+ if (network_interface != NULL) {
+ rc = if_nametoindex(network_interface + 1);
+ if (rc == 0) {
+ free(s);
+ return 0;
+ }
+ *network_interface = '\0';
+ }
+ rc = WSAStringToAddressA((LPSTR) s,
AF_INET6,
NULL,
(struct sockaddr*)&ss,
&sslen);
if (rc == 0) {
+ free(s);
return 1;
}
}
+ free(s);
return ssh_is_ipaddr_v4(str);
}
#else /* _WIN32 */
@@ -335,17 +352,32 @@ int ssh_is_ipaddr_v4(const char *str) {
int ssh_is_ipaddr(const char *str) {
int rc = -1;
+ char *s = strdup(str);
- if (strchr(str, ':')) {
+ if (s == NULL) {
+ return -1;
+ }
+ if (strchr(s, ':')) {
struct in6_addr dest6;
+ char *network_interface = strchr(s, '%');
- /* TODO link-local (IP:v6:addr%ifname). */
- rc = inet_pton(AF_INET6, str, &dest6);
+ /* link-local (IP:v6:addr%ifname). */
+ if (network_interface != NULL) {
+ rc = if_nametoindex(network_interface + 1);
+ if (rc == 0) {
+ free(s);
+ return 0;
+ }
+ *network_interface = '\0';
+ }
+ rc = inet_pton(AF_INET6, s, &dest6);
if (rc > 0) {
+ free(s);
return 1;
}
}
+ free(s);
return ssh_is_ipaddr_v4(str);
}
--
2.41.0
From 6a8a18c73e73a338283dfbade0a7d83e5cfafe3b Mon Sep 17 00:00:00 2001
From: Norbert Pocs <norbertpocs0@gmail.com>
Date: Tue, 28 Nov 2023 15:27:31 +0100
Subject: [PATCH 10/12] CVE-2023-6004: torture_misc: Add tests for ipv6
link-local
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
---
tests/unittests/torture_misc.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c
index 073bc54c..f16b766e 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -17,7 +17,14 @@
#include "misc.c"
#include "error.c"
+#ifdef _WIN32
+#include <netioapi.h>
+#else
+#include <net/if.h>
+#endif
+
#define TORTURE_TEST_DIR "/usr/local/bin/truc/much/.."
+#define TORTURE_IPV6_LOCAL_LINK "fe80::98e1:82ff:fe8d:28b3%%%s"
const char template[] = "temp_dir_XXXXXX";
@@ -730,14 +737,27 @@ static void torture_ssh_check_hostname_syntax(void **state)
static void torture_ssh_is_ipaddr(void **state) {
int rc;
+ char *interf = malloc(64);
+ char *test_interf = malloc(128);
(void)state;
+ assert_non_null(interf);
+ assert_non_null(test_interf);
rc = ssh_is_ipaddr("201.255.3.69");
assert_int_equal(rc, 1);
rc = ssh_is_ipaddr("::1");
assert_int_equal(rc, 1);
rc = ssh_is_ipaddr("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
assert_int_equal(rc, 1);
+ if_indextoname(1, interf);
+ assert_non_null(interf);
+ rc = sprintf(test_interf, TORTURE_IPV6_LOCAL_LINK, interf);
+ /* the "%%s" is not written */
+ assert_int_equal(rc, strlen(interf) + strlen(TORTURE_IPV6_LOCAL_LINK) - 3);
+ rc = ssh_is_ipaddr(test_interf);
+ assert_int_equal(rc, 1);
+ free(interf);
+ free(test_interf);
rc = ssh_is_ipaddr("..");
assert_int_equal(rc, 0);
--
2.41.0
From 72f59157e6ccbd4c0bb806690931413169a0886f Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Fri, 22 Dec 2023 10:32:40 +0100
Subject: [PATCH 11/12] Fix regression in IPv6 addresses in hostname parsing
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
(cherry picked from commit 4f997aee7c7d7ea346b3e8ba505da0b7601ff318)
---
include/libssh/config_parser.h | 11 ++++++++---
src/config.c | 4 ++--
src/config_parser.c | 16 +++++++++++-----
src/options.c | 10 ++--------
4 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/include/libssh/config_parser.h b/include/libssh/config_parser.h
index e974917c..ee647bfb 100644
--- a/include/libssh/config_parser.h
+++ b/include/libssh/config_parser.h
@@ -26,6 +26,8 @@
#ifndef CONFIG_PARSER_H_
#define CONFIG_PARSER_H_
+#include <stdbool.h>
+
char *ssh_config_get_cmd(char **str);
char *ssh_config_get_token(char **str);
@@ -45,13 +47,16 @@ int ssh_config_get_yesno(char **str, int notfound);
* be stored or NULL if we do not care about the result.
* @param[out] port Pointer to the location, where the new port will
* be stored or NULL if we do not care about the result.
+ * @param[in] ignore_port Set to true if the we should not attempt to parse
+ * port number.
*
* @returns SSH_OK if the provided string is in format of SSH URI,
* SSH_ERROR on failure
*/
int ssh_config_parse_uri(const char *tok,
- char **username,
- char **hostname,
- char **port);
+ char **username,
+ char **hostname,
+ char **port,
+ bool ignore_port);
#endif /* LIBSSH_CONFIG_H_ */
diff --git a/src/config.c b/src/config.c
index 54ada276..a813568d 100644
--- a/src/config.c
+++ b/src/config.c
@@ -324,7 +324,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing)
}
if (parse_entry) {
/* We actually care only about the first item */
- rv = ssh_config_parse_uri(cp, &username, &hostname, &port);
+ rv = ssh_config_parse_uri(cp, &username, &hostname, &port, false);
/* The rest of the list needs to be passed on */
if (endp != NULL) {
next = strdup(endp + 1);
@@ -335,7 +335,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing)
}
} else {
/* The rest is just sanity-checked to avoid failures later */
- rv = ssh_config_parse_uri(cp, NULL, NULL, NULL);
+ rv = ssh_config_parse_uri(cp, NULL, NULL, NULL, false);
}
if (rv != SSH_OK) {
goto out;
diff --git a/src/config_parser.c b/src/config_parser.c
index 87bac5d4..a2da0a62 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -134,9 +134,10 @@ int ssh_config_get_yesno(char **str, int notfound)
}
int ssh_config_parse_uri(const char *tok,
- char **username,
- char **hostname,
- char **port)
+ char **username,
+ char **hostname,
+ char **port,
+ bool ignore_port)
{
char *endp = NULL;
long port_n;
@@ -182,12 +183,17 @@ int ssh_config_parse_uri(const char *tok,
if (endp == NULL) {
goto error;
}
- } else {
- /* Hostnames or aliases expand to the last colon or to the end */
+ } else if (!ignore_port) {
+ /* Hostnames or aliases expand to the last colon (if port is requested)
+ * or to the end */
endp = strrchr(tok, ':');
if (endp == NULL) {
endp = strchr(tok, '\0');
}
+ } else {
+ /* If no port is requested, expand to the end of line
+ * (to accommodate the IPv6 addresses) */
+ endp = strchr(tok, '\0');
}
if (tok == endp) {
/* Zero-length hostnames are not valid */
diff --git a/src/options.c b/src/options.c
index 7c03e7ab..0890ff2e 100644
--- a/src/options.c
+++ b/src/options.c
@@ -491,17 +491,11 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
ssh_set_error_invalid(session);
return -1;
} else {
- char *username = NULL, *hostname = NULL, *port = NULL;
- rc = ssh_config_parse_uri(value, &username, &hostname, &port);
+ char *username = NULL, *hostname = NULL;
+ rc = ssh_config_parse_uri(value, &username, &hostname, NULL, true);
if (rc != SSH_OK) {
return -1;
}
- if (port != NULL) {
- SAFE_FREE(username);
- SAFE_FREE(hostname);
- SAFE_FREE(port);
- return -1;
- }
if (username != NULL) {
SAFE_FREE(session->opts.username);
session->opts.username = username;
--
2.41.0
From 5dc10ff63ca2e8db91abfdccf1d095f5b4261b8e Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Fri, 22 Dec 2023 09:52:18 +0100
Subject: [PATCH 12/12] tests: Increase test coverage for IPv6 address parsing
as hostnames
This was an issue in cockpit:
https://github.com/cockpit-project/cockpit/issues/19772
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
(cherry picked from commit 6f6e453d7b0ad4ee6a6f6a1c96a9a6b27821410d)
---
tests/unittests/torture_config.c | 49 +++++++++++++++++++++++++++++++
tests/unittests/torture_options.c | 22 ++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c
index 3a5a74bf..d8097e79 100644
--- a/tests/unittests/torture_config.c
+++ b/tests/unittests/torture_config.c
@@ -2,6 +2,7 @@
#define LIBSSH_STATIC
+#include <errno.h>
#include "torture.h"
#include "libssh/options.h"
#include "libssh/session.h"
@@ -997,6 +998,53 @@ static void torture_config_match_pattern(void **state)
}
+static void torture_config_parse_uri(void **state)
+{
+ char *username = NULL;
+ char *hostname = NULL;
+ char *port = NULL;
+ int rc;
+
+ (void)state; /* unused */
+
+ rc = ssh_config_parse_uri("localhost", &username, &hostname, &port, false);
+ assert_return_code(rc, errno);
+ assert_null(username);
+ assert_string_equal(hostname, "localhost");
+ SAFE_FREE(hostname);
+ assert_null(port);
+
+ rc = ssh_config_parse_uri("1.2.3.4", &username, &hostname, &port, false);
+ assert_return_code(rc, errno);
+ assert_null(username);
+ assert_string_equal(hostname, "1.2.3.4");
+ SAFE_FREE(hostname);
+ assert_null(port);
+
+ rc = ssh_config_parse_uri("1.2.3.4:2222", &username, &hostname, &port, false);
+ assert_return_code(rc, errno);
+ assert_null(username);
+ assert_string_equal(hostname, "1.2.3.4");
+ SAFE_FREE(hostname);
+ assert_string_equal(port, "2222");
+ SAFE_FREE(port);
+
+ rc = ssh_config_parse_uri("[1:2:3::4]:2222", &username, &hostname, &port, false);
+ assert_return_code(rc, errno);
+ assert_null(username);
+ assert_string_equal(hostname, "1:2:3::4");
+ SAFE_FREE(hostname);
+ assert_string_equal(port, "2222");
+ SAFE_FREE(port);
+
+ /* do not want port */
+ rc = ssh_config_parse_uri("1:2:3::4", &username, &hostname, NULL, true);
+ assert_return_code(rc, errno);
+ assert_null(username);
+ assert_string_equal(hostname, "1:2:3::4");
+ SAFE_FREE(hostname);
+}
+
int torture_run_tests(void) {
int rc;
@@ -1012,6 +1060,7 @@ int torture_run_tests(void) {
cmocka_unit_test(torture_config_rekey),
cmocka_unit_test(torture_config_pubkeyacceptedkeytypes),
cmocka_unit_test(torture_config_match_pattern),
+ cmocka_unit_test(torture_config_parse_uri),
};
diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c
index d0fdaed1..576ca9cd 100644
--- a/tests/unittests/torture_options.c
+++ b/tests/unittests/torture_options.c
@@ -59,12 +59,34 @@ static void torture_options_set_host(void **state) {
assert_non_null(session->opts.host);
assert_string_equal(session->opts.host, "localhost");
+ /* IPv4 address */
+ rc = ssh_options_set(session, SSH_OPTIONS_HOST, "127.1.1.1");
+ assert_true(rc == 0);
+ assert_non_null(session->opts.host);
+ assert_string_equal(session->opts.host, "127.1.1.1");
+ assert_null(session->opts.username);
+
+ /* IPv6 address */
+ rc = ssh_options_set(session, SSH_OPTIONS_HOST, "::1");
+ assert_true(rc == 0);
+ assert_non_null(session->opts.host);
+ assert_string_equal(session->opts.host, "::1");
+ assert_null(session->opts.username);
+
rc = ssh_options_set(session, SSH_OPTIONS_HOST, "guru@meditation");
assert_true(rc == 0);
assert_non_null(session->opts.host);
assert_string_equal(session->opts.host, "meditation");
assert_non_null(session->opts.username);
assert_string_equal(session->opts.username, "guru");
+
+ /* more @ in uri is OK -- it should go to the username */
+ rc = ssh_options_set(session, SSH_OPTIONS_HOST, "at@login@hostname");
+ assert_true(rc == 0);
+ assert_non_null(session->opts.host);
+ assert_string_equal(session->opts.host, "hostname");
+ assert_non_null(session->opts.username);
+ assert_string_equal(session->opts.username, "at@login");
}
static void torture_options_set_ciphers(void **state) {
--
2.41.0
Reference in New Issue View Git Blame Copy Permalink