Fix connected/connecting semantics

In case a connection is established, but the stream negotiation has not
completed yet, a user may think that they can start sending data,
but this is not the case.

All of the following quotes originate in RFC6120.

As of Ch. 8:

> After a client and a server (or two servers) have completed stream
> negotiation, either party can send XML stanzas.

As of Ch. 7.3.1:

> The parties to a stream MUST consider resource binding as
> mandatory-to-negotiate.

As of Ch. 6.3.1:

> The parties to a stream MUST consider SASL as mandatory-to-negotiate.

As of Ch. 4.3.5:

> The initiating entity MUST NOT attempt to send XML stanzas to entities
> other than itself (i.e., the client's connected resource or any other
> authenticated resource of the client's account) or the server to which
> it is connected until stream negotiation has been completed.
> Even if the initiating entity does attempt to do so, the receiving
> entity MUST NOT accept such stanzas and MUST close the stream with a
> <not-authorized/> stream error [...]

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
This commit is contained in:
Steffen Jaeckel
2023-11-01 00:54:08 +01:00
parent 69f42360cf
commit 7fbf507f58
4 changed files with 29 additions and 16 deletions

View File

@@ -816,7 +816,7 @@ static void _auth(xmpp_conn_t *conn)
static void _auth_success(xmpp_conn_t *conn)
{
tls_clear_password_cache(conn);
conn->authenticated = 1;
conn->stream_negotiation_completed = 1;
/* call connection handler */
conn->conn_handler(conn, XMPP_CONN_CONNECT, 0, NULL, conn->userdata);
}

View File

@@ -272,7 +272,7 @@ struct _xmpp_conn_t {
xmpp_open_handler open_handler;
/* user handlers only get called after authentication */
int authenticated;
int stream_negotiation_completed;
/* connection events handler */
xmpp_conn_handler conn_handler;

View File

@@ -78,6 +78,7 @@
#define KEEPALIVE_COUNT 3
#endif
static int _is_connected(xmpp_conn_t *conn, xmpp_send_queue_owner_t owner);
static int _disconnect_cleanup(xmpp_conn_t *conn, void *userdata);
static void _reset_sm_state_for_reconnect(xmpp_conn_t *conn);
static char *_conn_build_stream_tag(xmpp_conn_t *conn,
@@ -207,7 +208,7 @@ xmpp_conn_t *xmpp_conn_new(xmpp_ctx_t *ctx)
_handle_stream_stanza, conn);
conn->reset_parser = 0;
conn->authenticated = 0;
conn->stream_negotiation_completed = 0;
conn->conn_handler = NULL;
conn->userdata = NULL;
conn->timed_handlers = NULL;
@@ -864,9 +865,10 @@ void conn_established(xmpp_conn_t *conn)
if (conn->is_raw) {
handler_reset_timed(conn, 0);
/* we skip authentication for a "raw" connection, but the event loop
ignores user's handlers when conn->authenticated is not set. */
conn->authenticated = 1;
/* we skip all the mandatory steps of the stream negotiation for a "raw"
connection, but the event loop ignores user's handlers when
conn->stream_negotiation_completed is not set. */
conn->stream_negotiation_completed = 1;
conn->conn_handler(conn, XMPP_CONN_RAW_CONNECT, 0, NULL,
conn->userdata);
} else {
@@ -960,6 +962,7 @@ void conn_disconnect(xmpp_conn_t *conn)
{
strophe_debug(conn->ctx, "xmpp", "Closing socket.");
conn->state = XMPP_STATE_DISCONNECTED;
conn->stream_negotiation_completed = 0;
if (conn->tls) {
tls_stop(conn->tls);
tls_free(conn->tls);
@@ -1028,7 +1031,7 @@ void xmpp_send_raw_string(xmpp_conn_t *conn, const char *fmt, ...)
{
va_list ap;
if (conn->state != XMPP_STATE_CONNECTED)
if (!_is_connected(conn, XMPP_QUEUE_USER))
return;
va_start(ap, fmt);
@@ -1222,17 +1225,26 @@ int xmpp_conn_is_secured(xmpp_conn_t *conn)
*/
int xmpp_conn_is_connecting(xmpp_conn_t *conn)
{
return conn->state == XMPP_STATE_CONNECTING;
return conn->state == XMPP_STATE_CONNECTING ||
(conn->state == XMPP_STATE_CONNECTED &&
conn->stream_negotiation_completed == 0);
}
static int _is_connected(xmpp_conn_t *conn, xmpp_send_queue_owner_t owner)
{
return conn->state == XMPP_STATE_CONNECTED &&
(owner != XMPP_QUEUE_USER ||
conn->stream_negotiation_completed == 1);
}
/**
* @return TRUE if connection is in connected state and FALSE otherwise
* @return TRUE if connection is established and FALSE otherwise
*
* @ingroup Connections
*/
int xmpp_conn_is_connected(xmpp_conn_t *conn)
{
return conn->state == XMPP_STATE_CONNECTED;
return _is_connected(conn, XMPP_QUEUE_USER);
}
/**
@@ -1793,7 +1805,7 @@ static void _conn_reset(xmpp_conn_t *conn)
strophe_free_and_null(ctx, conn->domain);
strophe_free_and_null(ctx, conn->bound_jid);
strophe_free_and_null(ctx, conn->stream_id);
conn->authenticated = 0;
conn->stream_negotiation_completed = 0;
conn->secured = 0;
conn->tls_failed = 0;
conn->error = 0;
@@ -1880,7 +1892,7 @@ static void _send_valist(xmpp_conn_t *conn,
char buf[1024]; /* small buffer for common case */
char *bigbuf;
if (conn->state != XMPP_STATE_CONNECTED)
if (!_is_connected(conn, owner))
return;
va_copy(apdup, ap);
@@ -1928,7 +1940,7 @@ void send_stanza(xmpp_conn_t *conn,
char *buf = NULL;
size_t len;
if (conn->state != XMPP_STATE_CONNECTED)
if (!_is_connected(conn, owner))
goto out;
if (xmpp_stanza_to_text(stanza, &buf, &len) != 0) {

View File

@@ -83,7 +83,7 @@ void handler_fire_stanza(xmpp_conn_t *conn, xmpp_stanza_t *stanza)
while (item) {
/* don't fire user handlers until authentication succeeds and
and skip newly added handlers */
if ((item->user_handler && !conn->authenticated) ||
if ((item->user_handler && !conn->stream_negotiation_completed) ||
!item->enabled) {
item = item->next;
continue;
@@ -119,7 +119,8 @@ void handler_fire_stanza(xmpp_conn_t *conn, xmpp_stanza_t *stanza)
while (item) {
/* don't fire user handlers until authentication succeeds and
skip newly added handlers */
if ((item->user_handler && !conn->authenticated) || !item->enabled) {
if ((item->user_handler && !conn->stream_negotiation_completed) ||
!item->enabled) {
item = item->next;
continue;
}
@@ -177,7 +178,7 @@ uint64_t handler_fire_timed(xmpp_ctx_t *ctx)
while (item) {
/* don't fire user handlers until authentication succeeds and
skip newly added handlers */
if ((item->user_handler && !conn->authenticated) ||
if ((item->user_handler && !conn->stream_negotiation_completed) ||
!item->enabled) {
item = item->next;
continue;