Auto-generate client ids in MQTT5 (#404)

Co-authored-by: Bret Ambrose <bambrose@amazon.com>

Author: bretambrose

source/v5/mqtt5_options_storage.c

@@ -8,9 +8,9 @@
 #include <aws/common/clock.h>
 #include <aws/common/encoding.h>
 #include <aws/common/string.h>
+#include <aws/common/uuid.h>
 #include <aws/io/channel_bootstrap.h>
 #include <aws/io/event_loop.h>
-#include <aws/io/stream.h>
 #include <aws/mqtt/private/v5/mqtt5_client_impl.h>
 #include <aws/mqtt/private/v5/mqtt5_utils.h>
 #include <aws/mqtt/v5/mqtt5_client.h>
@@ -3870,8 +3870,70 @@ struct aws_mqtt5_client_options_storage *aws_mqtt5_client_options_storage_new(
         options_storage->topic_aliasing_options = *options->topic_aliasing_options;
     }
 
+    struct aws_byte_buf auto_assign_id_buf;
+    AWS_ZERO_STRUCT(auto_assign_id_buf);
+
+    struct aws_mqtt5_packet_connect_view connect_options = *options->connect_options;
+    if (connect_options.client_id.len == 0) {
+        if (options->extended_validation_and_flow_control_options == AWS_MQTT5_EVAFCO_AWS_IOT_CORE_DEFAULTS) {
+            /*
+             * We're (probably) using an SDK builder to create the client and there's no client id set.  Assume this is
+             * targeting IoT Core.  Iot Core is not compliant to the MQTT311/5 spec with regard to auto-assigned client
+             * ids.
+             *
+             * In particular, IoT Core makes a client id of the pattern "$GEN/[uuid]" but it forbids a client id of that
+             * pattern to be specified by the user.  This is contrary to the spec that requires an auto-assigned client
+             * id to be able to reconnect using the same client id:
+             *
+             *   "It MUST then process the CONNECT packet as if the Client had provided that unique ClientID, and MUST
+             *     return the Assigned Client Identifier in the CONNACK packet."
+             *
+             * See (https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059) for details.
+             *
+             * The result is that using auto assigned client ids with IoT Core leads to permanent reconnect failures
+             * because we use the client id value returned in the CONNACK for all subsequent connection attempts.  This
+             * behavioral choice varies across MQTT clients, but I am firmly convinced it is proper.
+             *
+             * To work around this issue, when we think we're connecting to IoT Core (ie the condition above) and we
+             * don't have a client id, we generate one of the form "gen[uuid]" which will be able to successfully
+             * reconnect since it does not use the reserved prefix "$GEN/"
+             */
+            aws_byte_buf_init(&auto_assign_id_buf, allocator, 64);
+            struct aws_byte_cursor auto_assign_prefix_cursor = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("gen");
+
+            aws_byte_buf_append(&auto_assign_id_buf, &auto_assign_prefix_cursor);
+
+            struct aws_uuid uuid;
+            AWS_FATAL_ASSERT(aws_uuid_init(&uuid) == AWS_OP_SUCCESS);
+
+            aws_uuid_to_str_compact(&uuid, &auto_assign_id_buf);
+
+            connect_options.client_id = aws_byte_cursor_from_buf(&auto_assign_id_buf);
+
+            AWS_LOGF_INFO(
+                AWS_LS_MQTT5_GENERAL,
+                "Inferring the server is IoT Core and no client id has been set.  IoT Core's auto-assigned client ids "
+                "cannot be reconnected with.  Avoiding this issue by locally setting the client id to \"" PRInSTR "\"",
+                AWS_BYTE_CURSOR_PRI(connect_options.client_id));
+        } else {
+            /*
+             * Log the fact that we're not manually generating the client id in case someone turns off iot core
+             * validation and then sees reconnect failures.
+             */
+            AWS_LOGF_WARN(
+                AWS_LS_MQTT5_GENERAL,
+                "Letting server assign the client id.  If reconnects fail with a connect reason code of "
+                "AWS_MQTT5_CRC_CLIENT_IDENTIFIER_NOT_VALID (133), then consider manually setting the client id to a "
+                "UUID.");
+        }
+    }
+
     options_storage->connect = aws_mem_calloc(allocator, 1, sizeof(struct aws_mqtt5_packet_connect_storage));
-    if (aws_mqtt5_packet_connect_storage_init(options_storage->connect, allocator, options->connect_options)) {
+    int connect_storage_result =
+        aws_mqtt5_packet_connect_storage_init(options_storage->connect, allocator, &connect_options);
+
+    aws_byte_buf_clean_up(&auto_assign_id_buf);
+    if (connect_storage_result != AWS_OP_SUCCESS) {
         goto error;
     }
 

tests/CMakeLists.txt

@@ -323,6 +323,8 @@ add_test_case(mqtt5_first_byte_reserved_header_check_subscribe)
 add_test_case(mqtt5_first_byte_reserved_header_check_unsubscribe)
 add_test_case(mqtt5_first_byte_reserved_header_check_disconnect)
 
+add_test_case(mqtt5_client_auto_assigned_client_id)
+add_test_case(mqtt5_client_auto_assigned_client_id_iot_core)
 add_test_case(mqtt5_client_direct_connect_success)
 add_test_case(mqtt5_client_direct_connect_sync_channel_failure)
 add_test_case(mqtt5_client_direct_connect_async_channel_failure)

tests/v5/mqtt5_client_tests.c

@@ -18,6 +18,7 @@
 #include <aws/testing/aws_test_harness.h>
 
 #include <math.h>
+
 /* The delay margin accounts for system-level latencies, particularly when scheduling tasks on dispatch queues. In
  * typical test environments, we observe delays of around 150–170ms before the task begins execution. To ensure
  * stability, we define a margin of 200ms (AWS_MQTT5_TESTING_DELAY_NS). If related tests fail, it may indicate this
@@ -6600,4 +6601,54 @@ static int s_mqtt5_client_dynamic_operation_timeout_default_fn(struct aws_alloca
     return AWS_OP_SUCCESS;
 }
 
-AWS_TEST_CASE(mqtt5_client_dynamic_operation_timeout_default, s_mqtt5_client_dynamic_operation_timeout_default_fn)
+AWS_TEST_CASE(mqtt5_client_dynamic_operation_timeout_default, s_mqtt5_client_dynamic_operation_timeout_default_fn)
+
+static int s_mqtt5_do_auto_assigned_client_id_test(struct aws_allocator *allocator, bool use_iot_core_validation) {
+    aws_mqtt_library_init(allocator);
+
+    struct mqtt5_client_test_options test_options;
+    aws_mqtt5_client_test_init_default_options(&test_options);
+    AWS_ZERO_STRUCT(((struct aws_mqtt5_packet_connect_view *)test_options.client_options.connect_options)->client_id);
+    if (use_iot_core_validation) {
+        test_options.client_options.extended_validation_and_flow_control_options =
+            AWS_MQTT5_EVAFCO_AWS_IOT_CORE_DEFAULTS;
+    }
+
+    struct aws_mqtt5_client_mqtt5_mock_test_fixture_options test_fixture_options = {
+        .client_options = &test_options.client_options,
+        .server_function_table = &test_options.server_function_table,
+    };
+
+    struct aws_mqtt5_client_mock_test_fixture test_context;
+    ASSERT_SUCCESS(aws_mqtt5_client_mock_test_fixture_init(&test_context, allocator, &test_fixture_options));
+
+    struct aws_mqtt5_client *client = test_context.client;
+
+    const struct aws_byte_cursor auto_assigned_client_id = client->config->connect->storage_view.client_id;
+    if (use_iot_core_validation) {
+        ASSERT_INT_EQUALS(35, auto_assigned_client_id.len); /* "gen" + uuid as string no dashes length */
+    } else {
+        ASSERT_INT_EQUALS(0, auto_assigned_client_id.len);
+    }
+
+    aws_mqtt5_client_mock_test_fixture_clean_up(&test_context);
+    aws_mqtt_library_clean_up();
+
+    return AWS_OP_SUCCESS;
+}
+
+static int s_mqtt5_client_auto_assigned_client_id_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    return s_mqtt5_do_auto_assigned_client_id_test(allocator, false);
+}
+
+AWS_TEST_CASE(mqtt5_client_auto_assigned_client_id, s_mqtt5_client_auto_assigned_client_id_fn)
+
+static int s_mqtt5_client_auto_assigned_client_id_iot_core_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    return s_mqtt5_do_auto_assigned_client_id_test(allocator, true);
+}
+
+AWS_TEST_CASE(mqtt5_client_auto_assigned_client_id_iot_core, s_mqtt5_client_auto_assigned_client_id_iot_core_fn)