From b040856ccfde1a5d4c21791f46ca6ee00c21a47b Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Tue, 2 Jul 2019 10:21:15 +0200
Subject: [PATCH 1/6] knownhosts: Fix possible memory leak
The memory allocated for host_port can leak if the global knownhosts
file is unaccessible.
Found by address sanitizer build in CI.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit fe248414fec1e654e4ee1259927d68777dd870ae)
---
src/knownhosts.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/knownhosts.c b/src/knownhosts.c
index 8a4a8ba7..9383cc97 100644
--- a/src/knownhosts.c
+++ b/src/knownhosts.c
@@ -706,13 +706,15 @@ enum ssh_known_hosts_e ssh_session_has_known_hosts_entry(ssh_session session)
rc = ssh_known_hosts_read_entries(host_port,
session->opts.global_knownhosts,
&entry_list);
- SAFE_FREE(host_port);
if (rc != 0) {
+ SAFE_FREE(host_port);
ssh_list_free(entry_list);
return SSH_KNOWN_HOSTS_ERROR;
}
}
+ SAFE_FREE(host_port);
+
if (ssh_list_count(entry_list) == 0) {
ssh_list_free(entry_list);
return SSH_KNOWN_HOSTS_UNKNOWN;
--
2.21.0
From 7ff0af75436ee6e549907dd563e968b92f7f8db2 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Fri, 28 Jun 2019 13:19:51 +0200
Subject: [PATCH 2/6] tests: Check if known_hosts works with single
unaccessible file
Make sure known hosts check works when local known_hosts file is
unaccessible, but the host is present in global known_hosts file.
Remove double return value check in previous existing test.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit ad68de7271e6ccda261df4d9fc827321e7d90fd0)
---
tests/unittests/torture_knownhosts_parsing.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tests/unittests/torture_knownhosts_parsing.c b/tests/unittests/torture_knownhosts_parsing.c
index ac8d7f31..a087caef 100644
--- a/tests/unittests/torture_knownhosts_parsing.c
+++ b/tests/unittests/torture_knownhosts_parsing.c
@@ -384,22 +384,19 @@ static void torture_knownhosts_host_exists(void **state)
/* This makes sure the system's known_hosts are not used */
ssh_options_set(session, SSH_OPTIONS_GLOBAL_KNOWNHOSTS, "/dev/null");
-
found = ssh_session_has_known_hosts_entry(session);
assert_int_equal(found, SSH_KNOWN_HOSTS_OK);
- assert_true(found == SSH_KNOWN_HOSTS_OK);
/* This makes sure the check will not fail when the system's known_hosts is
* not accessible*/
ssh_options_set(session, SSH_OPTIONS_GLOBAL_KNOWNHOSTS, "./unaccessible");
-
found = ssh_session_has_known_hosts_entry(session);
assert_int_equal(found, SSH_KNOWN_HOSTS_OK);
- assert_true(found == SSH_KNOWN_HOSTS_OK);
+ /* This makes sure the check will fail for an unknown host */
ssh_options_set(session, SSH_OPTIONS_HOST, "wurstbrot");
found = ssh_session_has_known_hosts_entry(session);
- assert_true(found == SSH_KNOWN_HOSTS_UNKNOWN);
+ assert_int_equal(found, SSH_KNOWN_HOSTS_UNKNOWN);
ssh_free(session);
}
@@ -414,17 +411,23 @@ static void torture_knownhosts_host_exists_global(void **state)
assert_non_null(session);
ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
+ ssh_options_set(session, SSH_OPTIONS_GLOBAL_KNOWNHOSTS, knownhosts_file);
+
/* This makes sure the user's known_hosts are not used */
ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, "/dev/null");
- ssh_options_set(session, SSH_OPTIONS_GLOBAL_KNOWNHOSTS, knownhosts_file);
+ found = ssh_session_has_known_hosts_entry(session);
+ assert_int_equal(found, SSH_KNOWN_HOSTS_OK);
+ /* This makes sure the check will not fail when the user's known_hosts is
+ * not accessible*/
+ ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, "./unaccessible");
found = ssh_session_has_known_hosts_entry(session);
assert_int_equal(found, SSH_KNOWN_HOSTS_OK);
- assert_true(found == SSH_KNOWN_HOSTS_OK);
+ /* This makes sure the check will fail for an unknown host */
ssh_options_set(session, SSH_OPTIONS_HOST, "wurstbrot");
found = ssh_session_has_known_hosts_entry(session);
- assert_true(found == SSH_KNOWN_HOSTS_UNKNOWN);
+ assert_int_equal(found, SSH_KNOWN_HOSTS_UNKNOWN);
ssh_free(session);
}
--
2.21.0
From b9530cedbeb169762307096dfeb485ab94e09740 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Fri, 28 Jun 2019 13:27:34 +0200
Subject: [PATCH 3/6] knownhosts: Read knownhosts file only if found
Avoid trying to open the files if they are not accessible. This was
already treated as a non-error, but with this we save one function call.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit e5a64a3d6b1b601cbaf207468a6658d1a4fa0031)
---
src/knownhosts.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/knownhosts.c b/src/knownhosts.c
index 9383cc97..8040a0c0 100644
--- a/src/knownhosts.c
+++ b/src/knownhosts.c
@@ -691,7 +691,7 @@ enum ssh_known_hosts_e ssh_session_has_known_hosts_entry(ssh_session session)
return SSH_KNOWN_HOSTS_ERROR;
}
- if (session->opts.knownhosts != NULL) {
+ if (known_hosts_found) {
rc = ssh_known_hosts_read_entries(host_port,
session->opts.knownhosts,
&entry_list);
@@ -702,7 +702,7 @@ enum ssh_known_hosts_e ssh_session_has_known_hosts_entry(ssh_session session)
}
}
- if (session->opts.global_knownhosts != NULL) {
+ if (global_known_hosts_found) {
rc = ssh_known_hosts_read_entries(host_port,
session->opts.global_knownhosts,
&entry_list);
--
2.21.0
From aaa978ad06ebdeff88a39b9a894696254263162c Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Fri, 28 Jun 2019 22:35:38 +0200
Subject: [PATCH 4/6] token: Added function to remove duplicates
Added a function to remove duplicates from lists. This function is used
in a new provided function to append lists removing duplicates.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 548753b3389518ebce98a7ddbf0640db3ad72de8)
---
include/libssh/token.h | 6 +-
src/token.c | 152 ++++++++++++++++++++++++++++++-
tests/unittests/torture_tokens.c | 122 +++++++++++++++++++++++++
3 files changed, 278 insertions(+), 2 deletions(-)
diff --git a/include/libssh/token.h b/include/libssh/token.h
index 7b244189..9896fb06 100644
--- a/include/libssh/token.h
+++ b/include/libssh/token.h
@@ -38,7 +38,11 @@ void ssh_tokens_free(struct ssh_tokens_st *tokens);
char *ssh_find_matching(const char *available_d,
const char *preferred_d);
-
char *ssh_find_all_matching(const char *available_d,
const char *preferred_d);
+
+char *ssh_remove_duplicates(const char *list);
+
+char *ssh_append_without_duplicates(const char *list,
+ const char *appended_list);
#endif /* TOKEN_H_ */
diff --git a/src/token.c b/src/token.c
index aee235ac..0924d3bd 100644
--- a/src/token.c
+++ b/src/token.c
@@ -26,6 +26,7 @@
#include <stdio.h>
#include <string.h>
+#include <stdbool.h>
#include "libssh/priv.h"
#include "libssh/token.h"
@@ -175,7 +176,7 @@ char *ssh_find_matching(const char *available_list,
for (i = 0; p_tok->tokens[i]; i++) {
for (j = 0; a_tok->tokens[j]; j++) {
- if (strcmp(a_tok->tokens[j], p_tok->tokens[i]) == 0){
+ if (strcmp(a_tok->tokens[j], p_tok->tokens[i]) == 0) {
ret = strdup(a_tok->tokens[j]);
goto out;
}
@@ -260,3 +261,152 @@ out:
ssh_tokens_free(p_tok);
return ret;
}
+
+/**
+ * @internal
+ *
+ * @brief Given a string containing a list of elements, remove all duplicates
+ * and return in a newly allocated string.
+ *
+ * @param[in] list The list to be freed of duplicates
+ *
+ * @return A newly allocated copy of the string free of duplicates; NULL in
+ * case of error.
+ */
+char *ssh_remove_duplicates(const char *list)
+{
+ struct ssh_tokens_st *tok = NULL;
+
+ size_t i, j, num_tokens, max_len;
+ char *ret = NULL;
+ bool *should_copy = NULL, need_comma = false;
+
+ if (list == NULL) {
+ return NULL;
+ }
+
+ /* The maximum number of tokens is the size of the list */
+ max_len = strlen(list);
+ if (max_len == 0) {
+ return NULL;
+ }
+
+ /* Add space for ending '\0' */
+ max_len++;
+
+ tok = ssh_tokenize(list, ',');
+ if ((tok == NULL) || (tok->tokens == NULL) || (tok->tokens[0] == NULL)) {
+ goto out;
+ }
+
+ should_copy = calloc(1, max_len);
+ if (should_copy == NULL) {
+ goto out;
+ }
+
+ if (strlen(tok->tokens[0]) > 0) {
+ should_copy[0] = true;
+ }
+
+ for (i = 1; tok->tokens[i]; i++) {
+ for (j = 0; j < i; j++) {
+ if (strcmp(tok->tokens[i], tok->tokens[j]) == 0) {
+ /* Found a duplicate; do not copy */
+ should_copy[i] = false;
+ break;
+ }
+ }
+
+ /* No matching token before */
+ if (j == i) {
+ /* Only copy if it is not an empty string */
+ if (strlen(tok->tokens[i]) > 0) {
+ should_copy[i] = true;
+ } else {
+ should_copy[i] = false;
+ }
+ }
+ }
+
+ num_tokens = i;
+
+ ret = calloc(1, max_len);
+ if (ret == NULL) {
+ goto out;
+ }
+
+ for (i = 0; i < num_tokens; i++) {
+ if (should_copy[i]) {
+ if (need_comma) {
+ strncat(ret, ",", (max_len - strlen(ret) - 1));
+ }
+ strncat(ret, tok->tokens[i], (max_len - strlen(ret) - 1));
+ need_comma = true;
+ }
+ }
+
+ /* If no comma is needed, nothing was copied */
+ if (!need_comma) {
+ SAFE_FREE(ret);
+ }
+
+out:
+ SAFE_FREE(should_copy);
+ ssh_tokens_free(tok);
+ return ret;
+}
+
+/**
+ * @internal
+ *
+ * @brief Given two strings containing lists of tokens, return a newly
+ * allocated string containing all the elements of the first list appended with
+ * all the elements of the second list, without duplicates. The order of the
+ * elements will be preserved.
+ *
+ * @param[in] list The first list
+ * @param[in] appended_list The list to be appended
+ *
+ * @return A newly allocated copy list containing all the elements of the
+ * kept_list appended with the elements of the appended_list without duplicates;
+ * NULL in case of error.
+ */
+char *ssh_append_without_duplicates(const char *list,
+ const char *appended_list)
+{
+ size_t concat_len = 0;
+ char *ret = NULL, *concat = NULL;
+
+ if (list != NULL) {
+ concat_len = strlen(list);
+ }
+
+ if (appended_list != NULL) {
+ concat_len += strlen(appended_list);
+ }
+
+ if (concat_len == 0) {
+ return NULL;
+ }
+
+ /* Add room for ending '\0' and for middle ',' */
+ concat_len += 2;
+ concat = calloc(1, concat_len);
+ if (concat == NULL) {
+ return NULL;
+ }
+
+ if (list != NULL) {
+ strcpy(concat, list);
+ strncat(concat, ",", concat_len - strlen(concat) - 1);
+ }
+ if (appended_list != NULL) {
+ strncat(concat, appended_list, concat_len - strlen(concat) - 1);
+ }
+
+ ret = ssh_remove_duplicates(concat);
+
+ SAFE_FREE(concat);
+
+ return ret;
+}
diff --git a/tests/unittests/torture_tokens.c b/tests/unittests/torture_tokens.c
index e192842f..6b52b847 100644
--- a/tests/unittests/torture_tokens.c
+++ b/tests/unittests/torture_tokens.c
@@ -146,6 +146,126 @@ static void torture_tokens_sanity(UNUSED_PARAM(void **state))
tokenize_compare_expected(",", single_colon, 1);
}
+static void torture_remove_duplicate(UNUSED_PARAM(void **state))
+{
+
+ const char *simple[] = {"a,a,b,b,c,c",
+ "a,b,c,a,b,c",
+ "a,b,c,c,b,a",
+ "a,a,,b,b,,c,c",
+ ",a,a,b,b,c,c",
+ "a,a,b,b,c,c,"};
+ const char *empty[] = {"",
+ ",,,,,,,,,",
+ NULL};
+ char *ret = NULL;
+ int i;
+
+ for (i = 0; i < 6; i++) {
+ ret = ssh_remove_duplicates(simple[i]);
+ assert_non_null(ret);
+ assert_string_equal("a,b,c", ret);
+ printf("simple[%d] resulted in '%s'\n", i, ret);
+ SAFE_FREE(ret);
+ }
+
+ for (i = 0; i < 3; i++) {
+ ret = ssh_remove_duplicates(empty[i]);
+ if (ret != NULL) {
+ printf("empty[%d] resulted in '%s'\n", i, ret);
+ }
+ assert_null(ret);
+ }
+
+ ret = ssh_remove_duplicates("a");
+ assert_non_null(ret);
+ assert_string_equal("a", ret);
+ SAFE_FREE(ret);
+}
+
+static void torture_append_without_duplicate(UNUSED_PARAM(void **state))
+{
+ const char *s1[] = {"a,a,b,b,c,c",
+ "a,b,c,a,b,c",
+ "a,b,c,c,b,a",
+ "a,a,,b,b,,c,c",
+ ",a,a,b,b,c,c",
+ "a,a,b,b,c,c,"};
+ const char *s2[] = {"a,a,b,b,c,c,d,d",
+ "a,b,c,d,a,b,c,d",
+ "a,b,c,d,d,c,b,a",
+ "a,a,,b,b,,c,c,,d,d",
+ ",a,a,b,b,c,c,d,d",
+ "a,a,b,b,c,c,d,d,",
+ "d"};
+ const char *empty[] = {"",
+ ",,,,,,,,,",
+ NULL,
+ NULL};
+ char *ret = NULL;
+ int i, j;
+
+ ret = ssh_append_without_duplicates("a", "a");
+ assert_non_null(ret);
+ assert_string_equal("a", ret);
+ SAFE_FREE(ret);
+
+ ret = ssh_append_without_duplicates("a", "b");
+ assert_non_null(ret);
+ assert_string_equal("a,b", ret);
+ SAFE_FREE(ret);
+
+ ret = ssh_append_without_duplicates("a", NULL);
+ assert_non_null(ret);
+ assert_string_equal("a", ret);
+ SAFE_FREE(ret);
+
+ ret = ssh_append_without_duplicates(NULL, "b");
+ assert_non_null(ret);
+ assert_string_equal("b", ret);
+ SAFE_FREE(ret);
+
+ for (i = 0; i < 6; i++) {
+ for (j = 0; j < 7; j++) {
+ ret = ssh_append_without_duplicates(s1[i], s2[j]);
+ assert_non_null(ret);
+ printf("s1[%d] + s2[%d] resulted in '%s'\n", i, j, ret);
+ assert_string_equal("a,b,c,d", ret);
+ SAFE_FREE(ret);
+ }
+ }
+
+ for (i = 0; i < 6; i++) {
+ for (j = 0; j < 3; j++) {
+ ret = ssh_append_without_duplicates(s1[i], empty[j]);
+ assert_non_null(ret);
+ printf("s1[%d] + empty[%d] resulted in '%s'\n", i, j, ret);
+ assert_string_equal("a,b,c", ret);
+ SAFE_FREE(ret);
+ }
+ }
+
+ for (i = 0; i < 3; i++) {
+ for (j = 0; j < 6; j++) {
+ ret = ssh_append_without_duplicates(empty[i], s1[j]);
+ assert_non_null(ret);
+ printf("empty[%d] + s1[%d] resulted in '%s'\n", i, j, ret);
+ assert_string_equal("a,b,c", ret);
+ SAFE_FREE(ret);
+ }
+ }
+ for (i = 0; i < 4; i++) {
+ for (j = 0; j < 4; j++) {
+ ret = ssh_append_without_duplicates(empty[i], empty[j]);
+ if (ret != NULL) {
+ printf("empty[%d] + empty[%d] resulted in '%s'\n", i, j, ret);
+ }
+ assert_null(ret);
+ }
+ }
+}
+
+
int torture_run_tests(void)
{
int rc;
@@ -153,6 +273,8 @@ int torture_run_tests(void)
cmocka_unit_test(torture_tokens_sanity),
cmocka_unit_test(torture_find_matching),
cmocka_unit_test(torture_find_all_matching),
+ cmocka_unit_test(torture_remove_duplicate),
+ cmocka_unit_test(torture_append_without_duplicate),
};
ssh_init();
--
2.21.0
From fa3caa61fdb39269cd78c4919df515b71991d231 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Tue, 2 Jul 2019 13:48:17 +0200
Subject: [PATCH 5/6] knownhosts: Introduced
ssh_known_hosts_get_algorithms_names()
The added internal function obtain a newly allocated string containing a
list of the signature types that can be generated by the keys present in
the known_hosts files, separated by commas.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 65a38759ca872e8bec0158ab3676e74b6afd336f)
---
include/libssh/knownhosts.h | 1 +
src/knownhosts.c | 141 +++++++++++++++++++
tests/unittests/torture_knownhosts_parsing.c | 28 ++++
3 files changed, 170 insertions(+)
diff --git a/include/libssh/knownhosts.h b/include/libssh/knownhosts.h
index dcaa6c24..44e434c0 100644
--- a/include/libssh/knownhosts.h
+++ b/include/libssh/knownhosts.h
@@ -23,6 +23,7 @@
#define SSH_KNOWNHOSTS_H_
struct ssh_list *ssh_known_hosts_get_algorithms(ssh_session session);
+char *ssh_known_hosts_get_algorithms_names(ssh_session session);
enum ssh_known_hosts_e
ssh_session_get_known_hosts_entry_file(ssh_session session,
const char *filename,
diff --git a/src/knownhosts.c b/src/knownhosts.c
index 8040a0c0..cf9d8a6b 100644
--- a/src/knownhosts.c
+++ b/src/knownhosts.c
@@ -42,6 +42,7 @@
#include "libssh/pki.h"
#include "libssh/dh.h"
#include "libssh/knownhosts.h"
+#include "libssh/token.h"
/**
* @addtogroup libssh_session
@@ -451,6 +452,146 @@ error:
return NULL;
}
+/**
+ * @internal
+ *
+ * @brief Returns a static string containing a list of the signature types the
+ * given key type can generate.
+ *
+ * @returns A static cstring containing the signature types the key is able to
+ * generate separated by commas; NULL in case of error
+ */
+static const char *ssh_known_host_sigs_from_hostkey_type(enum ssh_keytypes_e type)
+{
+ switch (type) {
+ case SSH_KEYTYPE_RSA:
+ return "rsa-sha2-512,rsa-sha2-256,ssh-rsa";
+ case SSH_KEYTYPE_ED25519:
+ return "ssh-ed25519";
+#ifdef HAVE_DSA
+ case SSH_KEYTYPE_DSS:
+ return "ssh-dss";
+#endif
+#ifdef HAVE_ECDH
+ case SSH_KEYTYPE_ECDSA_P256:
+ return "ecdsa-sha2-nistp256";
+ case SSH_KEYTYPE_ECDSA_P384:
+ return "ecdsa-sha2-nistp384";
+ case SSH_KEYTYPE_ECDSA_P521:
+ return "ecdsa-sha2-nistp521";
+#endif
+ case SSH_KEYTYPE_UNKNOWN:
+ default:
+ SSH_LOG(SSH_LOG_WARN, "The given type %d is not a base private key type "
+ "or is unsupported", type);
+ return NULL;
+ }
+}
+
+/**
+ * @internal
+ * @brief Get the host keys algorithms identifiers from the known_hosts files
+ *
+ * This expands the signatures types that can be generated from the keys types
+ * present in the known_hosts files
+ *
+ * @param[in] session The ssh session to use.
+ *
+ * @return A newly allocated cstring containing a list of signature algorithms
+ * that can be generated by the host using the keys listed in the known_hosts
+ * files, NULL on error.
+ */
+char *ssh_known_hosts_get_algorithms_names(ssh_session session)
+{
+ char methods_buffer[256 + 1] = {0};
+ struct ssh_list *entry_list = NULL;
+ struct ssh_iterator *it = NULL;
+ char *host_port = NULL;
+ size_t count;
+ bool needcomma = false;
+ char *names;
+
+ int rc;
+
+ if (session->opts.knownhosts == NULL ||
+ session->opts.global_knownhosts == NULL) {
+ if (ssh_options_apply(session) < 0) {
+ ssh_set_error(session,
+ SSH_REQUEST_DENIED,
+ "Can't find a known_hosts file");
+
+ return NULL;
+ }
+ }
+
+ host_port = ssh_session_get_host_port(session);
+ if (host_port == NULL) {
+ return NULL;
+ }
+
+ rc = ssh_known_hosts_read_entries(host_port,
+ session->opts.knownhosts,
+ &entry_list);
+ if (rc != 0) {
+ SAFE_FREE(host_port);
+ ssh_list_free(entry_list);
+ return NULL;
+ }
+
+ rc = ssh_known_hosts_read_entries(host_port,
+ session->opts.global_knownhosts,
+ &entry_list);
+ SAFE_FREE(host_port);
+ if (rc != 0) {
+ ssh_list_free(entry_list);
+ return NULL;
+ }
+
+ if (entry_list == NULL) {
+ return NULL;
+ }
+
+ count = ssh_list_count(entry_list);
+ if (count == 0) {
+ ssh_list_free(entry_list);
+ return NULL;
+ }
+
+ for (it = ssh_list_get_iterator(entry_list);
+ it != NULL;
+ it = ssh_list_get_iterator(entry_list))
+ {
+ struct ssh_knownhosts_entry *entry = NULL;
+ const char *algo = NULL;
+
+ entry = ssh_iterator_value(struct ssh_knownhosts_entry *, it);
+ algo = ssh_known_host_sigs_from_hostkey_type(entry->publickey->type);
+ if (algo == NULL) {
+ continue;
+ }
+
+ if (needcomma) {
+ strncat(methods_buffer,
+ ",",
+ sizeof(methods_buffer) - strlen(methods_buffer) - 1);
+ }
+
+ strncat(methods_buffer,
+ algo,
+ sizeof(methods_buffer) - strlen(methods_buffer) - 1);
+ needcomma = true;
+
+ ssh_knownhosts_entry_free(entry);
+ ssh_list_remove(entry_list, it);
+ }
+
+ ssh_list_free(entry_list);
+
+ names = ssh_remove_duplicates(methods_buffer);
+
+ return names;
+}
+
/**
* @brief Parse a line from a known_hosts entry into a structure
*
diff --git a/tests/unittests/torture_knownhosts_parsing.c b/tests/unittests/torture_knownhosts_parsing.c
index a087caef..6952e858 100644
--- a/tests/unittests/torture_knownhosts_parsing.c
+++ b/tests/unittests/torture_knownhosts_parsing.c
@@ -369,6 +369,31 @@ static void torture_knownhosts_read_file(void **state)
ssh_list_free(entry_list);
}
+static void torture_knownhosts_get_algorithms_names(void **state)
+{
+ const char *knownhosts_file = *state;
+ ssh_session session;
+ const char *expect = "ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa";
+ char *names = NULL;
+ bool process_config = false;
+
+ session = ssh_new();
+ assert_non_null(session);
+
+ /* This makes sure the global configuration file is not processed */
+ ssh_options_set(session, SSH_OPTIONS_PROCESS_CONFIG, &process_config);
+
+ ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
+ ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, knownhosts_file);
+
+ names = ssh_known_hosts_get_algorithms_names(session);
+ assert_non_null(names);
+ assert_string_equal(names, expect);
+
+ SAFE_FREE(names);
+ ssh_free(session);
+}
+
#ifndef _WIN32 /* There is no /dev/null on Windows */
static void torture_knownhosts_host_exists(void **state)
{
@@ -510,6 +535,9 @@ int torture_run_tests(void) {
cmocka_unit_test_setup_teardown(torture_knownhosts_read_file,
setup_knownhosts_file_duplicate,
teardown_knownhosts_file),
+ cmocka_unit_test_setup_teardown(torture_knownhosts_get_algorithms_names,
+ setup_knownhosts_file,
+ teardown_knownhosts_file),
#ifndef _WIN32
cmocka_unit_test_setup_teardown(torture_knownhosts_host_exists,
setup_knownhosts_file,
--
2.21.0
From 1fd68ec732214a12ba3f59ca23f80463411e22dd Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Mon, 1 Jul 2019 19:39:07 +0200
Subject: [PATCH 6/6] kex: Do not ignore keys in known_hosts files
Previously, if the SSH_OPTIONS_HOSTKEYS option was set by any mean,
including the client configuration file, the keys in known_hosts files
wouldn't be considered before advertising the list of wanted host keys.
This could result in the client requesting the server to provide a
signature using a key not present in the known_hosts files (e.g. when
the first wanted algorithm in SSH_OPTIONS_HOSTKEYS is not present in the
known_hosts files), causing a host key mismatch and possible key
rejection.
Now, the keys present in the known_hosts files are prioritized over the
other wanted keys. This do not change the fact that only keys of types
present in the list set in SSH_OPTIONS_HOSTKEYS will be accepted and
prioritized following the order defined by such list.
The new wanted list of hostkeys is given by:
- The keys present in known_hosts files, ordered by preference defined
in SSH_OPTIONS_HOSTKEYS. If the option is not set, a default order
of preference is used.
- The other keys present in the same option are appended without adding
duplicates. If the option is not set, the default list of keys is
used.
Fixes: T156
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit f18a7cc17e399ae7bc92f707da3a676c52fd948e)
---
src/kex.c | 165 +++++++++----------
tests/unittests/torture_knownhosts_parsing.c | 148 ++++++++++++++++-
2 files changed, 223 insertions(+), 90 deletions(-)
diff --git a/src/kex.c b/src/kex.c
index 6ea5e8ba..0d4cad6d 100644
--- a/src/kex.c
+++ b/src/kex.c
@@ -561,103 +561,94 @@ void ssh_list_kex(struct ssh_kex_struct *kex) {
/**
* @internal
+ *
* @brief selects the hostkey mechanisms to be chosen for the key exchange,
- * as some hostkey mechanisms may be present in known_hosts file and preferred
+ * as some hostkey mechanisms may be present in known_hosts files.
+ *
* @returns a cstring containing a comma-separated list of hostkey methods.
* NULL if no method matches
*/
char *ssh_client_select_hostkeys(ssh_session session)
{
- char methods_buffer[128]={0};
- char tail_buffer[128]={0};
+ const char *wanted = NULL;
+ char *wanted_without_certs = NULL;
+ char *known_hosts_algorithms = NULL;
+ char *known_hosts_ordered = NULL;
char *new_hostkeys = NULL;
- static const char *preferred_hostkeys[] = {
- "ssh-ed25519",
- "ecdsa-sha2-nistp521",
- "ecdsa-sha2-nistp384",
- "ecdsa-sha2-nistp256",
- "rsa-sha2-512",
- "rsa-sha2-256",
- "ssh-rsa",
-#ifdef HAVE_DSA
- "ssh-dss",
-#endif
- NULL
- };
- struct ssh_list *algo_list = NULL;
- struct ssh_iterator *it = NULL;
- size_t algo_count;
- int needcomma = 0;
- size_t i, len;
-
- algo_list = ssh_known_hosts_get_algorithms(session);
- if (algo_list == NULL) {
- return NULL;
+ char *fips_hostkeys = NULL;
+
+ wanted = session->opts.wanted_methods[SSH_HOSTKEYS];
+ if (wanted == NULL) {
+ if (ssh_fips_mode()) {
+ wanted = ssh_kex_get_fips_methods(SSH_HOSTKEYS);
+ } else {
+ wanted = ssh_kex_get_default_methods(SSH_HOSTKEYS);
+ }
}
- algo_count = ssh_list_count(algo_list);
- if (algo_count == 0) {
- ssh_list_free(algo_list);
+ /* This removes the certificate types, unsupported for now */
+ wanted_without_certs = ssh_find_all_matching(HOSTKEYS, wanted);
+ if (wanted_without_certs == NULL) {
+ SSH_LOG(SSH_LOG_WARNING,
+ "List of allowed host key algorithms is empty or contains only "
+ "unsupported algorithms");
return NULL;
}
- for (i = 0; preferred_hostkeys[i] != NULL; ++i) {
- bool found = false;
- /* This is a signature type: We list also the SHA2 extensions */
- enum ssh_keytypes_e base_preferred =
- ssh_key_type_from_signature_name(preferred_hostkeys[i]);
-
- for (it = ssh_list_get_iterator(algo_list);
- it != NULL;
- it = it->next) {
- const char *algo = ssh_iterator_value(const char *, it);
- /* This is always key type so we do not have to care for the
- * SHA2 extension */
- enum ssh_keytypes_e base_algo = ssh_key_type_from_name(algo);
-
- if (base_preferred == base_algo) {
- /* Matching the keys already verified it is a known type */
- if (needcomma) {
- strncat(methods_buffer,
- ",",
- sizeof(methods_buffer) - strlen(methods_buffer) - 1);
- }
- strncat(methods_buffer,
- preferred_hostkeys[i],
- sizeof(methods_buffer) - strlen(methods_buffer) - 1);
- needcomma = 1;
- found = true;
- }
- }
- /* Collect the rest of the algorithms in other buffer, that will
- * follow the preferred buffer. This will signalize all the algorithms
- * we are willing to accept.
- */
- if (!found) {
- snprintf(tail_buffer + strlen(tail_buffer),
- sizeof(tail_buffer) - strlen(tail_buffer),
- ",%s", preferred_hostkeys[i]);
- }
+ SSH_LOG(SSH_LOG_DEBUG,
+ "Order of wanted host keys: \"%s\"",
+ wanted_without_certs);
+
+ known_hosts_algorithms = ssh_known_hosts_get_algorithms_names(session);
+ if (known_hosts_algorithms == NULL) {
+ SSH_LOG(SSH_LOG_DEBUG,
+ "No key found in known_hosts; "
+ "changing host key method to \"%s\"",
+ wanted_without_certs);
+
+ return wanted_without_certs;
}
- ssh_list_free(algo_list);
- if (strlen(methods_buffer) == 0) {
+ SSH_LOG(SSH_LOG_DEBUG,
+ "Algorithms found in known_hosts files: \"%s\"",
+ known_hosts_algorithms);
+
+ /* Filter and order the keys from known_hosts according to wanted list */
+ known_hosts_ordered = ssh_find_all_matching(known_hosts_algorithms,
+ wanted_without_certs);
+ SAFE_FREE(known_hosts_algorithms);
+ if (known_hosts_ordered == NULL) {
SSH_LOG(SSH_LOG_DEBUG,
- "No supported kex method for existing key in known_hosts file");
- return NULL;
+ "No key found in known_hosts is allowed; "
+ "changing host key method to \"%s\"",
+ wanted_without_certs);
+
+ return wanted_without_certs;
}
- /* Append the supported list to the preferred.
- * The length is maximum 128 + 128 + 1, which will not overflow
- */
- len = strlen(methods_buffer) + strlen(tail_buffer) + 1;
- new_hostkeys = malloc(len);
+ /* Append the other supported keys after the preferred ones
+ * This function tolerates NULL pointers in parameters */
+ new_hostkeys = ssh_append_without_duplicates(known_hosts_ordered,
+ wanted_without_certs);
+ SAFE_FREE(known_hosts_ordered);
+ SAFE_FREE(wanted_without_certs);
if (new_hostkeys == NULL) {
ssh_set_error_oom(session);
return NULL;
}
- snprintf(new_hostkeys, len,
- "%s%s", methods_buffer, tail_buffer);
+
+ if (ssh_fips_mode()) {
+ /* Filter out algorithms not allowed in FIPS mode */
+ fips_hostkeys = ssh_keep_fips_algos(SSH_HOSTKEYS, new_hostkeys);
+ SAFE_FREE(new_hostkeys);
+ if (fips_hostkeys == NULL) {
+ SSH_LOG(SSH_LOG_WARNING,
+ "None of the wanted host keys or keys in known_hosts files "
+ "is allowed in FIPS mode.");
+ return NULL;
+ }
+ new_hostkeys = fips_hostkeys;
+ }
SSH_LOG(SSH_LOG_DEBUG,
"Changing host key method to \"%s\"",
@@ -672,7 +663,7 @@ char *ssh_client_select_hostkeys(ssh_session session)
*/
int ssh_set_client_kex(ssh_session session)
{
- struct ssh_kex_struct *client= &session->next_crypto->client_kex;
+ struct ssh_kex_struct *client = &session->next_crypto->client_kex;
const char *wanted;
char *kex = NULL;
char *kex_tmp = NULL;
@@ -687,14 +678,22 @@ int ssh_set_client_kex(ssh_session session)
}
memset(client->methods, 0, KEX_METHODS_SIZE * sizeof(char **));
- /* first check if we have specific host key methods */
- if (session->opts.wanted_methods[SSH_HOSTKEYS] == NULL) {
- /* Only if no override */
- session->opts.wanted_methods[SSH_HOSTKEYS] =
- ssh_client_select_hostkeys(session);
- }
+ /* Set the list of allowed algorithms in order of preference, if it hadn't
+ * been set yet. */
for (i = 0; i < KEX_METHODS_SIZE; i++) {
+ if (i == SSH_HOSTKEYS) {
+ /* Set the hostkeys in the following order:
+ * - First: keys present in known_hosts files ordered by preference
+ * - Next: other wanted algorithms ordered by preference */
+ client->methods[i] = ssh_client_select_hostkeys(session);
+ if (client->methods[i] == NULL) {
+ ssh_set_error_oom(session);
+ return SSH_ERROR;
+ }
+ continue;
+ }
+
wanted = session->opts.wanted_methods[i];
if (wanted == NULL) {
if (ssh_fips_mode()) {
diff --git a/tests/unittests/torture_knownhosts_parsing.c b/tests/unittests/torture_knownhosts_parsing.c
index 6952e858..1c2ccc10 100644
--- a/tests/unittests/torture_knownhosts_parsing.c
+++ b/tests/unittests/torture_knownhosts_parsing.c
@@ -28,6 +28,8 @@
#define TMP_FILE_NAME "/tmp/known_hosts_XXXXXX"
+const char template[] = "temp_dir_XXXXXX";
+
static int setup_knownhosts_file(void **state)
{
char *tmp_file = NULL;
@@ -394,6 +396,115 @@ static void torture_knownhosts_get_algorithms_names(void **state)
ssh_free(session);
}
+static void torture_knownhosts_algorithms_wanted(void **state)
+{
+ const char *knownhosts_file = *state;
+ char *algo_list = NULL;
+ ssh_session session;
+ bool process_config = false;
+ const char *wanted = "ecdsa-sha2-nistp384,ecdsa-sha2-nistp256,"
+ "rsa-sha2-256,ecdsa-sha2-nistp521";
+ const char *expect = "rsa-sha2-256,ecdsa-sha2-nistp384,"
+ "ecdsa-sha2-nistp256,ecdsa-sha2-nistp521";
+ int verbose = 4;
+
+ session = ssh_new();
+ assert_non_null(session);
+
+ ssh_options_set(session, SSH_OPTIONS_LOG_VERBOSITY, &verbose);
+
+ /* This makes sure the global configuration file is not processed */
+ ssh_options_set(session, SSH_OPTIONS_PROCESS_CONFIG, &process_config);
+
+ /* Set the wanted list of hostkeys, ordered by preference */
+ ssh_options_set(session, SSH_OPTIONS_HOSTKEYS, wanted);
+
+ ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
+ ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, knownhosts_file);
+
+ algo_list = ssh_client_select_hostkeys(session);
+ assert_non_null(algo_list);
+ assert_string_equal(algo_list, expect);
+ free(algo_list);
+
+ ssh_free(session);
+}
+
+static void torture_knownhosts_algorithms_negative(UNUSED_PARAM(void **state))
+{
+ const char *wanted = NULL;
+ const char *expect = NULL;
+
+ char *algo_list = NULL;
+
+ char *cwd = NULL;
+ char *tmp_dir = NULL;
+
+ bool process_config = false;
+ int verbose = 4;
+ int rc = 0;
+
+ ssh_session session;
+ /* Create temporary directory */
+ cwd = torture_get_current_working_dir();
+ assert_non_null(cwd);
+
+ tmp_dir = torture_make_temp_dir(template);
+ assert_non_null(tmp_dir);
+
+ rc = torture_change_dir(tmp_dir);
+ assert_int_equal(rc, 0);
+
+ session = ssh_new();
+ assert_non_null(session);
+
+ ssh_options_set(session, SSH_OPTIONS_LOG_VERBOSITY, &verbose);
+ ssh_options_set(session, SSH_OPTIONS_PROCESS_CONFIG, &process_config);
+ ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
+
+ /* Test with unknown key type in known_hosts */
+ wanted = "rsa-sha2-256";
+ ssh_options_set(session, SSH_OPTIONS_HOSTKEYS, wanted);
+ torture_write_file("unknown_key_type", "localhost unknown AAAABBBBCCCC");
+ ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, "unknown_key_type");
+ algo_list = ssh_client_select_hostkeys(session);
+ assert_non_null(algo_list);
+ assert_string_equal(algo_list, wanted);
+ SAFE_FREE(algo_list);
+
+ /* Test with unsupported, but existing types */
+ wanted = "rsa-sha2-256-cert-v01@openssh.com,"
+ "rsa-sha2-512-cert-v01@openssh.com";
+ ssh_options_set(session, SSH_OPTIONS_HOSTKEYS, wanted);
+ algo_list = ssh_client_select_hostkeys(session);
+ assert_null(algo_list);
+
+ /* In FIPS mode, test filtering keys not allowed */
+ if (ssh_fips_mode()) {
+ wanted = "ssh-ed25519,rsa-sha2-256,ssh-rsa";
+ expect = "rsa-sha2-256";
+ ssh_options_set(session, SSH_OPTIONS_HOSTKEYS, wanted);
+ torture_write_file("no_fips", LOCALHOST_DEFAULT_ED25519);
+ ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, "no_fips");
+ algo_list = ssh_client_select_hostkeys(session);
+ assert_non_null(algo_list);
+ assert_string_equal(algo_list, expect);
+ SAFE_FREE(algo_list);
+ }
+
+ ssh_free(session);
+
+ /* Teardown */
+ rc = torture_change_dir(cwd);
+ assert_int_equal(rc, 0);
+
+ rc = torture_rmdirs(tmp_dir);
+ assert_int_equal(rc, 0);
+
+ SAFE_FREE(tmp_dir);
+ SAFE_FREE(cwd);
+}
+
#ifndef _WIN32 /* There is no /dev/null on Windows */
static void torture_knownhosts_host_exists(void **state)
{
@@ -457,12 +568,12 @@ static void torture_knownhosts_host_exists_global(void **state)
ssh_free(session);
}
-static void
-torture_knownhosts_algorithms(void **state)
+static void torture_knownhosts_algorithms(void **state)
{
const char *knownhosts_file = *state;
char *algo_list = NULL;
ssh_session session;
+ bool process_config = false;
const char *expect = "ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa,"
"ecdsa-sha2-nistp521,ecdsa-sha2-nistp384,"
"ecdsa-sha2-nistp256"
@@ -470,10 +581,15 @@ torture_knownhosts_algorithms(void **state)
",ssh-dss"
#endif
;
+ const char *expect_fips = "rsa-sha2-512,rsa-sha2-256,ecdsa-sha2-nistp521,"
+ "ecdsa-sha2-nistp384,ecdsa-sha2-nistp256";
session = ssh_new();
assert_non_null(session);
+ /* This makes sure the global configuration file is not processed */
+ ssh_options_set(session, SSH_OPTIONS_PROCESS_CONFIG, &process_config);
+
ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, knownhosts_file);
/* This makes sure the system's known_hosts are not used */
@@ -481,18 +597,22 @@ torture_knownhosts_algorithms(void **state)
algo_list = ssh_client_select_hostkeys(session);
assert_non_null(algo_list);
- assert_string_equal(algo_list, expect);
+ if (ssh_fips_mode()) {
+ assert_string_equal(algo_list, expect_fips);
+ } else {
+ assert_string_equal(algo_list, expect);
+ }
free(algo_list);
ssh_free(session);
}
-static void
-torture_knownhosts_algorithms_global(void **state)
+static void torture_knownhosts_algorithms_global(void **state)
{
const char *knownhosts_file = *state;
char *algo_list = NULL;
ssh_session session;
+ bool process_config = false;
const char *expect = "ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa,"
"ecdsa-sha2-nistp521,ecdsa-sha2-nistp384,"
"ecdsa-sha2-nistp256"
@@ -500,10 +620,15 @@ torture_knownhosts_algorithms_global(void **state)
",ssh-dss"
#endif
;
+ const char *expect_fips = "rsa-sha2-512,rsa-sha2-256,ecdsa-sha2-nistp521,"
+ "ecdsa-sha2-nistp384,ecdsa-sha2-nistp256";
session = ssh_new();
assert_non_null(session);
+ /* This makes sure the global configuration file is not processed */
+ ssh_options_set(session, SSH_OPTIONS_PROCESS_CONFIG, &process_config);
+
ssh_options_set(session, SSH_OPTIONS_HOST, "localhost");
/* This makes sure the current-user's known hosts are not used */
ssh_options_set(session, SSH_OPTIONS_KNOWNHOSTS, "/dev/null");
@@ -511,12 +636,17 @@ torture_knownhosts_algorithms_global(void **state)
algo_list = ssh_client_select_hostkeys(session);
assert_non_null(algo_list);
- assert_string_equal(algo_list, expect);
+ if (ssh_fips_mode()) {
+ assert_string_equal(algo_list, expect_fips);
+ } else {
+ assert_string_equal(algo_list, expect);
+ }
free(algo_list);
ssh_free(session);
}
-#endif
+
+#endif /* _WIN32 There is no /dev/null on Windows */
int torture_run_tests(void) {
int rc;
@@ -538,6 +668,10 @@ int torture_run_tests(void) {
cmocka_unit_test_setup_teardown(torture_knownhosts_get_algorithms_names,
setup_knownhosts_file,
teardown_knownhosts_file),
+ cmocka_unit_test_setup_teardown(torture_knownhosts_algorithms_wanted,
+ setup_knownhosts_file,
+ teardown_knownhosts_file),
+ cmocka_unit_test(torture_knownhosts_algorithms_negative),
#ifndef _WIN32
cmocka_unit_test_setup_teardown(torture_knownhosts_host_exists,
setup_knownhosts_file,
--
2.21.0