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

Crypto endpoint relation compare routines cleanup

Browse source 
* Trying not to assume an int is at least 32 bits.

* Technically speaking, comparing "unrelated" addresses is undefined
  behaviour which can be avoided by a cast to uintptr_t.

* The early out if either local_crypto == 0 does work in context,
  provided the nodes in tree never have local_crypto == 0.  That implies
  crypto_insert_endpoint_relation must never have a 0 in there, which I
  think the callers do respect.  Still I think it is better to not hide
  these assumptions in the compare function and address the problem in
  the lookup function instead.

These changes likely make the code fractionally slower, but I do think
they improve clarity.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2020-03-31 16:27:21 +02:00 • committed by eboasson
parent 00710a4e5d
commit 99df0956e7
2 changed files with 17 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

42
src/security/builtin_plugins/cryptographic/src/crypto_objects.c
View file

@ -52,17 +52,13 @@ static int compare_endpoint_relation (const void *va, const void *vb)
{
const key_relation *ra = va;
const key_relation *rb = vb;
int r;
r = (int)(ra->key_id - rb->key_id);
if (r != 0 || ra->local_crypto == 0 || rb->local_crypto == 0)
{
return r;
}
if (ra->local_crypto > rb->local_crypto)
if (ra->key_id > rb->key_id)
return 1;
else if (ra->local_crypto < rb->local_crypto)
else if (ra->key_id < rb->key_id)
return -1;
else if ((uintptr_t) ra->local_crypto > (uintptr_t) rb->local_crypto)
return 1;
else if ((uintptr_t) ra->local_crypto < (uintptr_t) rb->local_crypto)
return -1;
else
return 0;
@ -72,8 +68,7 @@ static int compare_relation_key (const void *va, const void *vb)
{
const uint32_t *ka = va;
const uint32_t *kb = vb;
return (int)(*ka - *kb);
return (*ka == *kb) ? 0 : (*ka < *kb) ? -1 : 1;
}
bool crypto_object_valid(CryptoObject *obj, CryptoObjectKind_t kind)
@ -692,6 +687,7 @@ void crypto_insert_endpoint_relation(
remote_participant_crypto *rpc,
key_relation *relation)
{
assert (relation->local_crypto != NULL && relation->remote_crypto != NULL);
ddsrt_mutex_lock(&rpc->lock);
ddsrt_avl_insert(&endpoint_relation_treedef, &rpc->relation_index, CRYPTO_OBJECT_KEEP(relation));
ddsrt_mutex_unlock(&rpc->lock);
@ -702,15 +698,11 @@ void crypto_remove_endpoint_relation(
CryptoObject *lch,
uint32_t key_id)
{
key_relation template;
const key_relation template = { .key_id = key_id, .local_crypto = lch };
key_relation *relation;
ddsrt_avl_dpath_t dpath;
template.key_id = key_id;
template.local_crypto = lch;
ddsrt_mutex_lock(&rpc->lock);
relation = ddsrt_avl_lookup_dpath(&endpoint_relation_treedef, &rpc->relation_index, &template, &dpath);
relation = ddsrt_avl_lookup_dpath(&endpoint_relation_treedef, &rpc->relation_index, &template, &dpath);
if (relation)
{
ddsrt_avl_delete_dpath(&endpoint_relation_treedef, &rpc->relation_index, relation, &dpath);
@ -724,16 +716,14 @@ key_relation * crypto_find_endpoint_relation(
CryptoObject *lch,
uint32_t key_id)
{
key_relation template;
key_relation *relation;
template.key_id = key_id;
template.local_crypto = lch;
const key_relation template = { .key_id = key_id, .local_crypto = lch };
key_relation *relation, *cand;
ddsrt_mutex_lock(&rpc->lock);
relation = CRYPTO_OBJECT_KEEP(ddsrt_avl_lookup(&endpoint_relation_treedef, &rpc->relation_index, &template));
if ((cand = ddsrt_avl_lookup_succ_eq (&endpoint_relation_treedef, &rpc->relation_index, &template)) == NULL || cand->key_id != key_id)
relation = NULL;
else
relation = CRYPTO_OBJECT_KEEP (cand);
ddsrt_mutex_unlock(&rpc->lock);
return relation;
}

2
src/security/core/tests/secure_communication.c
View file

@ -544,7 +544,7 @@ CU_TheoryDataPoints(ddssec_secure_communication, multiple_readers) = {
CU_DataPoints(size_t, 1, 3, 1, 3), /* number of participants per domain */
CU_DataPoints(size_t, 3, 1, 3, 3), /* number of readers per participant */
};
CU_Theory((size_t n_dom, size_t n_pp, size_t n_rd), ddssec_secure_communication, multiple_readers, .timeout = 60, .disabled = false)
CU_Theory((size_t n_dom, size_t n_pp, size_t n_rd), ddssec_secure_communication, multiple_readers, .timeout = 90, .disabled = false)
{
DDS_Security_ProtectionKind metadata_pk[] = { PK_N, PK_SOA, PK_EOA };
DDS_Security_BasicProtectionKind payload_pk[] = { BPK_N, BPK_S, BPK_E };