fix: support zero-length clientId (#331)

Add support for a zero-length clientId field
to match the MQTT v3.1.1 spec. Both
empty string and NULL client IDs are handled
the same when given a length of 0.

Author: Ryan-CW-Code

source/core_mqtt_serializer.c

@@ -1706,9 +1706,14 @@ MQTTStatus_t MQTT_GetConnectPacketSize( const MQTTConnectInfo_t * pConnectInfo,
                     ( void * ) pPacketSize ) );
         status = MQTTBadParameter;
     }
-    else if( ( pConnectInfo->clientIdentifierLength == 0U ) || ( pConnectInfo->pClientIdentifier == NULL ) )
+    else if( ( pConnectInfo->clientIdentifierLength == 0U ) ^ ( ( pConnectInfo->pClientIdentifier == NULL ) || ( *( pConnectInfo->pClientIdentifier ) == '\0' ) ) )
     {
-        LogError( ( "Mqtt_GetConnectPacketSize() client identifier must be set." ) );
+        LogError( ( "Client ID length and value mismatch." ) );
+        status = MQTTBadParameter;
+    }
+    else if( ( pConnectInfo->clientIdentifierLength == 0U ) && ( pConnectInfo->cleanSession == false ) )
+    {
+        LogError( ( "Zero-length client identifier requires cleanSession=true per MQTT 3.1.1." ) );
         status = MQTTBadParameter;
     }
     else if( ( pWillInfo != NULL ) && ( pWillInfo->payloadLength > ( size_t ) UINT16_MAX ) )

test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c

@@ -39,7 +39,12 @@ void harness()
     MQTTStatus_t status = MQTTSuccess;
 
     pConnectInfo = allocateMqttConnectInfo( NULL );
-    __CPROVER_assume( isValidMqttConnectInfo( pConnectInfo ) );
+
+    /* Avoid a malloc(0U) which causes dereference errors */
+    if( ( pConnectInfo != NULL ) && ( pConnectInfo->clientIdentifierLength == 0U ) )
+    {
+        pConnectInfo->pClientIdentifier = NULL;
+    }
 
     pWillInfo = allocateMqttPublishInfo( NULL );
     __CPROVER_assume( isValidMqttPublishInfo( pWillInfo ) );

test/unit-test/core_mqtt_serializer_utest.c

@@ -463,11 +463,59 @@ void test_MQTT_GetConnectPacketSize( void )
     status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
     TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status );
 
+    /* Verify NULL client identifier with provided non-zero length fails. */
     connectInfo.pClientIdentifier = NULL;
     connectInfo.clientIdentifierLength = CLIENT_IDENTIFIER_LENGTH;
+    connectInfo.cleanSession = true;
     status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
     TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status );
 
+    /* Verify empty string client ID with provided non-zero length fails. */
+    connectInfo.pClientIdentifier = "";
+    connectInfo.clientIdentifierLength = 99U;
+    connectInfo.cleanSession = true;
+    status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
+    TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status );
+
+    /* Verify zero-length client identifier must have clean session. */
+    connectInfo.pClientIdentifier = NULL;
+    connectInfo.clientIdentifierLength = 0U;
+    connectInfo.cleanSession = false;
+    status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
+    TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status );
+
+    /* Verify zero-length client identifier must have clean session. */
+    connectInfo.pClientIdentifier = "";
+    connectInfo.clientIdentifierLength = 0U;
+    connectInfo.cleanSession = false;
+    status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
+    TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status );
+
+    /* NULL client ID and client ID length of 0 is treated as zero-length identifier */
+    connectInfo.pClientIdentifier = NULL;
+    connectInfo.clientIdentifierLength = 0U;
+    connectInfo.cleanSession = true;
+    connectInfo.pUserName = NULL;
+    connectInfo.pPassword = NULL;
+    status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
+    TEST_ASSERT_EQUAL_INT( MQTTSuccess, status );
+    /* Make sure remaining size returned is 12. */
+    TEST_ASSERT_EQUAL_INT( 12, remainingLength );
+    /* Make sure packet size is 14. */
+    TEST_ASSERT_EQUAL_INT( 14, packetSize );
+
+    /* Empty string client ID and client ID length of 0 is treated as zero-length identifier */
+    connectInfo.pClientIdentifier = "";
+    connectInfo.clientIdentifierLength = 0U;
+    connectInfo.pUserName = NULL;
+    connectInfo.pPassword = NULL;
+    status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize );
+    TEST_ASSERT_EQUAL_INT( MQTTSuccess, status );
+    /* Make sure remaining size returned is 12. */
+    TEST_ASSERT_EQUAL_INT( 12, remainingLength );
+    /* Make sure packet size is 14. */
+    TEST_ASSERT_EQUAL_INT( 14, packetSize );
+
     /* Test a will message payload length that is too large. */
     memset( &connectInfo, 0x0, sizeof( connectInfo ) );
     connectInfo.pClientIdentifier = CLIENT_IDENTIFIER;