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

1113 lines
37 KiB
Diff
Raw Permalink Blame History

From c2c56bacab00766d01671413321d564227aabf19 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
tests/unittests/torture_config.c | 56 +++++++++++++++++---------------
1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c
index 406f1985..b7c763af 100644
--- a/tests/unittests/torture_config.c
+++ b/tests/unittests/torture_config.c
@@ -995,23 +995,22 @@ static void torture_config_proxyjump(void **state,
assert_string_equal(session->opts.ProxyCommand,
"ssh -W [%h]:%p 2620:52:0::fed");
- /* In this part, we try various other config files and strings. */
-
- /* Try to create some invalid configurations */
- /* Non-numeric port */
- config = "Host bad-port\n"
- "\tProxyJump jumpbox:22bad22\n";
+ /* Multiple @ is allowed in second jump */
+ config = "Host allowed-hostname\n"
+ "\tProxyJump localhost,user@principal.com@jumpbox:22\n";
if (file != NULL) {
torture_write_file(file, config);
} else {
string = config;
}
torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port");
- _parse_config(session, file, string, SSH_ERROR);
+ ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname");
+ _parse_config(session, file, string, SSH_OK);
+ assert_string_equal(session->opts.ProxyCommand,
+ "ssh -J user@principal.com@jumpbox:22 -W '[%h]:%p' localhost");
- /* Too many @ */
- config = "Host bad-hostname\n"
+ /* Multiple @ is allowed */
+ config = "Host allowed-hostname\n"
"\tProxyJump user@principal.com@jumpbox:22\n";
if (file != NULL) {
torture_write_file(file, config);
@@ -1019,7 +1018,24 @@ static void torture_config_proxyjump(void **state,
string = config;
}
torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname");
+ ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname");
+ _parse_config(session, file, string, SSH_OK);
+ 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 */
+ config = "Host bad-port\n"
+ "\tProxyJump jumpbox:22bad22\n";
+ if (file != NULL) {
+ torture_write_file(file, config);
+ } else {
+ string = config;
+ }
+ torture_reset_config(session);
+ ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port");
_parse_config(session, file, string, SSH_ERROR);
/* Braces mismatch in hostname */
@@ -1094,18 +1110,6 @@ static void torture_config_proxyjump(void **state,
ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port-2");
_parse_config(session, file, string, SSH_ERROR);
- /* Too many @ in second jump */
- config = "Host bad-hostname\n"
- "\tProxyJump localhost,user@principal.com@jumpbox:22\n";
- if (file != NULL) {
- torture_write_file(file, config);
- } else {
- string = config;
- }
- torture_reset_config(session);
- ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname");
- _parse_config(session, file, string, SSH_ERROR);
-
/* Braces mismatch in second jump */
config = "Host mismatch\n"
"\tProxyJump localhost,[::1:20\n";
--
2.41.0
From a66b4a6eae6614d200a3625862d77565b96a7cd3 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
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 0d988fec..cf83e2c5 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -180,7 +180,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 8615c24647f773a5e04203c7459512715d698be1 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
src/options.c | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/src/options.c b/src/options.c
index 6f2c9397..38511455 100644
--- a/src/options.c
+++ b/src/options.c
@@ -37,6 +37,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"
@@ -515,33 +516,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 = strrchr(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 c6180409677c765e6b9ae2b18a3a7a9671ac1dbe 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
include/libssh/misc.h | 3 ++
src/misc.c | 68 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/src/misc.c b/src/misc.c
index 7c478a77..be6ee836 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -1974,4 +1976,70 @@ char *ssh_strerror(int err_num, char *buf, size_t buflen)
#endif /* defined(__linux__) && defined(__GLIBC__) && defined(_GNU_SOURCE) */
}
+/**
+ * @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 9bbb817c0c5434f03613d0783b2ef5f52235b901 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
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 9e346ff8..e682b6d4 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -760,6 +760,78 @@ static void torture_ssh_strerror(void **state)
assert_non_null(out);
}
+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[] = {
@@ -784,6 +856,7 @@ int torture_run_tests(void) {
cmocka_unit_test(torture_ssh_quote_file_name),
cmocka_unit_test(torture_ssh_strreplace),
cmocka_unit_test(torture_ssh_strerror),
+ cmocka_unit_test(torture_ssh_check_hostname_syntax),
};
ssh_init();
--
2.41.0
From 22492b69bba22b102342afc574800d354a08e405 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
src/config_parser.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/config_parser.c b/src/config_parser.c
index cf83e2c5..b8b94611 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"
/* Returns the original string after skipping the leading whitespace
* and optional quotes.
@@ -167,6 +168,7 @@ int ssh_config_parse_uri(const char *tok,
{
char *endp = NULL;
long port_n;
+ int rc;
/* Sanitize inputs */
if (username != NULL) {
@@ -224,6 +226,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 d7467498fd988949edde9c6384973250fd454a8b 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
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 9b8019ca..1bad4ccc 100644
--- a/tests/client/torture_proxycommand.c
+++ b/tests/client/torture_proxycommand.c
@@ -166,6 +166,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[] = {
@@ -181,6 +231,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 62d3101c1f76b6891b70c50154e0e934d6b8cb57 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
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 e682b6d4..b07392ab 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -832,6 +832,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[] = {
@@ -857,6 +882,7 @@ int torture_run_tests(void) {
cmocka_unit_test(torture_ssh_strreplace),
cmocka_unit_test(torture_ssh_strerror),
cmocka_unit_test(torture_ssh_check_hostname_syntax),
+ cmocka_unit_test(torture_ssh_is_ipaddr),
};
ssh_init();
--
2.41.0
From cea841d71c025f9c998b7d5fc9f2a2839df62921 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
src/CMakeLists.txt | 1 +
src/connect.c | 2 +-
src/misc.c | 44 ++++++++++++++++++++++++++++++++++++++------
3 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d6245c0d..807313b5 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -91,6 +91,7 @@ endif()
if (WIN32)
set(LIBSSH_LINK_LIBRARIES
${LIBSSH_LINK_LIBRARIES}
+ iphlpapi
ws2_32
)
endif (WIN32)
diff --git a/src/connect.c b/src/connect.c
index 57e37e63..15cae644 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 be6ee836..7081f12a 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>
@@ -222,22 +224,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 */
@@ -343,17 +360,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 2c492ee179d5caa2718c5e768bab6e0b2b64a8b0 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>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
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 b07392ab..77166759 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -17,7 +17,14 @@
#include "torture.h"
#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";
@@ -834,14 +841,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 1a02364b5107a4125ea3cb76fcdb6beabaebf3be 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 a7dd42a2..ca353432 100644
--- a/include/libssh/config_parser.h
+++ b/include/libssh/config_parser.h
@@ -30,6 +30,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);
diff --git a/src/config.c b/src/config.c
index c5c40125..273db7c8 100644
--- a/src/config.c
+++ b/src/config.c
@@ -464,7 +464,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);
@@ -475,7 +475,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 b8b94611..d4b2d2c3 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -162,9 +162,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;
@@ -210,12 +211,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 38511455..b3ecffe1 100644
--- a/src/options.c
+++ b/src/options.c
@@ -516,17 +516,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 6f1b1e76bb38bc89819132e1810e4301ec9034a4 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 | 16 ++++++++++
2 files changed, 65 insertions(+)
diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c
index b7c763af..26a24215 100644
--- a/tests/unittests/torture_config.c
+++ b/tests/unittests/torture_config.c
@@ -1850,6 +1850,53 @@ static void torture_config_make_absolute_no_sshdir(void **state)
torture_config_make_absolute_int(state, 1);
}
+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;
@@ -1922,6 +1969,8 @@ int torture_run_tests(void)
setup, teardown),
cmocka_unit_test_setup_teardown(torture_config_make_absolute_no_sshdir,
setup_no_sshdir, teardown),
+ cmocka_unit_test_setup_teardown(torture_config_parse_uri,
+ setup, teardown),
};
diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c
index 425d7295..8a5505a1 100644
--- a/tests/unittests/torture_options.c
+++ b/tests/unittests/torture_options.c
@@ -60,6 +60,20 @@ 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);
@@ -67,12 +81,14 @@ static void torture_options_set_host(void **state) {
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
diff -up libssh-0.10.4/include/libssh/config_parser.h.CVE-2023-6004.patch libssh-0.10.4/include/libssh/config_parser.h
--- libssh-0.10.4/include/libssh/config_parser.h.CVE-2023-6004.patch 2024-01-08 19:48:48.964006647 +0100
+++ libssh-0.10.4/include/libssh/config_parser.h 2024-01-08 19:56:38.850726165 +0100
@@ -47,13 +47,16 @@ int ssh_config_get_yesno(char **str, int
* 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 -up libssh-0.10.4/include/libssh/misc.h.CVE-2023-6004.patch libssh-0.10.4/include/libssh/misc.h
--- libssh-0.10.4/include/libssh/misc.h.CVE-2023-6004.patch 2024-01-08 19:48:07.422589393 +0100
+++ libssh-0.10.4/include/libssh/misc.h 2024-01-08 19:53:33.830867829 +0100
@@ -99,4 +99,7 @@ int ssh_newline_vis(const char *string,
int ssh_tmpname(char *template);
char *ssh_strreplace(const char *src, const char *pattern, const char *repl);
+
+int ssh_check_hostname_syntax(const char *hostname);
+
#endif /* MISC_H_ */
diff -up libssh-0.10.4/src/misc.c.CVE-2023-6004.patch libssh-0.10.4/src/misc.c
--- libssh-0.10.4/src/misc.c.CVE-2023-6004.patch 2024-01-08 19:48:26.779783823 +0100
+++ libssh-0.10.4/src/misc.c 2024-01-08 19:54:55.209685193 +0100
@@ -96,6 +96,8 @@
#define ZLIB_STRING ""
#endif
+#define ARPA_DOMAIN_MAX_LEN 63
+
/**
* @defgroup libssh_misc The SSH helper functions.
* @ingroup libssh
Reference in New Issue View Git Blame Copy Permalink