Do not silently ignore security QoS settings

When built without support for DDS Security, any attempt to create a
participant QoS settings in the security name space (those prefixed by
"dds.sec.") must fail.

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsc/tests/config.c

@@ -141,3 +141,43 @@ CU_Test(ddsc_security_config, empty, .init = ddsrt_init, .fini = ddsrt_fini)
   CU_ASSERT_FATAL(found == 0x7);
 #endif
 }
+
+CU_Test(ddsc_security_qos, empty, .init = ddsrt_init, .fini = ddsrt_fini)
+{
+  /* Expected traces when creating participant with some (not all) security QoS
+     settings.  We need to test this one here to be sure that it also refuses to
+     start when security is configured but the implementation doesn't include
+     support for it. */
+  const char *log_expected[] = {
+#ifdef DDSI_INCLUDE_SECURITY
+    "new_participant(*): using security settings from QoS*",
+    "new_participant(*): required security property * missing*",
+#endif
+    NULL
+  };
+
+  /* Set up the trace sinks to detect the config parsing. */
+  dds_set_log_mask (DDS_LC_FATAL|DDS_LC_ERROR|DDS_LC_WARNING|DDS_LC_CONFIG);
+  dds_set_log_sink (&logger, (void *) log_expected);
+  dds_set_trace_sink (&logger, (void *) log_expected);
+
+  /* Create participant with incomplete/nonsensical security configuration: this should always fail */
+  found = 0;
+  dds_qos_t *qos = dds_create_qos ();
+  dds_qset_prop (qos, "dds.sec.nonsense", "");
+  dds_entity_t domain = dds_create_domain (0, "<Tracing><Category>trace</Category>");
+  CU_ASSERT_FATAL (domain > 0);
+  dds_entity_t participant = dds_create_participant (0, qos, NULL);
+  dds_delete_qos (qos);
+  CU_ASSERT_FATAL (participant < 0);
+  (void) dds_delete (domain);
+  dds_set_log_sink (NULL, NULL);
+  dds_set_trace_sink (NULL, NULL);
+
+  /* All traces should have been provided. */
+#ifndef DDSI_INCLUDE_SECURITY
+  CU_ASSERT_FATAL (found == 0x0);
+#else
+  CU_ASSERT_FATAL (found == 0x3);
+#endif
+}

src/core/ddsi/include/dds/ddsi/ddsi_security_omg.h

@@ -184,11 +184,12 @@ bool q_omg_proxy_participant_is_secure(const struct proxy_participant *proxypp);
  * @param[in] pp         The participant to check if alloweed by security.
  * #param[in] domain_id  The domain_id
  *
- * @returns bool
+ * @returns dds_return_t
- * @retval true   Participant is allowed
+ * @retval DDS_RETCODE_OK   Participant is allowed
- * @retval false  Participant is not allowed
+ * @retval DDS_RETCODE_NOT_ALLOWED_BY_SECURITY
+ *                          Participant is not allowed
  */
-bool q_omg_security_check_create_participant(struct participant *pp, uint32_t domain_id);
+dds_return_t q_omg_security_check_create_participant(struct participant *pp, uint32_t domain_id);
 
 void q_omg_security_participant_set_initialized(struct participant *pp);
 

src/core/ddsi/include/dds/ddsi/ddsi_xqos.h

