Merge remote-tracking branch 'upstream/master' into security

src/core/ddsc/include/dds/dds.h

@@ -187,17 +187,21 @@ typedef struct dds_builtintopic_guid
 }
 dds_builtintopic_guid_t;
 
+/* "dds_builtintopic_guid_t" is a bit of a weird name for what everyone just calls a GUID,
+   so let us try and switch to using the more logical one */
+typedef struct dds_builtintopic_guid dds_guid_t;
+
 typedef struct dds_builtintopic_participant
 {
-  dds_builtintopic_guid_t key;
+  dds_guid_t key;
   dds_qos_t *qos;
 }
 dds_builtintopic_participant_t;
 
 typedef struct dds_builtintopic_endpoint
 {
-  dds_builtintopic_guid_t key;
-  dds_builtintopic_guid_t participant_key;
+  dds_guid_t key;
+  dds_guid_t participant_key;
   dds_instance_handle_t participant_instance_handle;
   char *topic_name;
   char *type_name;
@@ -415,6 +419,26 @@ dds_get_mask(dds_entity_t condition, uint32_t *mask);
 DDS_EXPORT dds_return_t
 dds_get_instance_handle(dds_entity_t entity, dds_instance_handle_t *ihdl);
 
+/**
+ * @brief Returns the GUID that represents the entity in the network,
+ * and therefore only supports participants, readers and writers.
+ *
+ * @param[in]   entity  Entity of which to get the instance handle.
+ * @param[out]  guid    Where to store the GUID.
+ *
+ * @returns A dds_return_t indicating success or failure.
+ *
+ * @retval DDS_RETCODE_OK
+ *             Success.
+ * @retval DDS_RETCODE_ILLEGAL_OPERATION
+ *             The operation is invoked on an inappropriate object.
+ * @retval DDS_RETCODE_ERROR
+ *             An internal error has occurred.
+ */
+/* TODO: Check list of return codes is complete. */
+DDS_EXPORT dds_return_t
+dds_get_guid (dds_entity_t entity, dds_guid_t *guid);
+
 /*
   All entities have a set of "status conditions" (following the DCPS
   spec), read peeks, take reads & resets (analogously to read & take

src/core/ddsc/src/dds_entity.c

@@ -25,6 +25,7 @@
 #include "dds/ddsi/ddsi_pmd.h"
 #include "dds/ddsi/ddsi_xqos.h"
 #include "dds/ddsi/q_transmit.h"
+#include "dds/ddsi/q_bswap.h"
 
 extern inline dds_entity *dds_entity_from_handle_link (struct dds_handle_link *hdllink);
 extern inline bool dds_entity_is_enabled (const dds_entity *e);
@@ -1285,6 +1286,36 @@ dds_return_t dds_get_instance_handle (dds_entity_t entity, dds_instance_handle_t
   return ret;
 }
 
+dds_return_t dds_get_guid (dds_entity_t entity, dds_guid_t *guid)
+{
+  dds_entity *e;
+  dds_return_t ret;
+
+  if (guid == NULL)
+    return DDS_RETCODE_BAD_PARAMETER;
+
+  if ((ret = dds_entity_pin (entity, &e)) != DDS_RETCODE_OK)
+    return ret;
+  switch (dds_entity_kind (e))
+  {
+    case DDS_KIND_PARTICIPANT:
+    case DDS_KIND_READER:
+    case DDS_KIND_WRITER: {
+      DDSRT_STATIC_ASSERT (sizeof (dds_guid_t) == sizeof (ddsi_guid_t));
+      ddsi_guid_t tmp = nn_ntoh_guid (e->m_guid);
+      memcpy (guid, &tmp, sizeof (*guid));
+      ret = DDS_RETCODE_OK;
+      break;
+    }
+    default: {
+      ret = DDS_RETCODE_ILLEGAL_OPERATION;
+      break;
+    }
+  }
+  dds_entity_unpin(e);
+  return ret;
+}
+
 dds_return_t dds_entity_pin (dds_entity_t hdl, dds_entity **eptr)
 {
   dds_return_t hres;

src/core/ddsc/src/dds_serdata_builtintopic.c

@@ -175,7 +175,7 @@ static struct ddsi_serdata *serdata_builtin_to_topicless (const struct ddsi_serd
   return ddsi_serdata_ref (serdata_common);
 }
 
-static void convkey (dds_builtintopic_guid_t *key, const ddsi_guid_t *guid)
+static void convkey (dds_guid_t *key, const ddsi_guid_t *guid)
 {
   ddsi_guid_t tmp;
   tmp = nn_hton_guid (*guid);

src/core/ddsc/tests/CMakeLists.txt

@@ -15,6 +15,7 @@ idlc_generate(RoundTrip RoundTrip.idl)
 idlc_generate(Space Space.idl)
 idlc_generate(TypesArrayKey TypesArrayKey.idl)
 idlc_generate(WriteTypes WriteTypes.idl)
+idlc_generate(InstanceHandleTypes InstanceHandleTypes.idl)
 
 set(ddsc_test_sources
     "basic.c"
@@ -28,6 +29,7 @@ set(ddsc_test_sources
     "entity_status.c"
     "err.c"
     "instance_get_key.c"
+    "instance_handle.c"
     "listener.c"
     "liveliness.c"
     "loan.c"
@@ -73,7 +75,7 @@ target_include_directories(
   "$<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/src/include/>"
   "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../ddsc/src>"
   "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../ddsi/include>")
-target_link_libraries(cunit_ddsc PRIVATE RoundTrip Space TypesArrayKey WriteTypes ddsc)
+target_link_libraries(cunit_ddsc PRIVATE RoundTrip Space TypesArrayKey WriteTypes InstanceHandleTypes ddsc)
 
 # Setup environment for config-tests
 get_test_property(CUnit_ddsc_config_simple_udp ENVIRONMENT CUnit_ddsc_config_simple_udp_env)

src/core/ddsc/tests/InstanceHandleTypes.idl

@@ -0,0 +1,23 @@
+/*
+ * Copyright(c) 2020 ADLINK Technology Limited and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License
+ * v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+module InstanceHandleTypes {
+  struct A {
+    unsigned long k; //@Key
+    unsigned long v;
+  };
+#pragma keylist A k
+  struct C {
+    octet k[4]; //@Key
+    unsigned long v;
+  };
+#pragma keylist C k
+};

src/core/ddsc/tests/entity_api.c

@@ -255,6 +255,28 @@ CU_Test(ddsc_entity, status, .init = create_entity, .fini = delete_entity)
     CU_ASSERT_EQUAL_FATAL(status1, DDS_RETCODE_OK);
 }
 
+CU_Test(ddsc_entity, guid, .init = create_entity, .fini = delete_entity)
+{
+    dds_return_t status;
+    dds_guid_t guid, zero;
+    memset(&zero, 0, sizeof(zero));
+
+    /* Don't check actual handle contents. That's a job
+     * for the specific entity children, not for the generic part. */
+
+    /* Check getting Handle with bad parameters. */
+    status = dds_get_guid (0, NULL);
+    CU_ASSERT_EQUAL_FATAL(status, DDS_RETCODE_BAD_PARAMETER);
+    status = dds_get_guid (entity, NULL);
+    CU_ASSERT_EQUAL_FATAL(status, DDS_RETCODE_BAD_PARAMETER);
+    status = dds_get_guid (0, &guid);
+    CU_ASSERT_EQUAL_FATAL(status, DDS_RETCODE_BAD_PARAMETER);
+
+    /* Get Instance Handle, which should not be 0 for a participant. */
+    status = dds_get_guid (entity, &guid);
+    CU_ASSERT_EQUAL_FATAL(status, DDS_RETCODE_OK);
+    CU_ASSERT_FATAL(memcmp(&guid, &zero, sizeof(guid)) != 0);
+}
 
 CU_Test(ddsc_entity, instance_handle, .init = create_entity, .fini = delete_entity)
 {

src/core/ddsc/tests/instance_handle.c

@@ -0,0 +1,134 @@
+/*
+ * Copyright(c) 2020 ADLINK Technology Limited and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License
+ * v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+#include <assert.h>
+#include <limits.h>
+
+#include "dds/dds.h"
+#include "dds/ddsrt/bswap.h"
+
+#include "test_common.h"
+#include "InstanceHandleTypes.h"
+
+static dds_entity_t dp, tp[3], rd[3], wr[3];
+
+static void instance_handle_init (void)
+{
+  char topicname[100];
+  dds_qos_t *qos;
+  dp = dds_create_participant (DDS_DOMAIN_DEFAULT, NULL, NULL);
+  CU_ASSERT_FATAL (dp > 0);
+
+  /* not strictly necessary to explicitly set KEEP_LAST (it is the default), nor to make
+     it reliable (it is only used inside a process without any limits that might cause it
+     to drop samples) */
+  qos = dds_create_qos ();
+  CU_ASSERT_FATAL (qos != NULL);
+  dds_qset_history (qos, DDS_HISTORY_KEEP_LAST, 1);
+  dds_qset_reliability (qos, DDS_RELIABILITY_RELIABLE, DDS_INFINITY);
+  create_unique_topic_name ("instance_handle", topicname, sizeof (topicname));
+  tp[0] = dds_create_topic (dp, &InstanceHandleTypes_A_desc, topicname, NULL, NULL);
+  CU_ASSERT_FATAL (tp[0] > 0);
+  create_unique_topic_name ("instance_handle", topicname, sizeof (topicname));
+  tp[1] = dds_create_topic (dp, &InstanceHandleTypes_A_desc, topicname, NULL, NULL);
+  CU_ASSERT_FATAL (tp[1] > 0);
+  create_unique_topic_name ("instance_handle", topicname, sizeof (topicname));
+  tp[2] = dds_create_topic (dp, &InstanceHandleTypes_C_desc, topicname, NULL, NULL);
+  CU_ASSERT_FATAL (tp[2] > 0);
+  dds_delete_qos (qos);
+  for (size_t i = 0; i < 3; i++)
+  {
+    rd[i] = dds_create_reader (dp, tp[i], NULL, NULL);
+    CU_ASSERT_FATAL (rd[i] > 0);
+    wr[i] = dds_create_writer (dp, tp[i], NULL, NULL);
+    CU_ASSERT_FATAL (wr[i] > 0);
+  }
+}
+
+static void instance_handle_fini (void)
+{
+  dds_return_t rc;
+  rc = dds_delete (dp);
+  CU_ASSERT_FATAL (rc == 0);
+}
+
+CU_Test (ddsc_instance_handle, a, .init = instance_handle_init, .fini = instance_handle_fini)
+{
+  /* By design, Cyclone maintains a global map of (topic class, X, key value) to instance handle,
+     where the "topic class" is the implementation of the topic (e.g., "default" or "builtin"),
+     "X" is dependent on the topic class but by design is a fixed constant for the default one,
+     and key value, again by design, is taken as the serialised representation of the key value.
+
+     The point behind this model is that it allows one to use an instance handle obtained on
+     one reader and use it to read the matching instance in another reader.  So that bit of
+     behaviour needs to be checked.
+
+     I'm not sure whether the "serialised" part should be included in the test, I don't think
+     that's something that should be guaranteed in the API.  However, it is worth verifying that
+     it doesn't go off and do something weird. */
+  InstanceHandleTypes_A a, b;
+  InstanceHandleTypes_C c;
+  dds_return_t rc;
+
+  for (uint32_t i = 1; i <= 5; i++)
+  {
+    a.k = i;
+    a.v = i;
+    b.k = i;
+    b.v = 2 * a.k;
+    const uint32_t a_k_be = ddsrt_toBE4u (a.k);
+    memcpy (c.k, &a_k_be, sizeof (c.k));
+    c.v = 3 * a.k;
+    rc = dds_write (wr[0], &a);
+    CU_ASSERT_FATAL (rc == 0);
+    rc = dds_write (wr[1], &b);
+    CU_ASSERT_FATAL (rc == 0);
+    rc = dds_write (wr[2], &c);
+    CU_ASSERT_FATAL (rc == 0);
+  }
+
+  for (uint32_t i = 1; i <= 5; i++)
+  {
+    dds_sample_info_t siA, siB, siC;
+    void *rawA = &a, *rawB = &b, *rawC = &c;
+
+    /* take one sample from A; no guarantee about the order in which the data is returned */
+    rc = dds_take (rd[0], &rawA, &siA, 1, 1);
+    CU_ASSERT_FATAL (rc == 1);
+    CU_ASSERT_FATAL (siA.valid_data);
+    CU_ASSERT_FATAL (1 <= a.k && a.k <= 5 && a.v == a.k);
+
+    /* take one sample from B using the instance handle just returned */
+    rc = dds_take_instance (rd[1], &rawB, &siB, 1, 1, siA.instance_handle);
+    CU_ASSERT_FATAL (rc == 1);
+    CU_ASSERT_FATAL (siB.valid_data);
+    CU_ASSERT_FATAL (siB.instance_handle == siA.instance_handle);
+    CU_ASSERT_FATAL (b.k == a.k && b.v == 2 * a.k);
+
+    /* take one sample from C using the instance handle just returned, this should work
+       even though C uses an array of octets as key */
+    rc = dds_take_instance (rd[2], &rawC, &siC, 1, 1, siA.instance_handle);
+    CU_ASSERT_FATAL (rc == 1);
+    CU_ASSERT_FATAL (siC.valid_data);
+    CU_ASSERT_FATAL (siC.instance_handle == siA.instance_handle);
+    const uint32_t a_k_be = ddsrt_toBE4u (a.k);
+    CU_ASSERT_FATAL (memcmp (c.k, &a_k_be, sizeof (c.k)) == 0 && c.v == 3 * a.k);
+  }
+
+  /* there should be no data left */
+  for (size_t i = 0; i < 3; i++)
+  {
+    dds_sample_info_t si;
+    void *raw = NULL;
+    rc = dds_take (rd[0], &raw, &si, 1, 1);
+    CU_ASSERT_FATAL (rc == 0);
+  }
+}