From 942e4143cfdbd0adefad29d93b70db4dde49ba48 Mon Sep 17 00:00:00 2001 From: roman Date: Mon, 2 Jun 2025 12:24:54 +0200 Subject: [PATCH 1/4] session server ssh BUGFIX kbdint auth change Fix a bug where changing config from kbdint auth to any other type of auth requires you to still auth via kbdint. This commit also makes it possible for further extension of kbdint auth methods. --- ...libnetconf2-netconf-server@2025-01-23.yang | 2 - src/server_config.c | 33 ++++++++++++- src/session_p.h | 24 ++++++--- src/session_server_ssh.c | 49 ++++++++++++++----- 4 files changed, 83 insertions(+), 25 deletions(-) diff --git a/modules/libnetconf2-netconf-server@2025-01-23.yang b/modules/libnetconf2-netconf-server@2025-01-23.yang index e47ac302..59fc2e65 100644 --- a/modules/libnetconf2-netconf-server@2025-01-23.yang +++ b/modules/libnetconf2-netconf-server@2025-01-23.yang @@ -312,7 +312,6 @@ module libnetconf2-netconf-server { "Grouping for the SSH Keyboard interactive authentication method."; container keyboard-interactive { - presence "Indicates that the given client supports the SSH Keyboard Interactive authentication method."; description "Keyboard interactive SSH authentication method."; @@ -322,7 +321,6 @@ module libnetconf2-netconf-server { the Secure Shell Protocol (SSH)"; choice method { - mandatory true; description "Method to perform the authentication with."; diff --git a/src/server_config.c b/src/server_config.c index 985cb186..ffbede28 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -2188,6 +2188,33 @@ nc_server_config_password(const struct lyd_node *node, enum nc_operation op) return ret; } +static int +nc_server_config_keyboard_interactive(const struct lyd_node *node, enum nc_operation op) +{ + struct nc_auth_client *auth_client; + struct nc_ch_client *ch_client = NULL; + + assert(!strcmp(LYD_NAME(node), "keyboard-interactive")); + + if (op != NC_OP_DELETE) { + /* only do something on delete */ + return 0; + } + + if (is_ch(node) && nc_server_config_get_ch_client(node, &ch_client)) { + return 1; + } + + if (nc_server_config_get_auth_client(node, ch_client, &auth_client)) { + return 1; + } + + /* delete the keyboard-interactive authentication method */ + auth_client->kbdint_method = NC_KBDINT_AUTH_METHOD_NONE; + + return 0; +} + static int nc_server_config_use_system_auth(const struct lyd_node *node, enum nc_operation op) { @@ -2207,9 +2234,9 @@ nc_server_config_use_system_auth(const struct lyd_node *node, enum nc_operation } if (op == NC_OP_CREATE) { - auth_client->kb_int_enabled = 1; + auth_client->kbdint_method = NC_KBDINT_AUTH_METHOD_SYSTEM; } else { - auth_client->kb_int_enabled = 0; + auth_client->kbdint_method = NC_KBDINT_AUTH_METHOD_NONE; } cleanup: @@ -3598,6 +3625,8 @@ nc_server_config_parse_netconf_server(const struct lyd_node *node, enum nc_opera ret = nc_server_config_idle_time(node, op); } else if (!strcmp(name, "keepalives")) { ret = nc_server_config_keepalives(node, op); + } else if (!strcmp(name, "keyboard-interactive")) { + ret = nc_server_config_keyboard_interactive(node, op); } else if (!strcmp(name, "key-exchange-alg")) { ret = nc_server_config_kex_alg(node, op); } else if (!strcmp(name, "local-address")) { diff --git a/src/session_p.h b/src/session_p.h index 80270cf7..e95a7ea4 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -90,6 +90,14 @@ enum nc_privkey_format { NC_PRIVKEY_FORMAT_UNKNOWN /**< Unknown format */ }; +/** + * @brief Enumeration of SSH keyboard-interactive authentication methods. + */ +enum nc_kbdint_auth_method { + NC_KBDINT_AUTH_METHOD_NONE = 0, /**< Keyboard-interactive authentication disabled. */ + NC_KBDINT_AUTH_METHOD_SYSTEM /**< Keyboard-interactive authentication is left to the system (PAM, shadow, etc.). */ +}; + /** * @brief A basic certificate. */ @@ -175,20 +183,20 @@ struct nc_auth_state { * @brief A server's authorized client. */ struct nc_auth_client { - char *username; /**< Arbitrary username. */ + char *username; /**< Arbitrary username. */ - enum nc_store_type store; /**< Specifies how/where the client's public key is stored. */ + enum nc_store_type store; /**< Specifies how/where the client's public key is stored. */ union { struct { - struct nc_public_key *pubkeys; /**< The client's public keys. */ - uint16_t pubkey_count; /**< The number of client's public keys. */ + struct nc_public_key *pubkeys; /**< The client's public keys. */ + uint16_t pubkey_count; /**< The number of client's public keys. */ }; - char *ts_ref; /**< Name of the referenced truststore key. */ + char *ts_ref; /**< Name of the referenced truststore key. */ }; - char *password; /**< Client's password */ - int kb_int_enabled; /**< Indicates that the client supports keyboard-interactive authentication. */ - int none_enabled; /**< Implies that the client supports the none authentication method. */ + char *password; /**< Client's password */ + enum nc_kbdint_auth_method kbdint_method; /**< Keyboard-interactive authentication method. */ + int none_enabled; /**< Implies that the client supports the none authentication method. */ }; /** diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c index 01e03cf5..1b0163d5 100644 --- a/src/session_server_ssh.c +++ b/src/session_server_ssh.c @@ -923,7 +923,7 @@ nc_server_ssh_auth_kbdint_pam(struct nc_session *session, const char *username, * @return 0 on success, non-zero otherwise. */ static int -nc_server_ssh_auth_kbdint_system(struct nc_session *session, const char *username, ssh_message msg) +nc_server_ssh_auth_kbdint_passwd(struct nc_session *session, const char *username, ssh_message msg) { int ret = 0, n_answers; const char *name = "Keyboard-Interactive Authentication"; @@ -975,6 +975,32 @@ nc_server_ssh_auth_kbdint_system(struct nc_session *session, const char *usernam #endif +/** + * @brief Keyboard-interactive authentication method using the system's authentication methods. + * + * @param[in] session NETCONF session. + * @param[in] msg SSH message with a keyboard-interactive authentication request. + * @return 0 on success, non-zero otherwise. + */ +static int +nc_server_ssh_auth_kbdint_system(struct nc_session *session, ssh_message msg) +{ + int rc; + +#ifdef HAVE_LIBPAM + /* authenticate using PAM */ + rc = nc_server_ssh_auth_kbdint_pam(session, session->username, msg); +#elif defined (HAVE_SHADOW) + /* authenticate using /etc/passwd and /etc/shadow */ + rc = nc_server_ssh_auth_kbdint_passwd(session, session->username, msg); +#else + ERR(NULL, "Keyboard-interactive method not supported."); + rc = 1; +#endif + + return rc; +} + API void nc_server_ssh_set_interactive_auth_clb(int (*interactive_auth_clb)(const struct nc_session *session, ssh_session ssh_sess, ssh_message msg, void *user_data), void *user_data, void (*free_user_data)(void *user_data)) @@ -1319,22 +1345,19 @@ nc_server_ssh_auth_kbdint(struct nc_session *session, int local_users_supported, assert(!local_users_supported || auth_client); - if (local_users_supported && !auth_client->kb_int_enabled) { + if (local_users_supported && !auth_client->kbdint_method) { VRB(session, "User \"%s\" does not have Keyboard-interactive method configured, but a request was received.", session->username); return 1; } else if (server_opts.interactive_auth_clb) { rc = server_opts.interactive_auth_clb(session, session->ti.libssh.session, msg, server_opts.interactive_auth_data); } else { -#ifdef HAVE_LIBPAM - /* authenticate using PAM */ - rc = nc_server_ssh_auth_kbdint_pam(session, session->username, msg); -#elif defined (HAVE_SHADOW) - /* authenticate using the system */ - rc = nc_server_ssh_auth_kbdint_system(session, session->username, msg); -#else - ERR(NULL, "Keyboard-interactive method not supported."); - return 1; -#endif + /* perform the authentication based on the configured method */ + if (auth_client->kbdint_method == NC_KBDINT_AUTH_METHOD_SYSTEM) { + rc = nc_server_ssh_auth_kbdint_system(session, msg); + } else { + ERR(session, "Keyboard-interactive authentication method not supported."); + rc = 1; + } } return rc ? 1 : 0; @@ -1509,7 +1532,7 @@ nc_server_ssh_auth(struct nc_session *session, struct nc_server_ssh_opts *opts, auth_state->methods |= SSH_AUTH_METHOD_PASSWORD; auth_state->method_count++; } - if (auth_client->kb_int_enabled) { + if (auth_client->kbdint_method) { auth_state->methods |= SSH_AUTH_METHOD_INTERACTIVE; auth_state->method_count++; } From 5eb86e59b7312de58c8550865a4a23cd3b2375ca Mon Sep 17 00:00:00 2001 From: roman Date: Mon, 2 Jun 2025 13:00:41 +0200 Subject: [PATCH 2/4] valgrind supp BUGFIX supp pam_start_confdir leak --- tests/library_valgrind.supp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/library_valgrind.supp b/tests/library_valgrind.supp index 7b0ae3b2..2e52ea86 100644 --- a/tests/library_valgrind.supp +++ b/tests/library_valgrind.supp @@ -9,19 +9,11 @@ fun:ly_ctx_new } { - CI:test_pam:__wrap_pam_start + test_pam:__wrap_pam_start_leak Memcheck:Leak match-leak-kinds: definite fun:malloc ... - fun:ln2_glob_test_server_thread - fun:ln2_glob_test_server_thread -} -{ - test_pam:__wrap_pam_start - Memcheck:Leak - match-leak-kinds: definite - fun:malloc + fun:__wrap_pam_start ... - fun:ln2_glob_test_server_thread } From 64b0354e7e1dbee0d59895868e245fdf4394cf95 Mon Sep 17 00:00:00 2001 From: roman Date: Mon, 2 Jun 2025 13:14:10 +0200 Subject: [PATCH 3/4] session_p UPDATE add a comment about kbdint union --- src/session_p.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/session_p.h b/src/session_p.h index e95a7ea4..e252d526 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -195,7 +195,8 @@ struct nc_auth_client { }; char *password; /**< Client's password */ - enum nc_kbdint_auth_method kbdint_method; /**< Keyboard-interactive authentication method. */ + enum nc_kbdint_auth_method kbdint_method; /**< Keyboard-interactive authentication method, + * may be extended by e.g. a union for each method when needed. */ int none_enabled; /**< Implies that the client supports the none authentication method. */ }; From 9445cdac219ecea0c78e0eb83803fb8337297c4f Mon Sep 17 00:00:00 2001 From: roman Date: Mon, 2 Jun 2025 13:22:11 +0200 Subject: [PATCH 4/4] ln2 netconf server UPDATE revision to 2025-06-02 --- ...-01-23.yang => libnetconf2-netconf-server@2025-06-02.yang} | 4 ++++ 1 file changed, 4 insertions(+) rename modules/{libnetconf2-netconf-server@2025-01-23.yang => libnetconf2-netconf-server@2025-06-02.yang} (99%) diff --git a/modules/libnetconf2-netconf-server@2025-01-23.yang b/modules/libnetconf2-netconf-server@2025-06-02.yang similarity index 99% rename from modules/libnetconf2-netconf-server@2025-01-23.yang rename to modules/libnetconf2-netconf-server@2025-06-02.yang index 59fc2e65..8a2e62b0 100644 --- a/modules/libnetconf2-netconf-server@2025-01-23.yang +++ b/modules/libnetconf2-netconf-server@2025-06-02.yang @@ -31,6 +31,10 @@ module libnetconf2-netconf-server { prefix tlss; } + revision "2025-06-02" { + description "Removed presence from the container."; + } + revision "2025-01-23" { description "Added a list of YANG modules skipped in the server message."; }