@@ -322,7 +322,9 @@ DDS_EXPORT void ddsi_xqos_addtomsg (struct nn_xmsg *m, const dds_qos_t *xqos, ui
 DDS_EXPORT void ddsi_xqos_log (uint32_t cat, const struct ddsrt_log_cfg *logcfg, const dds_qos_t *xqos);
 DDS_EXPORT size_t ddsi_xqos_print (char * __restrict buf, size_t bufsize, const dds_qos_t *xqos);
 DDS_EXPORT dds_qos_t *ddsi_xqos_dup (const dds_qos_t *src);
-DDS_EXPORT bool ddsi_xqos_has_prop (const dds_qos_t *xqos, const char *pname, bool startswith, bool check_non_empty);
+DDS_EXPORT bool ddsi_xqos_has_prop_prefix (const dds_qos_t *xqos, const char *nameprefix);
+DDS_EXPORT bool ddsi_xqos_find_prop (const dds_qos_t *xqos, const char *name, const char **value);
+
 #ifdef DDSI_INCLUDE_SECURITY
 struct omg_security_configuration_type;
 DDS_EXPORT void ddsi_xqos_mergein_security_config (dds_qos_t *xqos, const struct omg_security_configuration_type *cfg);

src/core/ddsi/src/ddsi_plist.c

@@ -3179,16 +3179,30 @@ dds_qos_t * ddsi_xqos_dup (const dds_qos_t *src)
   return dst;
 }
 
-bool ddsi_xqos_has_prop (const dds_qos_t *xqos, const char *pname, bool startswith, bool check_non_empty)
+bool ddsi_xqos_has_prop_prefix (const dds_qos_t *xqos, const char *nameprefix)
 {
   if (!(xqos->present & QP_PROPERTY_LIST))
     return false;
+  const size_t len = strlen (nameprefix);
+  for (uint32_t i = 0; i < xqos->property.value.n; i++)
+  {
+    if (strncmp (xqos->property.value.props[i].name, nameprefix, len) == 0)
+      return true;
+  }
+  return false;
+}
 
