niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Replace pp null check by assert in dds_create_writer and dds_create_reader, and rewrite logic in q_omg_security_check_remote_writer_permissions

Browse source 
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
This commit is contained in:
Dennis Potman 2020-04-21 09:37:09 +02:00 • committed by eboasson
parent e8c349f96d
commit 7e6039763b
3 changed files with 38 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/core/ddsc/src/dds_reader.c
View file

@ -456,12 +456,11 @@ static dds_entity_t dds_create_reader_int (dds_entity_t participant_or_subscribe
thread_state_awake (lookup_thread_state (), gv); thread_state_awake (lookup_thread_state (), gv);
const struct ddsi_guid * ppguid = dds_entity_participant_guid (&sub->m_entity); const struct ddsi_guid * ppguid = dds_entity_participant_guid (&sub->m_entity);
struct participant * pp = entidx_lookup_participant_guid (gv->entity_index, ppguid); struct participant * pp = entidx_lookup_participant_guid (gv->entity_index, ppguid);
if (pp == NULL)
{ /* When deleting a participant, the child handles (that include the subscriber)
GVLOGDISC ("new_reader - participant "PGUIDFMT" not found\n", PGUID (*ppguid)); are removed before removing the DDSI participant. So at this point, within
rc = DDS_RETCODE_BAD_PARAMETER; the subscriber lock, we can assert that the participant exists. */
goto err_pp_not_found; assert (pp != NULL);
}
#ifdef DDSI_INCLUDE_SECURITY #ifdef DDSI_INCLUDE_SECURITY
/* Check if DDS Security is enabled */ /* Check if DDS Security is enabled */
@ -508,9 +507,8 @@ static dds_entity_t dds_create_reader_int (dds_entity_t participant_or_subscribe
#ifdef DDSI_INCLUDE_SECURITY #ifdef DDSI_INCLUDE_SECURITY
err_not_allowed: err_not_allowed:
#endif
err_pp_not_found:
thread_state_asleep (lookup_thread_state ()); thread_state_asleep (lookup_thread_state ());
#endif
err_bad_qos: err_bad_qos:
dds_delete_qos (rqos); dds_delete_qos (rqos);
dds_topic_allow_set_qos (tp); dds_topic_allow_set_qos (tp);

13
src/core/ddsc/src/dds_writer.c
View file

@ -329,12 +329,10 @@ dds_entity_t dds_create_writer (dds_entity_t participant_or_publisher, dds_entit
thread_state_awake (lookup_thread_state (), gv); thread_state_awake (lookup_thread_state (), gv);
const struct ddsi_guid *ppguid = dds_entity_participant_guid (&pub->m_entity); const struct ddsi_guid *ppguid = dds_entity_participant_guid (&pub->m_entity);
struct participant *pp = entidx_lookup_participant_guid (gv->entity_index, ppguid); struct participant *pp = entidx_lookup_participant_guid (gv->entity_index, ppguid);
if (pp == NULL) /* When deleting a participant, the child handles (that include the publisher)
{ are removed before removing the DDSI participant. So at this point, within
GVLOGDISC ("new_writer - participant "PGUIDFMT" not found\n", PGUID (*ppguid)); the publisher lock, we can assert that the participant exists. */
rc = DDS_RETCODE_BAD_PARAMETER; assert (pp != NULL);
goto err_pp_not_found;
}
#ifdef DDSI_INCLUDE_SECURITY #ifdef DDSI_INCLUDE_SECURITY
/* Check if DDS Security is enabled */ /* Check if DDS Security is enabled */
@ -377,9 +375,8 @@ dds_entity_t dds_create_writer (dds_entity_t participant_or_publisher, dds_entit
#ifdef DDSI_INCLUDE_SECURITY #ifdef DDSI_INCLUDE_SECURITY
err_not_allowed: err_not_allowed:
#endif
err_pp_not_found:
thread_state_asleep (lookup_thread_state ()); thread_state_asleep (lookup_thread_state ());
#endif
err_bad_qos: err_bad_qos:
dds_delete_qos(wqos); dds_delete_qos(wqos);
dds_topic_allow_set_qos (tp); dds_topic_allow_set_qos (tp);

58
src/core/ddsi/src/ddsi_security_omg.c
View file

@ -2137,7 +2137,6 @@ bool q_omg_security_check_remote_writer_permissions(const struct proxy_writer *p
DDS_Security_SecurityException exception = DDS_SECURITY_EXCEPTION_INIT; DDS_Security_SecurityException exception = DDS_SECURITY_EXCEPTION_INIT;
DDS_Security_PublicationBuiltinTopicDataSecure publication_data; DDS_Security_PublicationBuiltinTopicDataSecure publication_data;
DDS_Security_TopicBuiltinTopicData topic_data; DDS_Security_TopicBuiltinTopicData topic_data;
bool result = true;
if (!sc) if (!sc)
return true; return true;
@ -2156,42 +2155,39 @@ bool q_omg_security_check_remote_writer_permissions(const struct proxy_writer *p
} }
} }
if (SECURITY_INFO_IS_WRITE_PROTECTED(pwr->c.security_info)) if (!SECURITY_INFO_IS_WRITE_PROTECTED(pwr->c.security_info))
{ return true;
DDS_Security_PermissionsHandle permissions_handle;
if ((permissions_handle = get_permissions_handle(pp, pwr->c.proxypp)) == 0) DDS_Security_PermissionsHandle permissions_handle;
{ if ((permissions_handle = get_permissions_handle(pp, pwr->c.proxypp)) == 0)
GVTRACE("Secure remote writer "PGUIDFMT" proxypp does not have permissions handle yet\n", PGUID(pwr->e.guid)); {
return false; GVTRACE("Secure remote writer "PGUIDFMT" proxypp does not have permissions handle yet\n", PGUID(pwr->e.guid));
} return false;
}
q_omg_shallow_copy_PublicationBuiltinTopicDataSecure(&publication_data, &pwr->e.guid, pwr->c.xqos, &pwr->c.security_info);
bool result = sc->access_control_context->check_remote_datawriter(sc->access_control_context, permissions_handle, (int)domain_id, &publication_data, &exception);
if (!result)
{
if (!is_topic_discovery_protected(pp->sec_attr->permissions_handle, sc->access_control_context, publication_data.topic_name))
EXCEPTION_ERROR(gv, &exception, "Access control does not allow remote writer "PGUIDFMT": %s", PGUID(pwr->e.guid));
else else
DDS_Security_Exception_reset(&exception);
}
else
{
q_omg_shallow_copy_TopicBuiltinTopicData(&topic_data, publication_data.topic_name, publication_data.type_name);
result = sc->access_control_context->check_remote_topic(sc->access_control_context, permissions_handle, (int)domain_id, &topic_data, &exception);
q_omg_shallow_free_TopicBuiltinTopicData(&topic_data);
if (!result)
{ {
q_omg_shallow_copy_PublicationBuiltinTopicDataSecure(&publication_data, &pwr->e.guid, pwr->c.xqos, &pwr->c.security_info); if (!is_topic_discovery_protected(pp->sec_attr->permissions_handle, sc->access_control_context, publication_data.topic_name))
result = sc->access_control_context->check_remote_datawriter(sc->access_control_context, permissions_handle, (int)domain_id, &publication_data, &exception); EXCEPTION_ERROR(gv, &exception, "Access control does not allow remote topic %s: %s", publication_data.topic_name);
if (!result)
{
if (!is_topic_discovery_protected(pp->sec_attr->permissions_handle, sc->access_control_context, publication_data.topic_name))
EXCEPTION_ERROR(gv, &exception, "Access control does not allow remote writer "PGUIDFMT": %s", PGUID(pwr->e.guid));
else
DDS_Security_Exception_reset(&exception);
}
else else
{ DDS_Security_Exception_reset(&exception);
q_omg_shallow_copy_TopicBuiltinTopicData(&topic_data, publication_data.topic_name, publication_data.type_name);
result = sc->access_control_context->check_remote_topic(sc->access_control_context, permissions_handle, (int)domain_id, &topic_data, &exception);
q_omg_shallow_free_TopicBuiltinTopicData(&topic_data);
if (!result)
{
if (!is_topic_discovery_protected(pp->sec_attr->permissions_handle, sc->access_control_context, publication_data.topic_name))
EXCEPTION_ERROR(gv, &exception, "Access control does not allow remote topic %s: %s", publication_data.topic_name);
else
DDS_Security_Exception_reset(&exception);
}
}
q_omg_shallow_free_PublicationBuiltinTopicDataSecure(&publication_data);
} }
} }
q_omg_shallow_free_PublicationBuiltinTopicDataSecure(&publication_data);
return result; return result;
} }