264 lines
9.4 KiB
Diff
264 lines
9.4 KiB
Diff
From 4f997aee7c7d7ea346b3e8ba505da0b7601ff318 Mon Sep 17 00:00:00 2001
|
|
From: Jakub Jelen <jjelen@redhat.com>
|
|
Date: Fri, 22 Dec 2023 10:32:40 +0100
|
|
Subject: [PATCH 1/2] Fix regression in IPv6 addresses in hostname parsing
|
|
|
|
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
|
|
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
|
|
---
|
|
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 @@
|
|
extern "C" {
|
|
#endif
|
|
|
|
+#include <stdbool.h>
|
|
+
|
|
char *ssh_config_get_cmd(char **str);
|
|
|
|
char *ssh_config_get_token(char **str);
|
|
@@ -49,14 +51,17 @@ 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);
|
|
|
|
#ifdef __cplusplus
|
|
}
|
|
diff --git a/src/config.c b/src/config.c
|
|
index 5eedbce9..7135c3b1 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 9ffc8b8b..5f30cd3e 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 2e73be46..676c49e7 100644
|
|
--- a/src/options.c
|
|
+++ b/src/options.c
|
|
@@ -634,17 +634,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.43.0
|
|
|
|
|
|
From 6f6e453d7b0ad4ee6a6f6a1c96a9a6b27821410d Mon Sep 17 00:00:00 2001
|
|
From: Jakub Jelen <jjelen@redhat.com>
|
|
Date: Fri, 22 Dec 2023 09:52:18 +0100
|
|
Subject: [PATCH 2/2] 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>
|
|
---
|
|
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 bc6b08f9..751aa126 100644
|
|
--- a/tests/unittests/torture_config.c
|
|
+++ b/tests/unittests/torture_config.c
|
|
@@ -2332,6 +2332,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;
|
|
@@ -2424,6 +2471,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 5ba3bdc6..b07712d8 100644
|
|
--- a/tests/unittests/torture_options.c
|
|
+++ b/tests/unittests/torture_options.c
|
|
@@ -57,6 +57,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);
|
|
@@ -64,12 +78,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.43.0
|
|
|