-  for (uint32_t i = 0; i < xqos->property.value.n; i++) {
+bool ddsi_xqos_find_prop (const dds_qos_t *xqos, const char *name, const char **value)
-    if (startswith && (strncmp(xqos->property.value.props[i].name, pname, strlen(pname)) == 0)) {
+{
-      return !check_non_empty || strlen(xqos->property.value.props[i].value) != 0;
+  if (!(xqos->present & QP_PROPERTY_LIST))
-    } else if (!startswith && (strcmp(xqos->property.value.props[i].name, pname) == 0)) {
+    return false;
-      return !check_non_empty || strlen(xqos->property.value.props[i].value) != 0;
+  for (uint32_t i = 0; i < xqos->property.value.n; i++)
+  {
+    if (strcmp (xqos->property.value.props[i].name, name) == 0)
+    {
+      if (value)
+        *value = xqos->property.value.props[i].value;
+      return true;
     }
   }
   return false;

src/core/ddsi/src/ddsi_security_omg.c

@@ -844,9 +844,9 @@ bool q_omg_participant_allow_unauthenticated(struct participant *pp)
   return ((pp->sec_attr != NULL) && pp->sec_attr->attr.allow_unauthenticated_participants);
 }
 
-bool q_omg_security_check_create_participant(struct participant *pp, uint32_t domain_id)
+dds_return_t q_omg_security_check_create_participant(struct participant *pp, uint32_t domain_id)
 {
-  bool allowed = false;
+  dds_return_t ret = DDS_RETCODE_NOT_ALLOWED_BY_SECURITY;
   struct dds_security_context *sc = q_omg_security_get_secure_context(pp);
   DDS_Security_IdentityHandle identity_handle = DDS_SECURITY_HANDLE_NIL;
   DDS_Security_SecurityException exception = DDS_SECURITY_EXCEPTION_INIT;
@@ -860,7 +860,7 @@ bool q_omg_security_check_create_participant(struct participant *pp, uint32_t do
   ddsi_guid_t adjusted_guid;
 
   if (!sc)
-    return true;
+    return DDS_RETCODE_OK;
 
   /* Validate local identity */
   ETRACE (pp, "validate_local_identity: candidate_guid: "PGUIDFMT" ", PGUID (pp->e.guid));
@@ -953,7 +953,7 @@ bool q_omg_security_check_create_participant(struct participant *pp, uint32_t do
 
   ETRACE (pp, "\n");
 
-  allowed = true;
+  ret = DDS_RETCODE_OK;
 
 no_crypto:
 no_sec_attr:
@@ -963,11 +963,11 @@ no_credentials:
   if (credential_token.class_id)
     (void)sc->access_control_context->return_permissions_credential_token(sc->access_control_context, &credential_token, NULL);
 not_allowed:
-  if (!allowed)
+  if (ret != DDS_RETCODE_OK)
     participant_sec_attributes_free(sec_attr);
 validation_failed:
   q_omg_shallow_free_security_qos(&par_qos);
-  return allowed;
+  return ret;
 }
 
 void q_omg_security_participant_set_initialized(struct participant *pp)

src/core/ddsi/src/q_entity.c

@@ -204,7 +204,6 @@ int is_builtin_endpoint (ddsi_entityid_t id, nn_vendorid_t vendorid)
 }
 
 #ifdef DDSI_INCLUDE_SECURITY
-
 static int is_builtin_volatile_endpoint (ddsi_entityid_t id)
 {
   switch (id.u) {
@@ -217,13 +216,13 @@ static int is_builtin_volatile_endpoint (ddsi_entityid_t id)
   return 0;
 }
 #else
-
+#ifndef NDEBUG
 static int is_builtin_volatile_endpoint (ddsi_entityid_t id)
 {
   DDSRT_UNUSED_ARG(id);
   return 0;
 }
-
+#endif
 #endif
 
 bool is_local_orphan_endpoint (const struct entity_common *e)
@@ -633,9 +632,63 @@ static void add_security_builtin_endpoints(struct participant *pp, ddsi_guid_t *
 }
 #endif
 
+static void add_builtin_endpoints(struct participant *pp, ddsi_guid_t *subguid, const ddsi_guid_t *group_guid, struct ddsi_domaingv *gv, bool add_writers, bool add_readers)
+{
+  if (add_writers)
+  {
+    struct whc_writer_info *wrinfo = whc_make_wrinfo (NULL, &gv->builtin_endpoint_xqos_wr);
+
+    /* SEDP writers: */
+    subguid->entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_WRITER);
+    new_writer_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
+    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_ANNOUNCER;
+
+    subguid->entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_WRITER);
+    new_writer_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
+    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PUBLICATION_ANNOUNCER;
+
+    /* PMD writer: */
+    subguid->entityid = to_entityid (NN_ENTITYID_P2P_BUILTIN_PARTICIPANT_MESSAGE_WRITER);
+    new_writer_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
+    pp->bes |= NN_BUILTIN_ENDPOINT_PARTICIPANT_MESSAGE_DATA_WRITER;
+
+    if (gv->config.do_topic_discovery)
+    {
+      subguid->entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_TOPIC_WRITER);
+      new_writer_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
+      pp->bes |= NN_DISC_BUILTIN_ENDPOINT_TOPIC_ANNOUNCER;
+    }
+
+    whc_free_wrinfo (wrinfo);
+  }
+
+  /* SPDP, SEDP, PMD readers: */
+  if (add_readers)
+  {
+    subguid->entityid = to_entityid (NN_ENTITYID_SPDP_BUILTIN_PARTICIPANT_READER);
+    new_reader_guid (NULL, subguid, group_guid, pp, NULL, &gv->spdp_endpoint_xqos, NULL, NULL, NULL);
+    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PARTICIPANT_DETECTOR;
+
+    subguid->entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_READER);
+    new_reader_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
+    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_DETECTOR;
+
+    subguid->entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_READER);
+    new_reader_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
+    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PUBLICATION_DETECTOR;
+
+    subguid->entityid = to_entityid (NN_ENTITYID_P2P_BUILTIN_PARTICIPANT_MESSAGE_READER);
+    new_reader_guid (NULL, subguid, group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
+    pp->bes |= NN_BUILTIN_ENDPOINT_PARTICIPANT_MESSAGE_DATA_READER;
+  }
 
 #ifdef DDSI_INCLUDE_SECURITY
+  if (q_omg_participant_is_secure (pp))
+    add_security_builtin_endpoints (pp, subguid, group_guid, gv, add_writers, add_readers);
+#endif
+}
 
