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 <dennis.potman@adlinktech.com>
Refactoring security core tests and adding more tests:
- Dynamically generate ca and identity certificates in authentication tests, so that certificate expiry is tested.
Added writing/reading samples to these tests to ensure that nodes can (or cannot) communicate in a specific test case
- Secure communication tests: improved the validation of encryption in wrapper
- Added test for access control plugin settings
- Replaced the in-code test identities (and included ca private keys), added an additional identity
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
Including FreeBSD in preprocessor conditionals for APPLE fixes issues with UDP make_socket, as described in issue #488.
Signed-off-by: Michael Gary Dodson <md403@cam.ac.uk>
The security handshake is started when a node receives an SPDP message. The
SPDP receiver will reply with an SPDP, followed by a dds.sec.auth_request.
Because the initial SPDP sender will receive the auth_request immediately
after (or even before) the SPDP reply message, that node may not have finished
(or not even started) matching the remote writers and therefore it drops
the auth_request message. This results in a time-out in the handshake
process, and the auth_request has to be re-send. To avoid this, a short
(rather arbitrarily chosen, based on local testing) sleep is introduced
before the auth_request message is sent.
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
There exist implementations that advertise security-related
built-endpoints regardless of whether the participant has security
configured. Therefore, the test whether security is enabled for the
participant cannot simply be the presence of such an endpoint, because
the absence of an IDENTITY_TOKEN in the data is then considered an
error.
This commit simply changes the check to requiring the presence of the
endpoint and the presence of the IDENTITY_TOKEN.
Signed-off-by: Erik Boasson <eb@ilities.com>
This fixes some issues with the new discovery data ("plist" topics)
discovered on interoperating with some other DDS implementations:
* The interpretation of a keyhash as if it were a valid sample was wrong
in various ways: inconsistent endianness, incorrect encoding
identifier and a missing sentinel. As Cyclone follows the spec and
always provides a well-formed payload, the problem only surfaces when
interoperating with implementations that expect the recipient to make
do with a keyhash.
* Various paths failed to check for failure causing potential null
pointer dereferences.
Signed-off-by: Erik Boasson <eb@ilities.com>
This is merely a more convenient way of obtaining it: otherwise one has
subscribe to the correct built-in topic, read the sample corresponding
to the entity's instance handle and get the "key" field. That's a bit
of a detour to get the network-wide unique identifier.
Signed-off-by: Erik Boasson <eb@ilities.com>
The former name should be less confusing. Backwards compatibility is
preserved by only adding the sensible name as a typedef.
Signed-off-by: Erik Boasson <eb@ilities.com>
Interpretation of the c.dsign_algo and c.kagree_algo properties must not
assume the binary property to be a null-terminated string.
Signed-off-by: Erik Boasson <eb@ilities.com>
* 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>
* Remove the "plist" and "rawcdr" abuse of the "serdata_default" sample
representation.
* Introduce a new "plist" topic type and a new "pserop" topic type. The
former represents parameter lists as used in discovery, the second
arbitrary samples using the serialiser in ddsi_plist.c.
* Introduce sertopics for each of the built-in "topics" used by the DDSI
discovery protocol using the two new topic types, and reference these
in the readers/writers used in discovery.
* Construct and deconstruct the discovery message by using the
conversion routines for these sample types, rather than fiddling with,
e.g., the baroque interface for adding parameter lists to messages.
* As a consequence, it introduces standardized logging of received and
transmitted discovery data and eliminates the annoying "(null)/(null)"
and "(blob)" descriptions in the trace.
* Limits the dumping of octet sequences in discovery data to the first
100 bytes to make the embedded certificates and permissions
documents (somewhat) manageable.
* Eliminates the (many) null pointer checks on reader/writer topics.
* Fixes the printing of nested sequences in discovery data (not used
before) and the formatting of GUIDs.
Various interfaces remain unchanged and so while this removes cruft from
the core code, it moves some of it into the conversion routines for the
new topic types.
It also now allocates some memory when processing incoming discovery
data, whereas before it had no need to do so. Allowing for aliasing of
data in the new sertopics and adding a way to initialize these specific
types on the stack (both minor changes) suffices for eliminating those
allocations.
Signed-off-by: Erik Boasson <eb@ilities.com>
Check actual topic type before "downcasting"
Signed-off-by: Erik Boasson <eb@ilities.com>
Free the memory we own and is actually allocated
Signed-off-by: Erik Boasson <eb@ilities.com>
Ignore logging newlines if nothing is buffered
Signed-off-by: Erik Boasson <eb@ilities.com>
Suffix data with "(trunc)" one byte earlier
The sample printing code changed over time and now stops as soon as it
can once it has filled up the buffer. As the return value is simply the
number of bytes written, if that number is equal to buffer size less
one (because of the terminating nul) it may or may not have been
truncated, but the likelihood is that it has been. So add the "(trunc)"
suffix once that point has been reached.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Do not rewrite secure messages in retransmit queue
Messages to be retransmitted spend some time on a transmit queue, and
are subject to the rewriting of the destination information to reduce
the number of outgoing copies. Self-evidently, altering the message
header does not sit well with encryption and/or authentication of
messages.
The way the rewriting works is that the offset of the "reader entity id"
in the DATA submessage is saved on message construction (the GUID prefix
is at a fixed location), so that it can be read and possibly zero'd out
later. The crypto transformations move the message around and it so
happens that it can end up pointing to the key id in the encoded
message. Zeroing that one out leads to uninterpretable messages.
This commit adds a message/event kind to distinguish between retransmit
that may and retransmit that may not be merged (and thus rewritten) and
gets used when the crypto plugin is invoked to transform a message.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Update comment on changing REXMIT to REXMIT_NOMERGE
Signed-off-by: Erik Boasson <eb@ilities.com>
Deleting a writer with unacknowledged data present in its WHC causes it
to linger for a configurable duration. Once it is lingering, there are
two routes to actually deleting the writer: because the samples get
acknowledged, or because the linger duration elapses.
When these two happen roughly concurrently, there was a possibility of
both succeeding in looking up the writer by its GUID, in which case one
of them then asserts on removing it from the entity index (if assertions
are enabled, if not, things are worse).
This fixes that by ensuring only one of the two actually does something,
as was always the intent.
Signed-off-by: Erik Boasson <eb@ilities.com>
As opposed to NOT_ALLOWED_BY_SECURITY. There is a meaningful
difference between something being disallowed and something being
impossible.
Co-Authored-By: Kyle Fazzari <github@status.e4ward.com>
Signed-off-by: Erik Boasson <eb@ilities.com>
Currently:
* DDS_HAS_SECURITY for DDS Security support
* DDS_HAS_LIFESPAN for lifespan QoS support
* DDS_HAS_DEADLINE_MISSED for "deadline missed" event support
These are defined to 1 if support for the feature is included in the
build and left undefined if it isn't.
Signed-off-by: Erik Boasson <eb@ilities.com>
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>
* read/take failed to restore the null pointer in the first entry of the
sample pointer array it gets passed, in the case no "loan" had been
allocated yet and it returned an empty set. The consequence is that
on a subsequence read it will reuse the address without marking at as
in use, so that a *second* read using with a null pointer in that
first entry will overwrite the first result. (Introduced by
d16264fd82.)
* return_loan failed to free all memory if its argument wasn't actually
a loan. There are many good arguments why the read/take/return_loan
interface is messed up, but in the context of the existing interface
this is a perfectly reasonable case: there is at most one "loan" for
each reader, but one can keep calling read/take and return_loan as if
there's an infinite number of "loans". It's just that the first gets
cached and the others don't.
Signed-off-by: Erik Boasson <eb@ilities.com>
Fixed a bug in the subject compare function for identity subjects, that
could cause using the incorrect permission grant in case multiple grants
are provided in the permissions configuration of the access control plugin.
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
The moving around and cleaning up of network code broke the IPv6
multicast support by memcpy'ing a sockaddr_in6 instead of an in6_addr in
a multicast join record.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Remove duplicated code in authentication plugin
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Fix build warnings
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Fix memory leak and call create_validate_asymmetrical_signature directly from create_validate_signature_impl
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Fix refcount issue (assert in openssl) for identity cert in hs remote info
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Refactoring of validate_handshake_token function
Co-authored-by: Erik Boasson <eb@ilities.com>
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
The participant listener creates a pong writer, setting a publication
matched listener on it. That listener can be invoked immediately and as
it queries the subscriptions reader, it must not be enabled before the
latter reader has been created.
Signed-off-by: Erik Boasson <eb@ilities.com>
