From d53cdce8fe27b587c14232dcd8920192ff3b83c2 Mon Sep 17 00:00:00 2001 From: Dennis Potman Date: Thu, 26 Mar 2020 18:48:54 +0100 Subject: [PATCH] Access Control on_revoke_permissions implementation in DDSI Implement handler for access control on_revoke_permissions. This callback function disconnects and deletes all proxy participant that are using the revoked permissions handle (in case of remote permissions expire) and proxy participant that are connected with a participant for which the permissions expire (local permissions expire). Signed-off-by: Dennis Potman --- src/core/ddsi/src/ddsi_security_omg.c | 113 +++++++++++++++--- .../dds_security_api_authentication.h | 1 - .../authentication/src/authentication.c | 2 +- .../src/listeners_authentication_utests.c | 15 +-- .../core/tests/secure_communication.c | 6 +- 5 files changed, 103 insertions(+), 34 deletions(-) diff --git a/src/core/ddsi/src/ddsi_security_omg.c b/src/core/ddsi/src/ddsi_security_omg.c index 096608c..7766fcd 100644 --- a/src/core/ddsi/src/ddsi_security_omg.c +++ b/src/core/ddsi/src/ddsi_security_omg.c @@ -212,6 +212,8 @@ struct dds_security_context { struct pending_match_index security_matches; struct participant_sec_index partiticpant_index; + struct dds_security_access_control_listener ac_listener; + struct dds_security_authentication_listener auth_listener; }; typedef struct dds_security_context dds_security_context; @@ -525,34 +527,44 @@ static void proxypp_pp_match_free(struct ddsi_domaingv *gv, struct dds_security_ ddsrt_free(pm); } -static void pp_proxypp_unrelate(struct dds_security_context *sc, struct participant *pp, const ddsi_guid_t *proxypp_guid) +static void pp_proxypp_unrelate_locked(struct dds_security_context *sc, struct participant *pp, const ddsi_guid_t *proxypp_guid) { struct pp_proxypp_match *pm; ddsrt_avl_dpath_t dpath; - ddsrt_mutex_lock(&pp->sec_attr->lock); if ((pm = ddsrt_avl_clookup_dpath(&pp_proxypp_treedef, &pp->sec_attr->proxy_participants, proxypp_guid, &dpath)) != NULL) { ddsrt_avl_cdelete_dpath(&pp_proxypp_treedef, &pp->sec_attr->proxy_participants, pm, &dpath); pp_proxypp_match_free(sc, pm); } +} + +static void pp_proxypp_unrelate(struct dds_security_context *sc, struct participant *pp, const ddsi_guid_t *proxypp_guid) +{ + ddsrt_mutex_lock(&pp->sec_attr->lock); + pp_proxypp_unrelate_locked (sc, pp, proxypp_guid); ddsrt_mutex_unlock(&pp->sec_attr->lock); } +static void proxypp_pp_unrelate_locked(struct dds_security_context *sc, struct proxy_participant *proxypp, const ddsi_guid_t *pp_guid, int64_t pp_crypto_handle) +{ + DDSRT_UNUSED_ARG(pp_guid); + struct proxypp_pp_match *pm; + ddsrt_avl_dpath_t dpath; + + if ((pm = ddsrt_avl_lookup_dpath(&proxypp_pp_treedef, &proxypp->sec_attr->participants, &pp_crypto_handle, &dpath)) != NULL) + { + ddsrt_avl_delete_dpath(&proxypp_pp_treedef, &proxypp->sec_attr->participants, pm, &dpath); + proxypp_pp_match_free(proxypp->e.gv, sc, pm); + } +} + static void proxypp_pp_unrelate(struct dds_security_context *sc, struct proxy_participant *proxypp, const ddsi_guid_t *pp_guid, int64_t pp_crypto_handle) { - DDSRT_UNUSED_ARG(pp_guid); if (proxypp->sec_attr) { - struct proxypp_pp_match *pm; - ddsrt_avl_dpath_t dpath; - ddsrt_mutex_lock(&proxypp->sec_attr->lock); - if ((pm = ddsrt_avl_lookup_dpath(&proxypp_pp_treedef, &proxypp->sec_attr->participants, &pp_crypto_handle, &dpath)) != NULL) - { - ddsrt_avl_delete_dpath(&proxypp_pp_treedef, &proxypp->sec_attr->participants, pm, &dpath); - proxypp_pp_match_free(proxypp->e.gv, sc, pm); - } + proxypp_pp_unrelate_locked(sc, proxypp, pp_guid, pp_crypto_handle); ddsrt_mutex_unlock(&proxypp->sec_attr->lock); } } @@ -798,6 +810,70 @@ static void deinit_plugin_suite_config (dds_security_plugin_suite_config *suite_ deinit_plugin_config (&suite_config->cryptography); } +static DDS_Security_boolean on_revoke_permissions_cb(const dds_security_access_control *plugin, const DDS_Security_PermissionsHandle handle) +{ + struct ddsi_domaingv *gv = plugin->gv; + struct entidx_enum_participant epp; + struct entidx_enum_proxy_participant eproxypp; + struct participant *pp; + struct proxy_participant *proxypp; + bool local_perm = false; + thread_state_awake (lookup_thread_state (), gv); + + /* Find participants using this permissions handle */ + entidx_enum_participant_init (&epp, gv->entity_index); + while ((pp = entidx_enum_participant_next (&epp)) != NULL) + { + struct dds_security_context *sc = q_omg_security_get_secure_context(pp); + ddsrt_mutex_lock (&pp->sec_attr->lock); + if (pp->sec_attr->permissions_handle == handle) + { + uint32_t i = 0; + ddsrt_avl_citer_t it; + local_perm = true; + for (struct pp_proxypp_match *ppm = ddsrt_avl_citer_first (&pp_proxypp_treedef, &pp->sec_attr->proxy_participants, &it); ppm; ppm = ddsrt_avl_citer_next (&it), i++) + pp_proxypp_unrelate_locked (sc, pp, &ppm->proxypp_guid); + } + ddsrt_mutex_unlock (&pp->sec_attr->lock); + } + entidx_enum_participant_fini (&epp); + + /* Find proxy participants using this permissions handle */ + if (!local_perm) + { + entidx_enum_proxy_participant_init (&eproxypp, gv->entity_index); + while ((proxypp = entidx_enum_proxy_participant_next (&eproxypp)) != NULL) + { + ddsrt_mutex_lock (&proxypp->sec_attr->lock); + uint32_t i = 0; + ddsrt_avl_iter_t it; + bool del_proxypp = true; + for (struct proxypp_pp_match *ppm = ddsrt_avl_iter_first (&proxypp_pp_treedef, &proxypp->sec_attr->participants, &it); ppm; ppm = ddsrt_avl_iter_next (&it), i++) + { + if (ppm->permissions_handle == handle) + proxypp_pp_unrelate_locked (proxypp->sec_attr->sc, proxypp, &ppm->pp_guid, ppm->pp_crypto_handle); + else + del_proxypp = false; + } + ddsrt_mutex_unlock (&proxypp->sec_attr->lock); + if (del_proxypp) + delete_proxy_participant_by_guid (gv, &proxypp->e.guid, ddsrt_time_wallclock (), false); + } + entidx_enum_proxy_participant_fini (&eproxypp); + } + + thread_state_asleep (lookup_thread_state ()); + return true; +} + +static DDS_Security_boolean on_revoke_identity_cb(const dds_security_authentication *plugin, const DDS_Security_IdentityHandle handle) +{ + (void)plugin; + (void)handle; + return true; +} + + dds_return_t q_omg_security_load (dds_security_context *sc, const dds_qos_t *qos, struct ddsi_domaingv *gv) { dds_security_plugin_suite_config psc; @@ -836,18 +912,19 @@ dds_return_t q_omg_security_load (dds_security_context *sc, const dds_qos_t *qos } /* Add listeners */ -#if LISTENERS_IMPLEMENTED - if (!access_control_context->set_listener (access_control_context, &listener_ac, &ex)) + DDS_Security_SecurityException ex = DDS_SECURITY_EXCEPTION_INIT; + sc->ac_listener.on_revoke_permissions = on_revoke_permissions_cb; + if (!sc->access_control_context->set_listener (sc->access_control_context, &sc->ac_listener, &ex)) { GVERROR ("Could not set access_control listener: %s\n", ex.message ? ex.message : ""); goto error_set_ac_listener; } - if (!authentication_context->set_listener (authentication_context, &listener_auth, &ex)) + sc->auth_listener.on_revoke_identity = on_revoke_identity_cb; + if (!sc->authentication_context->set_listener (sc->authentication_context, &sc->auth_listener, &ex)) { GVERROR ("Could not set authentication listener: %s\n", ex.message ? ex.message : ""); - goto err_set_auth_listener; + goto error_set_auth_listener; } -#endif #if HANDSHAKE_IMPLEMENTED (void) q_handshake_initialize (); @@ -858,11 +935,9 @@ dds_return_t q_omg_security_load (dds_security_context *sc, const dds_qos_t *qos GVTRACE ("DDS Security plugins have been loaded\n"); return DDS_RETCODE_OK; -#if LISTENERS_IMPLEMENTED error_set_auth_listener: - access_control_context->set_listener (access_control_context, NULL, &ex); + sc->access_control_context->set_listener (sc->access_control_context, NULL, &ex); error_set_ac_listener: -#endif error_verify: release_plugins (gv, sc); error: diff --git a/src/security/api/include/dds/security/dds_security_api_authentication.h b/src/security/api/include/dds/security/dds_security_api_authentication.h index c0d9298..5909dac 100644 --- a/src/security/api/include/dds/security/dds_security_api_authentication.h +++ b/src/security/api/include/dds/security/dds_security_api_authentication.h @@ -29,7 +29,6 @@ typedef struct dds_security_authentication_listener dds_security_authentication_ /* AuthenticationListener interface */ typedef DDS_Security_boolean (*DDS_Security_authentication_listener_on_revoke_identity)( - dds_security_authentication_listener *context, const dds_security_authentication *plugin, const DDS_Security_IdentityHandle handle); diff --git a/src/security/builtin_plugins/authentication/src/authentication.c b/src/security/builtin_plugins/authentication/src/authentication.c index 937aa7e..0ff0864 100644 --- a/src/security/builtin_plugins/authentication/src/authentication.c +++ b/src/security/builtin_plugins/authentication/src/authentication.c @@ -639,7 +639,7 @@ static void validity_callback(struct dds_security_timed_dispatcher_t *d, dds_sec assert(listener); dds_security_authentication_listener *auth_listener = (dds_security_authentication_listener *)listener; if (auth_listener->on_revoke_identity) - auth_listener->on_revoke_identity(auth_listener, (dds_security_authentication *)info->auth, info->hdl); + auth_listener->on_revoke_identity((dds_security_authentication *)info->auth, info->hdl); } ddsrt_free(arg); } diff --git a/src/security/builtin_plugins/tests/listeners_authentication/src/listeners_authentication_utests.c b/src/security/builtin_plugins/tests/listeners_authentication/src/listeners_authentication_utests.c index 779b254..7575837 100644 --- a/src/security/builtin_plugins/tests/listeners_authentication/src/listeners_authentication_utests.c +++ b/src/security/builtin_plugins/tests/listeners_authentication/src/listeners_authentication_utests.c @@ -1811,7 +1811,7 @@ fill_handshake_message_token( } CU_ASSERT_FATAL (rc == DDS_SECURITY_VALIDATION_OK); assert(rc == DDS_SECURITY_VALIDATION_OK); // for Clang's static analyzer - + set_binary_property_value(signature, "signature", sign, (uint32_t)signlen); ddsrt_free(sign); @@ -1911,18 +1911,13 @@ fill_handshake_message_token( } -static DDS_Security_boolean -on_revoke_identity_cb( dds_security_authentication_listener *instance, - const dds_security_authentication *plugin, - const DDS_Security_IdentityHandle handle) +static DDS_Security_boolean on_revoke_identity_cb(const dds_security_authentication *plugin, const DDS_Security_IdentityHandle handle) { - DDSRT_UNUSED_ARG( instance ); - DDSRT_UNUSED_ARG( plugin ); - if (identity_handle_for_callback1 == DDS_SECURITY_HANDLE_NIL) { + DDSRT_UNUSED_ARG (plugin); + if (identity_handle_for_callback1 == DDS_SECURITY_HANDLE_NIL) identity_handle_for_callback1 = handle; - } else if (identity_handle_for_callback2 == DDS_SECURITY_HANDLE_NIL) { + else if (identity_handle_for_callback2 == DDS_SECURITY_HANDLE_NIL) identity_handle_for_callback2 = handle; - } printf( "Listener called for handle: %lld Local:%lld Remote:%lld\n", (long long) handle, (long long) local_identity_handle, (long long) remote_identity_handle2); return true; diff --git a/src/security/core/tests/secure_communication.c b/src/security/core/tests/secure_communication.c index 7ed18b5..92b40a4 100644 --- a/src/security/core/tests/secure_communication.c +++ b/src/security/core/tests/secure_communication.c @@ -445,13 +445,13 @@ static void test_discovery_liveliness_protection(DDS_Security_ProtectionKind dis { struct domain_sec_config domain_config = { discovery_pk, liveliness_pk, PK_N, PK_N, BPK_N, NULL }; /* FIXME: add more asserts in wrapper or test instead of just testing communication */ - test_write_read (&domain_config, 1, 1, 1, 1, 1, 1, &set_encryption_parameters_disc); + test_write_read (&domain_config, 1, 1, 1, 1, 1, 1, set_encryption_parameters_disc); } static void test_data_protection_kind(DDS_Security_ProtectionKind rtps_pk, DDS_Security_ProtectionKind metadata_pk, DDS_Security_BasicProtectionKind payload_pk) { struct domain_sec_config domain_config = { PK_N, PK_N, rtps_pk, metadata_pk, payload_pk, NULL }; - test_write_read (&domain_config, 1, 1, 1, 1, 1, 1, &set_encryption_parameters_basic); + test_write_read (&domain_config, 1, 1, 1, 1, 1, 1, set_encryption_parameters_basic); } static void test_multiple_readers(size_t n_dom, size_t n_pp, size_t n_rd, DDS_Security_ProtectionKind metadata_pk, DDS_Security_BasicProtectionKind payload_pk) @@ -486,7 +486,7 @@ static void test_payload_secret(DDS_Security_ProtectionKind rtps_pk, DDS_Securit memcpy (sample.text + n * strlen (secret), secret, strlen (secret)); sample.text[payload_sz - 1] = '\0'; - test_init (&domain_config, 1, 1, 1, 1, &set_encryption_parameters_secret); + test_init (&domain_config, 1, 1, 1, 1, set_encryption_parameters_secret); create_topic_name ("ddssec_secure_communication_", g_topic_nr++, name, sizeof name); qos = get_qos (); create_eps (&writers, &writer_topics, 1, 1, 1, name, &SecurityCoreTests_Type2_desc, g_pub_participants, qos, &dds_create_writer, DDS_PUBLICATION_MATCHED_STATUS);