+#ifdef DDSI_INCLUDE_SECURITY
 static void connect_participant_secure(struct ddsi_domaingv *gv, struct participant *pp)
 {
   struct proxy_participant *proxypp;
@@ -749,6 +802,81 @@ static void participant_remove_wr_lease_locked (struct participant * pp, struct
   }
 }
 
+#ifdef DDSI_INCLUDE_SECURITY
+static dds_return_t check_and_load_security_config (struct ddsi_domaingv * const gv, const ddsi_guid_t *ppguid, dds_qos_t *qos)
+{
+  /* If some security properties (name starts with dds.sec. conform DDS Security spec 7.2.4.1)
+     are present in the QoS, all must be and they will be used.  If none are, take the settings
+     from the configuration if it has them.  When no security configuration exists anywhere,
+     create an unsecured participant.
+
+     This may modify "qos" */
+  if (ddsi_xqos_has_prop_prefix (qos, "dds.sec."))
+  {
+    char const * const req[] = {
+      DDS_SEC_PROP_AUTH_IDENTITY_CA,
+      DDS_SEC_PROP_AUTH_PRIV_KEY,
+      DDS_SEC_PROP_AUTH_IDENTITY_CERT,
+      DDS_SEC_PROP_ACCESS_PERMISSIONS_CA,
+      DDS_SEC_PROP_ACCESS_GOVERNANCE,
+      DDS_SEC_PROP_ACCESS_PERMISSIONS,
+
+      DDS_SEC_PROP_AUTH_LIBRARY_PATH,
+      DDS_SEC_PROP_AUTH_LIBRARY_INIT,
+      DDS_SEC_PROP_AUTH_LIBRARY_FINALIZE,
+      DDS_SEC_PROP_CRYPTO_LIBRARY_PATH,
+      DDS_SEC_PROP_CRYPTO_LIBRARY_INIT,
+      DDS_SEC_PROP_CRYPTO_LIBRARY_FINALIZE,
+      DDS_SEC_PROP_ACCESS_LIBRARY_PATH,
+      DDS_SEC_PROP_ACCESS_LIBRARY_INIT,
+      DDS_SEC_PROP_ACCESS_LIBRARY_FINALIZE
+    };
+    GVLOGDISC ("new_participant("PGUIDFMT"): using security settings from QoS\n", PGUID (*ppguid));
+
+    /* check if all required security properties exist in qos; report all missing ones, not just the first */
+    dds_return_t ret = DDS_RETCODE_OK;
+    for (size_t i = 0; i < sizeof(req) / sizeof(req[0]); i++)
+    {
+      const char *value;
+      if (!ddsi_xqos_find_prop (qos, req[i], &value) || strlen (value) == 0)
+      {
+        GVERROR ("new_participant("PGUIDFMT"): required security property %s missing in Property QoS\n", PGUID (*ppguid), req[i]);
+        ret = DDS_RETCODE_PRECONDITION_NOT_MET;
+      }
+    }
+    if (ret != DDS_RETCODE_OK)
+      return ret;
+  }
+  else if (gv->config.omg_security_configuration)
+  {
+    /* For security, configuration can be provided through the configuration.  However, the specification
+       (and the plugins) expect it to be in the QoS, so merge it in. */
+    GVLOGDISC ("new_participant("PGUIDFMT"): using security settings from configuration\n", PGUID (*ppguid));
+    ddsi_xqos_mergein_security_config (qos, &gv->config.omg_security_configuration->cfg);
+  }
+  else
+  {
+    /* No security configuration */
+    return DDS_RETCODE_OK;
+  }
+
+  if (q_omg_is_security_loaded (gv->security_context))
+  {
+    GVLOGDISC ("new_participant("PGUIDFMT"): security is already loaded for this domain\n", PGUID (*ppguid));
+    return DDS_RETCODE_OK;
+  }
+  else if (q_omg_security_load (gv->security_context, qos) < 0)
+  {
+    GVERROR ("Could not load security\n");
+    return DDS_RETCODE_NOT_ALLOWED_BY_SECURITY;
+  }
+  else
+  {
+    return DDS_RETCODE_OK;
+  }
+}
+#endif
+
 dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv, unsigned flags, const ddsi_plist_t *plist)
 {
   struct participant *pp;
@@ -828,77 +956,18 @@ dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv
 
 #ifdef DDSI_INCLUDE_SECURITY
   pp->sec_attr = NULL;
-  /*
+  if ((ret = check_and_load_security_config (gv, ppguid, &pp->plist->qos)) != DDS_RETCODE_OK)
-   * if there there are security properties check them .
+    goto not_allowed;
-   * if there are no security properties, then merge from security configuration if there is
+  if ((ret = q_omg_security_check_create_participant (pp, gv->config.domainId)) != DDS_RETCODE_OK)
-   */
+    goto not_allowed;
-  /* check for existing security properties (name starts with dds.sec. conform DDS Security spec 7.2.4.1)
+  *ppguid = pp->e.guid;
-   * and return if any is found */
+#else
+  if (ddsi_xqos_has_prop_prefix (&pp->plist->qos, "dds.sec."))
   {
-    bool ready_to_load_security = false;
+    /* disallow creating a participant with a security configuration if there is support for security
-    if (ddsi_xqos_has_prop(&pp->plist->qos, "dds.sec.", true, false)) {
+       has been left out */
-      char *req[] = {DDS_SEC_PROP_AUTH_IDENTITY_CA,
+    ret = DDS_RETCODE_NOT_ALLOWED_BY_SECURITY;
-                     DDS_SEC_PROP_AUTH_PRIV_KEY,
+    goto not_allowed;
-                     DDS_SEC_PROP_AUTH_IDENTITY_CERT,
-                     DDS_SEC_PROP_ACCESS_PERMISSIONS_CA,
-                     DDS_SEC_PROP_ACCESS_GOVERNANCE,
-                     DDS_SEC_PROP_ACCESS_PERMISSIONS,
-
-                     DDS_SEC_PROP_AUTH_LIBRARY_PATH,
-                     DDS_SEC_PROP_AUTH_LIBRARY_INIT,
-                     DDS_SEC_PROP_AUTH_LIBRARY_FINALIZE,
-                     DDS_SEC_PROP_CRYPTO_LIBRARY_PATH,
-                     DDS_SEC_PROP_CRYPTO_LIBRARY_INIT,
-                     DDS_SEC_PROP_CRYPTO_LIBRARY_FINALIZE,
-                     DDS_SEC_PROP_ACCESS_LIBRARY_PATH,
-                     DDS_SEC_PROP_ACCESS_LIBRARY_INIT,
-                     DDS_SEC_PROP_ACCESS_LIBRARY_FINALIZE};
-      GVLOGDISC ("new_participant("
-                         PGUIDFMT
-                         "): using security settings from QoS\n", PGUID(*ppguid));
-
-      /* check if all required security properties exist in qos */
-      for (size_t i = 0; i < sizeof(req) / sizeof(req[0]); i++) {
-        if (!ddsi_xqos_has_prop(&pp->plist->qos, req[i], false, true)) {
-          GVERROR ("new_participant("
-                           PGUIDFMT
-                           "): required security property %s missing in Property QoS\n", PGUID(*ppguid), req[i]);
-          ret = DDS_RETCODE_PRECONDITION_NOT_MET;
-        }
-      }
-      if (ret == DDS_RETCODE_OK) {
-        ready_to_load_security = true;
-      } else {
-        goto new_pp_err_secprop;
-      }
-    } else if (gv->config.omg_security_configuration) {
-      /* For security, configuration can be provided through the configuration.
-       * However, the specification (and the plugins) expect it to be in the QoS. */
-      GVLOGDISC ("new_participant("
-                         PGUIDFMT
-                         "): using security settings from configuration\n", PGUID(*ppguid));
-      ddsi_xqos_mergein_security_config(&pp->plist->qos, &gv->config.omg_security_configuration->cfg);
-      ready_to_load_security = true;
-    }
-
-    if( q_omg_is_security_loaded( gv->security_context ) == false ){
-      if (ready_to_load_security && q_omg_security_load(gv->security_context, &pp->plist->qos) < 0) {
-        GVERROR("Could not load security\n");
-        ret = DDS_RETCODE_NOT_ALLOWED_BY_SECURITY;
-        goto new_pp_err_secprop;
-      }
-    } else {
-      GVLOGDISC ("new_participant("
-                               PGUIDFMT
-                               "): security is already loaded for this domain\n", PGUID(*ppguid));
-    }
-
-    if (!q_omg_security_check_create_participant (pp, gv->config.domainId))
-    {
-      ret = DDS_RETCODE_NOT_ALLOWED_BY_SECURITY;
-      goto not_allowed;
-    }
-    *ppguid = pp->e.guid;
   }
 #endif
 
