From 30f3cc9263de9b063b7b04de51d8cc0ca240f589 Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Tue, 1 Oct 2019 22:34:31 +0300 Subject: [PATCH] auth: disable legacy auth by default Legacy authentication can expose password in plaintext. Since this is not widely used mechanism, disable it by default. It can be enabled back with connection option XMPP_CONN_FLAG_LEGACY_AUTH. --- ChangeLog | 2 + src/auth.c | 222 +++++++++++++++++++++++++-------------------------- src/common.h | 1 + src/conn.c | 6 +- strophe.h | 4 + 5 files changed, 122 insertions(+), 113 deletions(-) diff --git a/ChangeLog b/ChangeLog index e71d563..6616bb4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,6 @@ 0.9.3 + - Legacy authentication is disabled by default, can be enabled with + connection flag XMPP_CONN_FLAG_LEGACY_AUTH - Session is not established if it is optional - Fixed a bug causing a reused connection not to cleanup properly - Improved debug logging in OpenSSL module diff --git a/src/auth.c b/src/auth.c index 6872b83..a23504e 100644 --- a/src/auth.c +++ b/src/auth.c @@ -61,6 +61,7 @@ #endif static void _auth(xmpp_conn_t * const conn); +static void _auth_legacy(xmpp_conn_t *conn); static void _handle_open_sasl(xmpp_conn_t * const conn); static void _handle_open_tls(xmpp_conn_t * const conn); @@ -69,11 +70,6 @@ static int _handle_component_hs_response(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * const userdata); -static int _handle_missing_legacy(xmpp_conn_t * const conn, - void * const userdata); -static int _handle_legacy(xmpp_conn_t * const conn, - xmpp_stanza_t * const stanza, - void * const userdata); static int _handle_features_sasl(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * const userdata); @@ -557,9 +553,11 @@ static xmpp_stanza_t *_make_sasl_auth(xmpp_conn_t * const conn, */ static void _auth(xmpp_conn_t * const conn) { - xmpp_stanza_t *auth, *authdata, *query, *child, *iq; - char *str, *authid; + xmpp_stanza_t *auth; + xmpp_stanza_t *authdata; + char *authid; char *scram_init; + char *str; int anonjid; /* if there is no node in conn->jid, we assume anonymous connect */ @@ -726,105 +724,12 @@ static void _auth(xmpp_conn_t * const conn) /* SASL PLAIN was tried */ conn->sasl_support &= ~SASL_MASK_PLAIN; - } else if (conn->type == XMPP_CLIENT) { - /* legacy client authentication */ - - iq = xmpp_iq_new(conn->ctx, "set", "_xmpp_auth1"); - if (!iq) { - disconnect_mem_error(conn); - return; - } - - query = xmpp_stanza_new(conn->ctx); - if (!query) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - xmpp_stanza_set_name(query, "query"); - xmpp_stanza_set_ns(query, XMPP_NS_AUTH); - xmpp_stanza_add_child(iq, query); - xmpp_stanza_release(query); - - child = xmpp_stanza_new(conn->ctx); - if (!child) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - xmpp_stanza_set_name(child, "username"); - xmpp_stanza_add_child(query, child); - xmpp_stanza_release(child); - - authdata = xmpp_stanza_new(conn->ctx); - if (!authdata) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - str = xmpp_jid_node(conn->ctx, conn->jid); - xmpp_stanza_set_text(authdata, str); - xmpp_free(conn->ctx, str); - xmpp_stanza_add_child(child, authdata); - xmpp_stanza_release(authdata); - - child = xmpp_stanza_new(conn->ctx); - if (!child) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - xmpp_stanza_set_name(child, "password"); - xmpp_stanza_add_child(query, child); - xmpp_stanza_release(child); - - authdata = xmpp_stanza_new(conn->ctx); - if (!authdata) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - xmpp_stanza_set_text(authdata, conn->pass); - xmpp_stanza_add_child(child, authdata); - xmpp_stanza_release(authdata); - - child = xmpp_stanza_new(conn->ctx); - if (!child) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - xmpp_stanza_set_name(child, "resource"); - xmpp_stanza_add_child(query, child); - xmpp_stanza_release(child); - - authdata = xmpp_stanza_new(conn->ctx); - if (!authdata) { - xmpp_stanza_release(iq); - disconnect_mem_error(conn); - return; - } - str = xmpp_jid_resource(conn->ctx, conn->jid); - if (str) { - xmpp_stanza_set_text(authdata, str); - xmpp_free(conn->ctx, str); - } else { - xmpp_stanza_release(authdata); - xmpp_stanza_release(iq); - xmpp_error(conn->ctx, "auth", - "Cannot authenticate without resource"); - xmpp_disconnect(conn); - return; - } - xmpp_stanza_add_child(child, authdata); - xmpp_stanza_release(authdata); - - handler_add_id(conn, _handle_legacy, "_xmpp_auth1", NULL); - handler_add_timed(conn, _handle_missing_legacy, - LEGACY_TIMEOUT, NULL); - - xmpp_send(conn, iq); - xmpp_stanza_release(iq); + } else if (conn->type == XMPP_CLIENT && conn->auth_legacy_enabled) { + /* legacy client authentication */ + _auth_legacy(conn); + } else { + xmpp_error(conn->ctx, "auth", "Cannot authenticate with known methods"); + xmpp_disconnect(conn); } } @@ -1105,6 +1010,15 @@ static int _handle_missing_session(xmpp_conn_t * const conn, return 0; } +static int _handle_missing_legacy(xmpp_conn_t * const conn, + void * const userdata) +{ + xmpp_error(conn->ctx, "xmpp", "Server did not reply to legacy "\ + "authentication request."); + xmpp_disconnect(conn); + return 0; +} + static int _handle_legacy(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * const userdata) @@ -1141,13 +1055,97 @@ static int _handle_legacy(xmpp_conn_t * const conn, return 0; } -static int _handle_missing_legacy(xmpp_conn_t * const conn, - void * const userdata) +static void _auth_legacy(xmpp_conn_t *conn) { - xmpp_error(conn->ctx, "xmpp", "Server did not reply to legacy "\ - "authentication request."); - xmpp_disconnect(conn); - return 0; + xmpp_stanza_t *iq; + xmpp_stanza_t *authdata; + xmpp_stanza_t *query; + xmpp_stanza_t *child; + char *str; + + xmpp_debug(conn->ctx, "auth", "Legacy authentication request"); + + iq = xmpp_iq_new(conn->ctx, "set", "_xmpp_auth1"); + if (!iq) + goto err; + + query = xmpp_stanza_new(conn->ctx); + if (!query) + goto err_free; + xmpp_stanza_set_name(query, "query"); + xmpp_stanza_set_ns(query, XMPP_NS_AUTH); + xmpp_stanza_add_child(iq, query); + xmpp_stanza_release(query); + + child = xmpp_stanza_new(conn->ctx); + if (!child) + goto err_free; + xmpp_stanza_set_name(child, "username"); + xmpp_stanza_add_child(query, child); + xmpp_stanza_release(child); + + authdata = xmpp_stanza_new(conn->ctx); + if (!authdata) + goto err_free; + str = xmpp_jid_node(conn->ctx, conn->jid); + if (!str) { + xmpp_stanza_release(authdata); + goto err_free; + } + xmpp_stanza_set_text(authdata, str); + xmpp_free(conn->ctx, str); + xmpp_stanza_add_child(child, authdata); + xmpp_stanza_release(authdata); + + child = xmpp_stanza_new(conn->ctx); + if (!child) + goto err_free; + xmpp_stanza_set_name(child, "password"); + xmpp_stanza_add_child(query, child); + xmpp_stanza_release(child); + + authdata = xmpp_stanza_new(conn->ctx); + if (!authdata) + goto err_free; + xmpp_stanza_set_text(authdata, conn->pass); + xmpp_stanza_add_child(child, authdata); + xmpp_stanza_release(authdata); + + child = xmpp_stanza_new(conn->ctx); + if (!child) + goto err_free; + xmpp_stanza_set_name(child, "resource"); + xmpp_stanza_add_child(query, child); + xmpp_stanza_release(child); + + authdata = xmpp_stanza_new(conn->ctx); + if (!authdata) + goto err_free; + str = xmpp_jid_resource(conn->ctx, conn->jid); + if (str) { + xmpp_stanza_set_text(authdata, str); + xmpp_free(conn->ctx, str); + } else { + xmpp_stanza_release(authdata); + xmpp_stanza_release(iq); + xmpp_error(conn->ctx, "auth", "Cannot authenticate without resource"); + xmpp_disconnect(conn); + return; + } + xmpp_stanza_add_child(child, authdata); + xmpp_stanza_release(authdata); + + handler_add_id(conn, _handle_legacy, "_xmpp_auth1", NULL); + handler_add_timed(conn, _handle_missing_legacy, LEGACY_TIMEOUT, NULL); + + xmpp_send(conn, iq); + xmpp_stanza_release(iq); + return; + +err_free: + xmpp_stanza_release(iq); +err: + disconnect_mem_error(conn); } void auth_handle_component_open(xmpp_conn_t * const conn) diff --git a/src/common.h b/src/common.h index f1356b0..e53e4ff 100644 --- a/src/common.h +++ b/src/common.h @@ -172,6 +172,7 @@ struct _xmpp_conn_t { int tls_failed; /* set when tls fails, so we don't try again */ int sasl_support; /* if true, field is a bitfield of supported mechanisms */ + int auth_legacy_enabled; int secured; /* set when stream is secured with TLS */ /* if server returns or we must do them */ diff --git a/src/conn.c b/src/conn.c index 91cd9ab..316552c 100644 --- a/src/conn.c +++ b/src/conn.c @@ -140,6 +140,7 @@ xmpp_conn_t *xmpp_conn_new(xmpp_ctx_t * const ctx) conn->tls_trust = 0; conn->tls_failed = 0; conn->sasl_support = 0; + conn->auth_legacy_enabled = 0; conn->secured = 0; conn->bind_required = 0; @@ -921,7 +922,8 @@ long xmpp_conn_get_flags(const xmpp_conn_t * const conn) flags = XMPP_CONN_FLAG_DISABLE_TLS * conn->tls_disabled | XMPP_CONN_FLAG_MANDATORY_TLS * conn->tls_mandatory | XMPP_CONN_FLAG_LEGACY_SSL * conn->tls_legacy_ssl | - XMPP_CONN_FLAG_TRUST_TLS * conn->tls_trust; + XMPP_CONN_FLAG_TRUST_TLS * conn->tls_trust | + XMPP_CONN_FLAG_LEGACY_AUTH * conn->auth_legacy_enabled;; return flags; } @@ -939,6 +941,7 @@ long xmpp_conn_get_flags(const xmpp_conn_t * const conn) * - XMPP_CONN_FLAG_MANDATORY_TLS * - XMPP_CONN_FLAG_LEGACY_SSL * - XMPP_CONN_FLAG_TRUST_TLS + * - XMPP_CONN_FLAG_LEGACY_AUTH * * @param conn a Strophe connection object * @param flags ORed connection flags @@ -965,6 +968,7 @@ int xmpp_conn_set_flags(xmpp_conn_t * const conn, long flags) conn->tls_mandatory = (flags & XMPP_CONN_FLAG_MANDATORY_TLS) ? 1 : 0; conn->tls_legacy_ssl = (flags & XMPP_CONN_FLAG_LEGACY_SSL) ? 1 : 0; conn->tls_trust = (flags & XMPP_CONN_FLAG_TRUST_TLS) ? 1 : 0; + conn->auth_legacy_enabled = (flags & XMPP_CONN_FLAG_LEGACY_AUTH) ? 1 : 0; return 0; } diff --git a/strophe.h b/strophe.h index 5a18f1a..d9f048f 100644 --- a/strophe.h +++ b/strophe.h @@ -169,6 +169,10 @@ typedef struct _xmpp_stanza_t xmpp_stanza_t; * Trust server's certificate even if it is invalid. */ #define XMPP_CONN_FLAG_TRUST_TLS (1UL << 3) +/** @def XMPP_CONN_FLAG_LEGACY_AUTH + * Enable legacy authentication support. + */ +#define XMPP_CONN_FLAG_LEGACY_AUTH (1UL << 4) /* connect callback */ typedef enum {