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 <[email protected]>

Author: sjaeckel

src/auth.c

@@ -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);
 }

src/common.h

@@ -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;

src/conn.c

@@ -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) {

src/handler.c

@@ -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;