@@ -927,9 +996,7 @@ dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv
   subguid.prefix = pp->e.guid.prefix;
   memset (&group_guid, 0, sizeof (group_guid));
 
-  /* SPDP writer */
+  /* SPDP is very much special and must be done first */
-  /* Note: skip SEDP <=> skip SPDP because of the way ddsi_discovery.c does things
-     currently.  */
   if (!(flags & RTPS_PF_NO_BUILTIN_WRITERS))
   {
     subguid.entityid = to_entityid (NN_ENTITYID_SPDP_BUILTIN_PARTICIPANT_WRITER);
@@ -938,70 +1005,15 @@ dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv
     whc_free_wrinfo (wrinfo);
     /* But we need the as_disc address set for SPDP, because we need to
        send it to everyone regardless of the existence of readers. */
-    force_as_disc_address(gv, &subguid);
+    force_as_disc_address (gv, &subguid);
     pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PARTICIPANT_ANNOUNCER;
   }
 
   /* Make it globally visible, else the endpoint matching won't work. */
   entidx_insert_participant_guid (gv->entity_index, pp);
 
-  /* SEDP writers: */
+  /* add all built-in endpoints other than the SPDP writer */
-  wrinfo = whc_make_wrinfo (NULL, &gv->builtin_endpoint_xqos_wr);
+  add_builtin_endpoints (pp, &subguid, &group_guid, gv, !(flags & RTPS_PF_NO_BUILTIN_WRITERS), !(flags & RTPS_PF_NO_BUILTIN_READERS));
-  if (!(flags & RTPS_PF_NO_BUILTIN_WRITERS))
-  {
-    subguid.entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_WRITER);
-    new_writer_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_ANNOUNCER;
-
-    subguid.entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_WRITER);
-    new_writer_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PUBLICATION_ANNOUNCER;
-  }
-
-  if (gv->config.do_topic_discovery)
-  {
-    /* TODO: make this one configurable, we don't want all participants to publish all topics (or even just those that they use themselves) */
-    subguid.entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_TOPIC_WRITER);
-    new_writer_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_TOPIC_ANNOUNCER;
-  }
-
-  /* PMD writer: */
-  if (!(flags & RTPS_PF_NO_BUILTIN_WRITERS))
-  {
-    subguid.entityid = to_entityid (NN_ENTITYID_P2P_BUILTIN_PARTICIPANT_MESSAGE_WRITER);
-    new_writer_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_wr, whc_new(gv, wrinfo), NULL, NULL);
-    pp->bes |= NN_BUILTIN_ENDPOINT_PARTICIPANT_MESSAGE_DATA_WRITER;
-  }
-
-  whc_free_wrinfo (wrinfo);
-
-  /* SPDP, SEDP, PMD readers: */
-  if (!(flags & RTPS_PF_NO_BUILTIN_READERS))
-  {
-    subguid.entityid = to_entityid (NN_ENTITYID_SPDP_BUILTIN_PARTICIPANT_READER);
-    new_reader_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->spdp_endpoint_xqos, NULL, NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PARTICIPANT_DETECTOR;
-
-    subguid.entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_READER);
-    new_reader_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_DETECTOR;
-
-    subguid.entityid = to_entityid (NN_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_READER);
-    new_reader_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
-    pp->bes |= NN_DISC_BUILTIN_ENDPOINT_PUBLICATION_DETECTOR;
-
-    subguid.entityid = to_entityid (NN_ENTITYID_P2P_BUILTIN_PARTICIPANT_MESSAGE_READER);
-    new_reader_guid (NULL, &subguid, &group_guid, pp, NULL, &gv->builtin_endpoint_xqos_rd, NULL, NULL, NULL);
-    pp->bes |= NN_BUILTIN_ENDPOINT_PARTICIPANT_MESSAGE_DATA_READER;
-  }
-
-#ifdef DDSI_INCLUDE_SECURITY
-  if (q_omg_participant_is_secure(pp))
-  {
-    add_security_builtin_endpoints(pp, &subguid, &group_guid, gv, !(flags & RTPS_PF_NO_BUILTIN_WRITERS), !(flags & RTPS_PF_NO_BUILTIN_READERS));
-  }
-#endif
 
   /* If the participant doesn't have the full set of builtin writers
      it depends on the privileged participant, which must exist, hence
@@ -1009,7 +1021,8 @@ dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv
      If it is the privileged participant, set the global variable
      pointing to it.
      Except when the participant is only locally available. */
