From 0b80601609aaa1b8435367f8f608d46fc24e6c95 Mon Sep 17 00:00:00 2001 From: Niklas Moser Date: Mon, 28 Sep 2026 12:23:11 +0200 Subject: [PATCH] session server ssh FEATURE lock a user out after repeated failed password authentication Nothing reads ssh_auth_attempts, so password guessing is unlimited both within a connection and across them. auth-timeout bounds how long one authentication may take, not how many may be tried, and pam_faillock only ever sees the keyboard-interactive method. Add max-auth-attempts to the SSH client-authentication of libnetconf2-netconf-server, which disconnects a session after that many failed authentication attempts, and an opt-in, server-wide ssh-password-lockout container under ln2-netconf-server. When present, consecutive failed password-based authentications are counted per user across connections and all SSH endpoints, listening and Call Home, and the user is refused password-based authentication for duration seconds after max-consecutive-failures. The hooks sit on the three credential checks in session_server_ssh.c that both auth backends funnel through, so the password, system keyboard-interactive and PAM methods share one tally. A custom keyboard-interactive callback is not covered. The tally is kept in memory and only ever holds users known to the server. Public key auth is deliberately not counted, which keeps a locked out deployment recoverable. Motivated by 3GPP TS 33.117 4.2.3.4.5 (consecutive failed login attempts) and 4.2.3.4.3.3 (brute force and dictionary attacks). Co-Authored-By: Claude Opus 5.5 (1M context) --- ...ibnetconf2-netconf-server@2026-09-18.yang} | 69 +++++ src/server_config.c | 61 ++++ src/session_p.h | 39 +++ src/session_server.c | 7 + src/session_server_ssh.c | 263 +++++++++++++++++- src/session_server_ssh_auth_callback.c | 24 +- src/session_server_ssh_auth_message.c | 8 +- src/session_server_ssh_wrapper.h | 11 +- tests/test_ssh.c | 67 ++++- 9 files changed, 521 insertions(+), 28 deletions(-) rename modules/{libnetconf2-netconf-server@2026-04-17.yang => libnetconf2-netconf-server@2026-09-18.yang} (86%) diff --git a/modules/libnetconf2-netconf-server@2026-04-17.yang b/modules/libnetconf2-netconf-server@2026-09-18.yang similarity index 86% rename from modules/libnetconf2-netconf-server@2026-04-17.yang rename to modules/libnetconf2-netconf-server@2026-09-18.yang index 4b88a94c..8b141ae5 100644 --- a/modules/libnetconf2-netconf-server@2026-04-17.yang +++ b/modules/libnetconf2-netconf-server@2026-09-18.yang @@ -31,6 +31,10 @@ module libnetconf2-netconf-server { prefix tlss; } + revision "2026-09-18" { + description "Added max-auth-attempts and password-based authentication lockout configuration."; + } + revision "2026-04-17" { description "Change SSH banner description, reference and string length to reflect its correct purpose."; } @@ -159,6 +163,19 @@ module libnetconf2-netconf-server { description "Represents the maximum amount of seconds an authentication can go on for."; } + + leaf max-auth-attempts { + type uint16; + default 0; + description + "Maximum number of failed authentication attempts allowed within a single SSH session, + after which the session is disconnected. Every rejected authentication request counts, + including the initial 'none' request most clients send and every public key a client + offers that the server does not accept, so this must be set high enough not to + disconnect legitimate clients. + + The value 0 means no limit."; + } } grouping ssh-server-banner-grouping { @@ -461,6 +478,58 @@ module libnetconf2-netconf-server { } } + container ssh-password-lockout { + presence + "Enables locking a user out of password-based authentication after repeated failures."; + + description + "Temporarily locks a user out of password-based authentication after too many consecutive + failures. Applies to all SSH endpoints, both listening and Call Home. + + Password-based authentication is the 'password' method and the 'keyboard-interactive' + method with 'use-system-auth'. + + While locked out, even the correct password is rejected. A successful password-based + authentication resets the count. + + Public key authentication is not affected."; + + reference + "3GPP TS 33.117: Catalogue of general security assurance requirements, + section 4.2.3.4.5 Policy regarding consecutive failed login attempts and + section 4.2.3.4.3.3 Protection against brute force and dictionary attacks"; + + leaf max-consecutive-failures { + type uint16 { + range "1..max"; + } + default 5; + description + "Number of consecutive failed password-based authentications of a user after which the + user is locked out."; + } + + leaf duration { + type uint16 { + range "1..max"; + } + default 300; + units "seconds"; + description + "How long a user stays locked out."; + } + + leaf reset-interval { + type uint16 { + range "1..max"; + } + default 900; + units "seconds"; + description + "The count of consecutive failures of a user is reset if no failure occurs for this long."; + } + } + leaf-list ignored-hello-module { type string; diff --git a/src/server_config.c b/src/server_config.c index 1efeb562..12633b95 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -1665,6 +1665,15 @@ config_ssh_auth_timeout(const struct lyd_node *node, enum nc_operation UNUSED(pa return 0; } +static int +config_ssh_max_auth_attempts(const struct lyd_node *node, enum nc_operation UNUSED(parent_op), + struct nc_server_ssh_opts *ssh) +{ + /* default value always present */ + ssh->max_auth_attempts = strtoul(lyd_get_value(node), NULL, 10); + return 0; +} + static int config_endpt_reference(const struct lyd_node *node, enum nc_operation parent_op, char **endpt_ref) { @@ -1716,6 +1725,12 @@ config_ssh_client_auth(const struct lyd_node *node, enum nc_operation parent_op, NC_CHECK_RET(config_ssh_auth_timeout(n, op, ssh)); } + /* config max auth attempts per session (augment) */ + nc_lyd_find_child_optional(node, "libnetconf2-netconf-server:max-auth-attempts", &n); + if (n) { + NC_CHECK_RET(config_ssh_max_auth_attempts(n, op, ssh)); + } + /* config endpoint reference (augment) */ nc_lyd_find_child_optional(node, "libnetconf2-netconf-server:endpoint-reference", &n); if (n) { @@ -5343,6 +5358,43 @@ config_ignored_hello_module(const struct lyd_node *node, enum nc_operation paren return 0; } +#ifdef NC_ENABLED_SSH_TLS + +static int +config_ssh_password_lockout(const struct lyd_node *node, enum nc_operation parent_op, struct nc_server_config *config) +{ + enum nc_operation op; + struct lyd_node *n; + + NC_NODE_GET_OP(node, parent_op, &op); + + if (op == NC_OP_DELETE) { + /* the container is gone, so the lockout is off again; max_fails of 0 disables it */ + config->authlock.max_fails = 0; + config->authlock.duration = 0; + config->authlock.reset_interval = 0; + return 0; + } + + /* default values always present */ + nc_lyd_find_child_optional(node, "max-consecutive-failures", &n); + if (n) { + config->authlock.max_fails = strtoul(lyd_get_value(n), NULL, 10); + } + nc_lyd_find_child_optional(node, "duration", &n); + if (n) { + config->authlock.duration = strtoul(lyd_get_value(n), NULL, 10); + } + nc_lyd_find_child_optional(node, "reset-interval", &n); + if (n) { + config->authlock.reset_interval = strtoul(lyd_get_value(n), NULL, 10); + } + + return 0; +} + +#endif /* NC_ENABLED_SSH_TLS */ + static int config_ln2_netconf_server(const struct lyd_node *node, enum nc_operation parent_op, struct nc_server_config *config) @@ -5361,6 +5413,12 @@ config_ln2_netconf_server(const struct lyd_node *node, enum nc_operation parent_ if (n) { NC_CHECK_RET(config_cert_exp_notif_intervals(n, op, config)); } + + /* config ssh-password-lockout */ + nc_lyd_find_child_optional(node, "ssh-password-lockout", &n); + if (n) { + NC_CHECK_RET(config_ssh_password_lockout(n, op, config)); + } #endif /* NC_ENABLED_SSH_TLS */ /* config all ignored-hello-modules */ @@ -5537,6 +5595,7 @@ nc_server_config_ssh_dup(const struct nc_server_ssh_opts *src, struct nc_server_ } (*dst)->auth_timeout = src->auth_timeout; + (*dst)->max_auth_attempts = src->max_auth_attempts; cleanup: if (rc) { @@ -6047,6 +6106,8 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config dst->cert_exp_notif_intervals[i] = src->cert_exp_notif_intervals[i]; LYA_INCREMENT(dst->cert_exp_notif_intervals); } + + dst->authlock = src->authlock; #endif /* NC_ENABLED_SSH_TLS */ cleanup: diff --git a/src/session_p.h b/src/session_p.h index fca13c12..565bf2b0 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -410,6 +410,16 @@ struct nc_hostkey { }; }; +/** + * @brief Password-based authentication lockout policy of the server, shared by all the SSH endpoints. + */ +struct nc_authlock_opts { + uint16_t max_fails; /**< consecutive failed password-based authentications that lock a user out, + 0 if the lockout is disabled */ + uint16_t duration; /**< how long a user stays locked out, seconds */ + uint16_t reset_interval; /**< the count of a user is reset if no failure occurs for this long, seconds */ +}; + /** * @brief Server options for configuring the SSH transport protocol. */ @@ -428,6 +438,9 @@ struct nc_server_ssh_opts { char *banner; /**< SSH banner message, sent before authentication. */ uint16_t auth_timeout; /**< Authentication timeout. */ + + uint16_t max_auth_attempts; /**< Failed authentication attempts allowed within a single session, + 0 for no limit. */ }; /** @@ -828,6 +841,8 @@ struct nc_server_config { struct nc_cert_exp_time anchor; /**< Lower bound of the given interval. */ struct nc_cert_exp_time period; /**< Period of the given interval. */ } *cert_exp_notif_intervals; /**< Certificate expiration notification intervals (sized-array, see libyang docs). */ + + struct nc_authlock_opts authlock; /**< SSH password-based authentication lockout policy. */ #endif /* NC_ENABLED_SSH_TLS */ }; @@ -947,6 +962,22 @@ struct nc_server_opts { pthread_mutex_t lock; /**< Certificate expiration notification thread's data and cond lock. */ pthread_cond_t cond; /**< Condition for the certificate expiration notification thread. */ } cert_exp_notif; + + /* ACCESS locked - authlock lock - leaf lock, never acquire another lock while holding it */ + pthread_mutex_t authlock_lock; /**< Lock for the password-based authentication lockout tally. */ + + /** + * @brief Failed password-based authentication tally of a single user. + * + * Shared by all the SSH endpoints, an entry is created on the first failure of a user. Only users + * known to the server ever get one, so the tally is bounded by the number of users. + */ + struct nc_authlock_entry { + char *username; /**< User the tally belongs to. */ + uint32_t fails; /**< Consecutive failed password-based authentications. */ + time_t last_fail; /**< When the last one was. */ + time_t locked_until; /**< No password-based authentication before this, 0 if not locked out. */ + } *authlock; /**< Password-based authentication lockout tally (sized-array, see libyang docs). */ #endif /* NC_ENABLED_SSH_TLS */ /** @@ -1162,6 +1193,7 @@ struct nc_session { ATOMIC_T *ch_thread_running; uint16_t ssh_auth_attempts; /**< number of failed SSH authentication attempts */ + void *client_cert; /**< TLS client certificate if used for authentication */ #endif /* NC_ENABLED_SSH_TLS */ } server; @@ -1738,6 +1770,13 @@ void nc_server_ch_thread_names_free(char **names); */ int nc_server_ch_threads_destroy(void); +/** + * @brief Free the password-based authentication lockout tally. + * + * Must not be called before every thread that may authenticate a client has been joined. + */ +void nc_server_ssh_authlock_free(void); + /** * @brief Stop a dispatched Call Home client thread, if such thread was dispatched for the given client. * diff --git a/src/session_server.c b/src/session_server.c index 15608196..37813316 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -60,6 +60,9 @@ struct nc_server_opts server_opts = { .binds_lock = PTHREAD_MUTEX_INITIALIZER, .opts_lock = PTHREAD_RWLOCK_INITIALIZER, .ch_threads_lock = PTHREAD_MUTEX_INITIALIZER, +#ifdef NC_ENABLED_SSH_TLS + .authlock_lock = PTHREAD_MUTEX_INITIALIZER, +#endif /* NC_ENABLED_SSH_TLS */ }; static nc_rpc_clb global_rpc_clb = NULL; @@ -1696,6 +1699,10 @@ nc_server_destroy(void) nc_server_config_release(config); #ifdef NC_ENABLED_SSH_TLS + /* free the password-based authentication lockout tally; only safe here, once the Call Home and + * accept threads that authenticate clients have been joined */ + nc_server_ssh_authlock_free(); + /* close the TLS keylog file */ if (server_opts.tls_keylog_file) { fclose(server_opts.tls_keylog_file); diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c index 32f03599..272847de 100644 --- a/src/session_server_ssh.c +++ b/src/session_server_ssh.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -48,6 +49,198 @@ #include "session_server_ssh_wrapper.h" #include "session_wrapper.h" +/* + * Password-based authentication lockout. + * + * Consecutive failed password-based authentications are counted per user across connections and + * all the SSH endpoints, and the user is refused password-based authentication for a while once too + * many fail in a row. The policy is server-wide, in ::nc_server_config. The tally only ever holds + * users the server knows: with local users, unknown users are rejected before any credential check, + * and without them only the failures of users that exist on the system are counted. + * + * Password-based authentication is the password method and the system keyboard-interactive method. + * A custom keyboard-interactive callback is not covered, what it verifies is up to the application. + * Public key and certificate authentication are deliberately left alone, which keeps a locked out + * deployment recoverable. + */ + +/** + * @brief Find the password-based authentication lockout tally of a user. Expects the lock to be held. + * + * @param[in] username User to look for. + * @return Its entry, NULL if it has none. + */ +static struct nc_authlock_entry * +nc_authlock_find(const char *username) +{ + LYA_COUNT_T u; + + LYA_FOR(server_opts.authlock, u) { + if (!strcmp(server_opts.authlock[u].username, username)) { + return &server_opts.authlock[u]; + } + } + + return NULL; +} + +/** + * @brief Remove a password-based authentication lockout tally. Expects the lock to be held. + * + * @param[in] entry Entry to remove. + */ +static void +nc_authlock_remove(struct nc_authlock_entry *entry) +{ + struct nc_authlock_entry *last = &server_opts.authlock[LYA_COUNT(server_opts.authlock) - 1]; + + free(entry->username); + if (entry != last) { + *entry = *last; + } + LYA_DECREMENT_FREE(server_opts.authlock); +} + +/** + * @brief Remove the tallies that are neither locked out nor recent enough to count towards a lockout. + * Expects the lock to be held. + * + * Also drops the tallies of users removed from the configuration, once they expire. + * + * @param[in] reset_interval Failures further apart than this do not count towards the same tally, seconds. + * @param[in] now Current time. + */ +static void +nc_authlock_prune(uint16_t reset_interval, time_t now) +{ + LYA_COUNT_T u = 0; + + while (u < LYA_COUNT(server_opts.authlock)) { + if ((server_opts.authlock[u].locked_until <= now) && + ((now - server_opts.authlock[u].last_fail) > reset_interval)) { + /* the last item takes its place */ + nc_authlock_remove(&server_opts.authlock[u]); + } else { + ++u; + } + } +} + +/** + * @brief Record the outcome of a password-based authentication. + * + * Does nothing unless the lockout is configured. + * + * @param[in] session NETCONF session, for the policy and the log message. + * @param[in] username User that authenticated. + * @param[in] success Whether it succeeded, which resets the count. + */ +static void +nc_authlock_record(struct nc_session *session, const char *username, int success) +{ + const struct nc_authlock_opts *opts = &session->opts.server.config->authlock; + struct nc_authlock_entry *entry; + time_t now = time(NULL); + char *name; + + if (!opts->max_fails || !username) { + /* the lockout is not configured */ + return; + } + + /* AUTHLOCK LOCK */ + pthread_mutex_lock(&server_opts.authlock_lock); + + if (success) { + entry = nc_authlock_find(username); + if (entry) { + nc_authlock_remove(entry); + } + goto cleanup; + } + + nc_authlock_prune(opts->reset_interval, now); + + entry = nc_authlock_find(username); + if (!entry) { + name = strdup(username); + if (!name) { + ERRMEM; + goto cleanup; + } + LYA_ADD_ITEM(server_opts.authlock, entry, ERRMEM; free(name); goto cleanup); + entry->username = name; + } else if (entry->locked_until && (entry->locked_until <= now)) { + /* the lockout expired, this failure starts a new count rather than locking the user out again */ + entry->fails = 0; + entry->locked_until = 0; + } + + ++entry->fails; + entry->last_fail = now; + + if (entry->fails >= opts->max_fails) { + entry->locked_until = now + opts->duration; + WRN(session, "User \"%s\" locked out of password-based authentication for %" PRIu16 " s after %" PRIu32 + " consecutive failures.", username, opts->duration, entry->fails); + } + +cleanup: + /* AUTHLOCK UNLOCK */ + pthread_mutex_unlock(&server_opts.authlock_lock); +} + +void +nc_server_ssh_authlock_free(void) +{ + LYA_COUNT_T u; + + LYA_FOR(server_opts.authlock, u) { + free(server_opts.authlock[u].username); + } + LYA_FREE(server_opts.authlock); + server_opts.authlock = NULL; +} + +/** + * @brief Check whether a user is locked out of password-based authentication and log it if it is. + * + * Always allows the authentication unless the lockout is configured. + * + * @param[in] session NETCONF session, for the policy and the log message. + * @param[in] username User to check. + * @return 0 if it may authenticate, 1 if it is locked out. + */ +static int +nc_authlock_denied(struct nc_session *session, const char *username) +{ + struct nc_authlock_entry *entry; + time_t now = time(NULL), remaining = 0; + + if (!session->opts.server.config->authlock.max_fails || !username) { + return 0; + } + + /* AUTHLOCK LOCK */ + pthread_mutex_lock(&server_opts.authlock_lock); + + entry = nc_authlock_find(username); + if (entry && (entry->locked_until > now)) { + remaining = entry->locked_until - now; + } + + /* AUTHLOCK UNLOCK */ + pthread_mutex_unlock(&server_opts.authlock_lock); + + if (!remaining) { + return 0; + } + + WRN(session, "User \"%s\" is locked out of password-based authentication for another %lld s.", + username, (long long)remaining); + return 1; +} + int nc_ssh_check_local_user_support(struct nc_session *session) { @@ -155,11 +348,29 @@ nc_ssh_auth_success(struct nc_session *session, struct nc_auth_state *auth_state } void -nc_server_ssh_auth_attempt_failed(struct nc_session *session) +nc_server_ssh_auth_attempt_failed(struct nc_session *session, const struct nc_server_ssh_opts *opts) { + uint16_t max_fails = opts->max_auth_attempts; + ++session->opts.server.ssh_auth_attempts; - VRB(session, "Failed user \"%s\" authentication attempt (#%d).", + VRB(session, "Failed user \"%s\" authentication attempt (#%" PRIu16 ").", session->username ? session->username : "unknown", session->opts.server.ssh_auth_attempts); + + /* every rejected credential gets here, including every public key the client offers that is not + * accepted, so the cap is off unless the endpoint configures max-auth-attempts */ + if (!max_fails || (session->opts.server.ssh_auth_attempts < max_fails)) { + return; + } + + /* the per-session cap, which bounds what a single connection may try; the accept loops end on a + * session that is no longer connected */ + if (NC_SESSION_STATUS_GET(session) != NC_STATUS_INVALID) { + ERR(session, "Too many failed authentication attempts (%" PRIu16 ") in a single session, disconnecting.", + session->opts.server.ssh_auth_attempts); + NC_SESSION_STATUS_SET(session, NC_STATUS_INVALID); + NC_SESSION_TERM_REASON_SET(session, NC_SESSION_TERM_OTHER); + ssh_disconnect(session->ti.libssh.session); + } } int @@ -171,6 +382,11 @@ nc_server_ssh_auth_password_check(struct nc_session *session, const char *user, assert(!local_users_supported || auth_client); + /* refuse a user that failed password-based authentication too many times */ + if (nc_authlock_denied(session, user)) { + return 1; + } + /* Get the stored password */ if (local_users_supported) { stored_password = auth_client->password; @@ -199,6 +415,9 @@ nc_server_ssh_auth_password_check(struct nc_session *session, const char *user, free(stored_password); } + /* a password that worked resets the user's count, one that did not counts against it */ + nc_authlock_record(session, user, rc ? 0 : 1); + return rc; } @@ -447,9 +666,17 @@ nc_server_ssh_pam_authenticate(struct nc_session *session, const char *username, const struct pam_conv *conv) { pam_handle_t *pam_h = NULL; - char *pam_config_name = NULL; + char *pam_config_name = NULL, *pw_str = NULL; + struct passwd pw; + size_t pw_str_size = 0; int ret; + /* refuse a user that failed password-based authentication too many times; pam_faillock, where it + * is configured, only sees the PAM methods, this tally is shared with the other ones */ + if (nc_authlock_denied(session, username)) { + return 1; + } + /* get the PAM configuration, PAM must not be called with the lock held */ if (nc_server_ssh_get_pam_conf_filename(&pam_config_name)) { return 1; @@ -474,9 +701,22 @@ nc_server_ssh_pam_authenticate(struct nc_session *session, const char *username, } else { VRB(session, "PAM error occurred (%s).", pam_strerror(pam_h, ret)); } + + /* only a rejected credential counts towards the lockout; an aborted, unavailable or + * misconfigured PAM stack is not the client getting the password wrong. Without local users + * any username reaches PAM, which commonly rejects a nonexistent user the same way, so only + * a user that exists gets a tally. */ + if (((ret == PAM_AUTH_ERR) || (ret == PAM_CRED_INSUFFICIENT) || (ret == PAM_MAXTRIES)) && + session->opts.server.config->authlock.max_fails && nc_getpw(0, username, &pw, &pw_str, &pw_str_size)) { + nc_authlock_record(session, username, 0); + } goto cleanup; } + /* the credential was accepted, which resets the count whatever the account management below + * has to say about the user */ + nc_authlock_record(session, username, 1); + /* correct token entered, check other requirements (the time of the day, expired token, ...) */ ret = pam_acct_mgmt(pam_h, 0); if ((ret != PAM_SUCCESS) && (ret != PAM_NEW_AUTHTOK_REQD)) { @@ -501,6 +741,7 @@ nc_server_ssh_pam_authenticate(struct nc_session *session, const char *username, ERR(NULL, "PAM error occurred (%s).", pam_strerror(pam_h, ret)); } free(pam_config_name); + free(pw_str); return ret; } @@ -1186,6 +1427,10 @@ nc_server_ssh_kbdint_verify_passwd(struct nc_session *session, const char *usern const char *answer; int rc; + if (nc_authlock_denied(session, username)) { + return 1; + } + if (n_answers != 1) { ERR(session, "Unexpected amount of answers in system auth. Expected 1, got \"%d\".", n_answers); return 1; @@ -1213,6 +1458,8 @@ nc_server_ssh_kbdint_verify_passwd(struct nc_session *session, const char *usern free(pw); free(received_pw); + nc_authlock_record(session, username, rc ? 0 : 1); + return rc; } @@ -1766,7 +2013,10 @@ nc_accept_ssh_session_auth(struct nc_session *session, struct nc_server_ssh_opts /* Run the event loop instead of ssh_message_get() */ while (!(session->flags & NC_SESSION_SSH_AUTHENTICATED)) { if (!ssh_is_connected(session->ti.libssh.session)) { - ERR(session, "Communication SSH socket unexpectedly closed."); + if (NC_SESSION_STATUS_GET(session) != NC_STATUS_INVALID) { + /* not disconnected by the server, which already logged the reason */ + ERR(session, "Communication SSH socket unexpectedly closed."); + } return -1; } @@ -1798,7 +2048,10 @@ nc_accept_ssh_session_auth(struct nc_session *session, struct nc_server_ssh_opts #else while (1) { if (!ssh_is_connected(session->ti.libssh.session)) { - ERR(session, "Communication SSH socket unexpectedly closed while waiting for authentication."); + if (NC_SESSION_STATUS_GET(session) != NC_STATUS_INVALID) { + /* not disconnected by the server, which already logged the reason */ + ERR(session, "Communication SSH socket unexpectedly closed while waiting for authentication."); + } return -1; } diff --git a/src/session_server_ssh_auth_callback.c b/src/session_server_ssh_auth_callback.c index 7df33317..08dbf5c7 100644 --- a/src/session_server_ssh_auth_callback.c +++ b/src/session_server_ssh_auth_callback.c @@ -490,7 +490,7 @@ nc_server_ssh_cb_auth_common_setup(struct nc_server_ssh_cb_data *cb_data, const *auth_client = NULL; if (!user) { - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); return -1; } @@ -506,7 +506,7 @@ nc_server_ssh_cb_auth_common_setup(struct nc_server_ssh_cb_data *cb_data, const ERR(session, "User \"%s\" changed its username to \"%s\".", session->username, user); NC_SESSION_STATUS_SET(session, NC_STATUS_INVALID); NC_SESSION_TERM_REASON_SET(session, NC_SESSION_TERM_OTHER); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); return -1; } @@ -514,7 +514,7 @@ nc_server_ssh_cb_auth_common_setup(struct nc_server_ssh_cb_data *cb_data, const *local_users_supported = nc_ssh_check_local_user_support(session); if (*local_users_supported < 0) { /* fatal error checking local users support */ - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); return -1; } @@ -526,7 +526,7 @@ nc_server_ssh_cb_auth_common_setup(struct nc_server_ssh_cb_data *cb_data, const ERR(session, "User \"%s\" not known by the server.", user); /* advertise only publickey so there is no interaction and it is simply denied */ ssh_set_auth_methods(session->ti.libssh.session, SSH_AUTH_METHOD_PUBLICKEY); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); return -1; } } @@ -554,7 +554,7 @@ nc_server_ssh_cb_auth_none(ssh_session UNUSED(libssh_sess), const char *user, vo return nc_ssh_auth_success(session, &cb_data->auth_state, SSH_AUTH_METHOD_NONE); } - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } @@ -576,7 +576,7 @@ nc_server_ssh_cb_auth_password(ssh_session UNUSED(libssh_sess), const char *user if (rc == 0) { return nc_ssh_auth_success(session, &cb_data->auth_state, SSH_AUTH_METHOD_PASSWORD); } else { - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } } @@ -604,11 +604,11 @@ nc_server_ssh_cb_auth_pubkey(ssh_session UNUSED(libssh_sess), const char *user, return nc_ssh_auth_success(session, &cb_data->auth_state, SSH_AUTH_METHOD_PUBLICKEY); } else { VRB(session, "User \"%s\" tried to use an invalid public key signature.", session->username); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } } else { - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } } @@ -682,7 +682,7 @@ nc_server_ssh_cb_auth_kbdint(ssh_message message, ssh_session UNUSED(libssh_sess /* select the kbdint backend based on the configuration */ if (nc_server_ssh_kbdint_select_method(session, local_users_supported, auth_client, &backend)) { /* denied, the reason was already logged */ - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); #ifdef HAVE_LIBPAM /* cancel any in-progress PAM exchange before denying */ nc_server_ssh_cb_kbdint_pam_cancel_stored(cb_data); @@ -693,13 +693,13 @@ nc_server_ssh_cb_auth_kbdint(ssh_message message, ssh_session UNUSED(libssh_sess if (backend == NC_KBDINT_BACKEND_CUSTOM_CLB) { /* custom interactive auth callback, it must not be called with the options lock held */ if (nc_server_ssh_get_interactive_auth_clb(&interactive_auth_clb, &interactive_auth_data)) { - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } if (!interactive_auth_clb) { /* the callback was unset in the meantime */ ERR(session, "Custom keyboard-interactive authentication callback not set."); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } @@ -719,7 +719,7 @@ nc_server_ssh_cb_auth_kbdint(ssh_message message, ssh_session UNUSED(libssh_sess return nc_ssh_auth_success(session, &cb_data->auth_state, SSH_AUTH_METHOD_INTERACTIVE); } else { VRB(session, "User \"%s\" authentication denied via keyboard-interactive.", user); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, cb_data->opts); return SSH_AUTH_DENIED; } } diff --git a/src/session_server_ssh_auth_message.c b/src/session_server_ssh_auth_message.c index 56b82908..bc7b0122 100644 --- a/src/session_server_ssh_auth_message.c +++ b/src/session_server_ssh_auth_message.c @@ -417,7 +417,7 @@ nc_server_ssh_msg_auth(struct nc_session *session, struct nc_server_ssh_opts *op ERR(session, "User \"%s\" changed its username to \"%s\".", session->username, username); NC_SESSION_STATUS_SET(session, NC_STATUS_INVALID); NC_SESSION_TERM_REASON_SET(session, NC_SESSION_TERM_OTHER); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); return 1; } @@ -429,7 +429,7 @@ nc_server_ssh_msg_auth(struct nc_session *session, struct nc_server_ssh_opts *op * there is no interaction and it will simply be denied */ ERR(session, "User \"%s\" not known by the server.", username); ssh_set_auth_methods(session->ti.libssh.session, SSH_AUTH_METHOD_PUBLICKEY); - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); ssh_message_reply_default(msg); return 0; } @@ -452,8 +452,8 @@ nc_server_ssh_msg_auth(struct nc_session *session, struct nc_server_ssh_opts *op } else if (method == SSH_AUTH_METHOD_INTERACTIVE) { ret = nc_server_ssh_msg_auth_kbdint(session, local_users_supported, auth_client, msg); } else { - ++session->opts.server.ssh_auth_attempts; VRB(session, "Authentication method \"%s\" not supported.", str_method); + nc_server_ssh_auth_attempt_failed(session, opts); ssh_message_reply_default(msg); return 0; } @@ -468,7 +468,7 @@ nc_server_ssh_msg_auth(struct nc_session *session, struct nc_server_ssh_opts *op } } else if (ret == 1) { /* failed attempt, msg wasnt yet replied to */ - nc_server_ssh_auth_attempt_failed(session); + nc_server_ssh_auth_attempt_failed(session, opts); ssh_message_reply_default(msg); } diff --git a/src/session_server_ssh_wrapper.h b/src/session_server_ssh_wrapper.h index 827445d2..54d8fee8 100644 --- a/src/session_server_ssh_wrapper.h +++ b/src/session_server_ssh_wrapper.h @@ -132,8 +132,8 @@ struct nc_server_ssh_cb_data { /** * @brief SSH server options, a pointer into a configuration generation. * - * Only valid during the transport handshake - it is dereferenced solely by - * ::nc_server_ssh_cb_auth_common_setup(), reached only from the four authentication callbacks. + * Only valid during the transport handshake - it is dereferenced solely from the four + * authentication callbacks. * The long-lived channel callbacks use only @p session. It is cleared at the end of the * handshake so that a later dereference fails immediately instead of reading freed memory. */ @@ -335,9 +335,14 @@ int nc_server_ssh_compare_password(const char *stored_pw, const char *received_p /** * @brief Increase the failed authentication attempt counter and log the attempt. * + * Disconnects the session once the counter reaches the endpoint's max-auth-attempts, which is + * unlimited unless configured. Every rejected credential is counted, including every public key + * the client offers that the server does not accept. + * * @param[in] session NETCONF session. + * @param[in] opts SSH server options of the endpoint. */ -void nc_server_ssh_auth_attempt_failed(struct nc_session *session); +void nc_server_ssh_auth_attempt_failed(struct nc_session *session, const struct nc_server_ssh_opts *opts); /** * @brief Authenticate user with password (retrieves stored hash and compares). diff --git a/tests/test_ssh.c b/tests/test_ssh.c index 3ecf2a25..278bed7e 100644 --- a/tests/test_ssh.c +++ b/tests/test_ssh.c @@ -35,6 +35,7 @@ struct test_ssh_data { const char *privkey_path; int check_protocol_string; int expect_fail; + int bad_password; }; int TEST_PORT = 10050; @@ -69,12 +70,13 @@ __wrap_ssh_get_issue_banner(ssh_session session) static char * auth_password(const char *username, const char *hostname, void *priv) { + const struct test_ssh_data *test_data = priv; + (void) hostname; - (void) priv; /* set the reply to password authentication */ if (!strcmp(username, "test_pw")) { - return strdup("testpw"); + return strdup((test_data && test_data->bad_password) ? "not-testpw" : "testpw"); } else { return NULL; } @@ -119,7 +121,7 @@ client_thread_ssh(void *arg) ret = nc_client_ssh_add_keypair(test_data->pubkey_path, test_data->privkey_path); assert_int_equal(ret, 0); } else { - nc_client_ssh_set_auth_password_clb(auth_password, NULL); + nc_client_ssh_set_auth_password_clb(auth_password, test_data); } /* wait for the server to be ready */ @@ -164,6 +166,50 @@ test_password(void **state) } } +static int setup_ssh(void **state); + +/** @brief Whether ::setup_ssh() configures the password-based authentication lockout on the endpoint. */ +static int setup_ssh_with_lockout; + +static int +setup_ssh_lockout(void **state) +{ + int ret; + + setup_ssh_with_lockout = 1; + ret = setup_ssh(state); + setup_ssh_with_lockout = 0; + + return ret; +} + +static void +test_password_lockout(void **state) +{ + int ret, i, round; + pthread_t tids[2]; + struct ln2_test_ctx *test_ctx = *state; + struct test_ssh_data *test_data = test_ctx->test_data; + + test_data->username = "test_pw"; + test_data->expect_fail = 1; + + /* the first round is refused because the password is wrong, the second one because that single + * failure locked the user out - the password the client sends there is the correct one */ + for (round = 0; round < 2; ++round) { + test_data->bad_password = (round == 0); + + ret = pthread_create(&tids[0], NULL, client_thread_ssh, *state); + assert_int_equal(ret, 0); + ret = pthread_create(&tids[1], NULL, ln2_glob_test_server_thread_fail, *state); + assert_int_equal(ret, 0); + + for (i = 0; i < 2; i++) { + pthread_join(tids[i], NULL); + } + } +} + static void test_none(void **state) { @@ -709,7 +755,7 @@ static int setup_ssh(void **state) { int ret; - struct lyd_node *tree = NULL; + struct lyd_node *tree = NULL, *lockout; struct ln2_test_ctx *test_ctx; struct test_ssh_data *test_data; @@ -751,6 +797,18 @@ setup_ssh(void **state) "ssh-server-parameters/client-authentication/users/user[name='test_none']/none", NULL, 0, NULL); assert_int_equal(ret, 0); + if (setup_ssh_with_lockout) { + /* one failed password is enough to lock the user out, so that the test does not depend on + * how many times the client retries within a single connection */ + ret = lyd_new_path(tree, test_ctx->ctx, "/libnetconf2-netconf-server:ln2-netconf-server/ssh-password-lockout/" + "max-consecutive-failures", "1", 0, &lockout); + assert_int_equal(ret, 0); + + /* a sibling of the tree, so the implicit nodes below do not reach it */ + ret = lyd_new_implicit_tree(lockout, LYD_IMPLICIT_NO_STATE, NULL); + assert_int_equal(ret, 0); + } + /* add all the default nodes/np containers */ ret = lyd_new_implicit_tree(tree, LYD_IMPLICIT_NO_STATE, NULL); assert_int_equal(ret, 0); @@ -769,6 +827,7 @@ main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test_setup_teardown(test_password, setup_ssh, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_password_lockout, setup_ssh_lockout, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_none, setup_ssh, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_rsa_pubkey, setup_ssh, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_ec256_pubkey, setup_ssh, ln2_glob_test_teardown),