Use random int as registration ID

Previously the internal client ID was used as the registration ID. As
that ID is recycled it can cause problems due to collisions.
Moreover exposing the internal ID does make it public and therefore not
internal anymore.

Author: LukasWoodtli

core/registration.c

@@ -1884,6 +1884,7 @@ uint8_t  registration_handleRequest(lwm2m_context_t * contextP,
                 }
                 memset(clientP, 0, sizeof(lwm2m_client_t));
                 clientP->internalID = lwm2m_list_newId((lwm2m_list_t *)contextP->clientList);
+                clientP->registration_id = utils_random_registration_id(contextP->clientList);
                 contextP->clientList = (lwm2m_client_t *)LWM2M_LIST_ADD(contextP->clientList, clientP);
             }
             clientP->name = name;
@@ -1897,8 +1898,7 @@ uint8_t  registration_handleRequest(lwm2m_context_t * contextP,
             clientP->objectList = objects;
             clientP->sessionH = fromSessionH;
 
-            if (prv_getLocationString(clientP->internalID, location) == 0)
-            {
+            if (prv_getLocationString(clientP->registration_id, location) == 0) {
                 registration_freeClient(contextP, clientP);
                 return COAP_500_INTERNAL_SERVER_ERROR;
             }
@@ -1919,8 +1919,9 @@ uint8_t  registration_handleRequest(lwm2m_context_t * contextP,
             // Registration update
             if (LWM2M_URI_IS_SET_INSTANCE(uriP)) return COAP_400_BAD_REQUEST;
 
-            clientP = (lwm2m_client_t *)lwm2m_list_find((lwm2m_list_t *)contextP->clientList, uriP->objectId);
-            if (clientP == NULL) return COAP_404_NOT_FOUND;
+            clientP = utils_find_client_by_registration_id(contextP->clientList, uriP->objectId);
+            if (clientP == NULL)
+                return COAP_404_NOT_FOUND;
 
             // Endpoint client name MUST NOT be present
             if (name != NULL)
@@ -2018,7 +2019,7 @@ uint8_t  registration_handleRequest(lwm2m_context_t * contextP,
         if (!LWM2M_URI_IS_SET_OBJECT(uriP)) return COAP_400_BAD_REQUEST;
         if (LWM2M_URI_IS_SET_INSTANCE(uriP)) return COAP_400_BAD_REQUEST;
 
-        clientP = (lwm2m_client_t *)LWM2M_LIST_FIND(contextP->clientList, uriP->objectId);
+        clientP = utils_find_client_by_registration_id(contextP->clientList, uriP->objectId);
 
         if (clientP == NULL) {
             return COAP_400_BAD_REQUEST;

core/utils.c

@@ -903,6 +903,19 @@ lwm2m_client_t * utils_findClient(lwm2m_context_t * contextP,
 
     return targetP;
 }
+
+lwm2m_client_t *utils_find_client_by_registration_id(lwm2m_client_t *head, const uint16_t registration_id) {
+    while (NULL != head) {
+        if (head->registration_id == registration_id) {
+            return head;
+        }
+
+        head = head->next;
+    }
+
+    return NULL;
+}
+
 #endif
 
 int utils_isAltPathValid(const char * altPath)
@@ -1155,3 +1168,29 @@ lwm2m_data_type_t utils_depthToDatatype(uri_depth_t depth)
 
     return LWM2M_TYPE_UNDEFINED;
 }
+
+static inline bool reg_id_in_use(const lwm2m_client_t *const client_list, const uint16_t reg_id) {
+    const lwm2m_client_t *head = client_list;
+    bool reg_id_already_in_use = false;
+    while (head != NULL) {
+        if (head->registration_id == reg_id) {
+            reg_id_already_in_use = true;
+            break;
+        }
+
+        head = head->next;
+    }
+
+    return reg_id_already_in_use;
+}
+
+uint16_t utils_random_registration_id(const lwm2m_client_t *const client_list) {
+    uint16_t reg_id = (uint16_t)(rand() % LWM2M_MAX_ID); // NOSONAR
+    bool reg_id_already_in_use = reg_id_in_use(client_list, reg_id);
+
+    if (reg_id_already_in_use) {
+        return utils_random_registration_id(client_list);
+    }
+
+    return reg_id;
+}

core/utils.h

@@ -20,11 +20,13 @@
 #include "internals.h"
 
 lwm2m_data_type_t utils_depthToDatatype(uri_depth_t depth);
+uint16_t utils_random_registration_id(const lwm2m_client_t *client_list);
 lwm2m_version_t utils_stringToVersion(uint8_t *buffer, size_t length);
 lwm2m_binding_t utils_stringToBinding(uint8_t *buffer, size_t length);
 lwm2m_media_type_t utils_convertMediaType(coap_content_type_t type);
 uint8_t utils_getResponseFormat(uint8_t accept_num, const uint16_t *accept, int numData, const lwm2m_data_t *dataP,
                                 bool singleResource, lwm2m_media_type_t *format);
+lwm2m_client_t *utils_find_client_by_registration_id(lwm2m_client_t *head, uint16_t registration_id);
 int utils_isAltPathValid(const char *altPath);
 int utils_stringCopy(char *buffer, size_t length, const char *str);
 size_t utils_intToText(int64_t data, uint8_t *string, size_t length);

examples/server/lwm2mserver.c

@@ -147,6 +147,7 @@ static void prv_dump_client(lwm2m_client_t * targetP)
     lwm2m_client_object_t * objectP;
 
     fprintf(stdout, "Client #%d:\r\n", targetP->internalID);
+    fprintf(stdout, "\tregistration id: \"%d\"\r\n", targetP->registration_id);
     fprintf(stdout, "\tname: \"%s\"\r\n", targetP->name);
     fprintf(stdout, "\tversion: \"%s\"\r\n", prv_dump_version(targetP->version));
     prv_dump_binding(targetP->binding);

include/liblwm2m.h

@@ -722,6 +722,7 @@ typedef struct _lwm2m_client_
 {
     struct _lwm2m_client_ * next;       // matches lwm2m_list_t::next
     uint16_t                internalID; // matches lwm2m_list_t::id
+    uint16_t registration_id;
     char *                  name;
     lwm2m_version_t         version;
     lwm2m_binding_t         binding;

tests/core_registration_tests.c

@@ -87,7 +87,7 @@ static void test_registration_message_to_server(void) {
     size_t send_buffer_len;
     uint8_t *send_buffer = test_get_response_buffer(&send_buffer_len);
     coap_packet_t actual_response_packet;
-    CU_ASSERT_EQUAL(14, send_buffer_len);
+    CU_ASSERT(18 >= send_buffer_len && send_buffer_len >= 14);
     coap_status_t status = coap_parse_message(&actual_response_packet, send_buffer, send_buffer_len);
     CU_ASSERT_EQUAL(status, NO_ERROR);
 

tests/integration/test_registration_interface.py

@@ -20,12 +20,13 @@ def parse_client_registration(server_output):
     """
     client_id, = re.findall(r".lient #([0-9]+) ", server_output)
     event, = re.findall(r"(registered|updated)", server_output)
+    registration_id, = re.findall(r"registration id: \"([0-9]+)\"\r\r\n", server_output)
     endpoint, = re.findall(r"name: \"([\w-]+)\"\r\r\n", server_output)
     version, = re.findall(r"version: \"([0-9.]+)\"\r\r\n", server_output)
     binding, = re.findall(r"binding: \"([A-Z]+)\"\r\r\n", server_output)
     lifetime, = re.findall(r"lifetime: ([0-9]+) sec\r\r\n", server_output)
     objects = re.search(r"objects: ([\/0-9]+ ?(\([0-9.]+\))?, ?)+\r\r\n", server_output)
-    return int(client_id), event, endpoint, version, binding, int(lifetime), objects.string
+    return int(client_id), registration_id, event, endpoint, version, binding, int(lifetime), objects.string
 
 
 def test_registration_interface(lwm2mserver, lwm2mclient):
@@ -34,9 +35,10 @@ def test_registration_interface(lwm2mserver, lwm2mclient):
 
     lwm2mclient.wait_for_text("STATE_READY")
     text = lwm2mserver.wait_for_packet()
-    client_id, event, endpoint, version, binding, lifetime, objects = \
+    client_id, registration_id, event, endpoint, version, binding, lifetime, objects = \
         parse_client_registration(text)
     assert client_id == 0
+    assert int(registration_id) < 0xFFFE # UINT16_MAX - 1
     assert event == "registered"
     assert endpoint == "testlwm2mclient"
     assert version == "1.1"