-  if (!(flags & RTPS_PF_ONLY_LOCAL)) {
+  if (!(flags & RTPS_PF_ONLY_LOCAL))
+  {
     ddsrt_mutex_lock (&gv->privileged_pp_lock);
     if ((pp->bes & builtin_writers_besmask) != builtin_writers_besmask)
     {
@@ -1066,16 +1079,16 @@ dds_return_t new_participant_guid (ddsi_guid_t *ppguid, struct ddsi_domaingv *gv
   }
 
 #ifdef DDSI_INCLUDE_SECURITY
-  if (q_omg_participant_is_secure(pp))
+  if (q_omg_participant_is_secure (pp))
   {
     connect_participant_secure (gv, pp);
   }
 #endif
   return ret;
 
-#ifdef DDSI_INCLUDE_SECURITY
 not_allowed:
-new_pp_err_secprop:
+  if (ppconn)
+    ddsi_conn_free (ppconn);
   ddsi_plist_fini (pp->plist);
   ddsrt_free (pp->plist);
   inverse_uint32_set_fini (&pp->avail_entityids.x);
@@ -1085,7 +1098,6 @@ new_pp_err_secprop:
   ddsrt_mutex_lock (&gv->participant_set_lock);
   gv->nparticipants--;
   ddsrt_mutex_unlock (&gv->participant_set_lock);
-#endif
 new_pp_err:
   return ret;
 }
@@ -4886,6 +4898,7 @@ void new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *pp
   const bool is_secure = ((bes & NN_DISC_BUILTIN_ENDPOINT_PARTICIPANT_SECURE_ANNOUNCER) != 0);
   assert (!is_secure || (plist->present & PP_IDENTITY_TOKEN));
   assert (is_secure || (bes & ~NN_BES_MASK_NON_SECURITY) == 0);
+  (void) is_secure;
 
   assert (ppguid->entityid.u == NN_ENTITYID_PARTICIPANT);
   assert (entidx_lookup_proxy_participant_guid (gv->entity_index, ppguid) == NULL);